diff --git a/client/scripts/feature-pairs.json b/client/scripts/feature-pairs.json index 915e41a8e..49f099c96 100644 --- a/client/scripts/feature-pairs.json +++ b/client/scripts/feature-pairs.json @@ -978,7 +978,6 @@ "src/mobile/screens/trip/sheets/MDaysSheet.tsx": "Button based day reorder sheet over planner.handleReorderDays; the desktop reorders by drag.", "src/mobile/screens/trip/sheets/MImportSheet.tsx": "Menu that routes to ImpFileStep and ImpListStep, with no logic of its own.", "src/mobile/screens/trip/sheets/MLinkedCosts.tsx": "Already shares useExpenseLinks with the desktop BookingCostsSection; only markup and a search filter are local.", - "src/mobile/screens/trip/sheets/MMehrSheet.tsx": "Phone dock overflow menu that only opens tabs and sheets.", "src/mobile/screens/trip/sheets/MTripSheetUi.tsx": "Phone sheet UI primitives (eyebrow, headers, stat boxes, action circles) and a time display helper.", "src/mobile/screens/trip/sheets/PlFileAttach.tsx": "Markup only; pending files and link actions come through props from the phone form sheets.", "src/mobile/screens/trip/sheets/PlSheetChrome.tsx": "Phone form sheet chrome primitives (header, footer, field classes).", diff --git a/client/scripts/i18n-unused-baseline.json b/client/scripts/i18n-unused-baseline.json index b5db1317a..04c6e7f33 100644 --- a/client/scripts/i18n-unused-baseline.json +++ b/client/scripts/i18n-unused-baseline.json @@ -30,7 +30,7 @@ "planner.ts": 53, "reservations.ts": 21, "roadtrip.ts": 38, - "settings.ts": 40, + "settings.ts": 37, "share.ts": 7, "shared.ts": 2, "storage.ts": 16, diff --git a/client/src/components/Admin/addons/AiAdminConfig.test.tsx b/client/src/components/Admin/addons/AiAdminConfig.test.tsx index 7fe6b9a48..993b71d9c 100644 --- a/client/src/components/Admin/addons/AiAdminConfig.test.tsx +++ b/client/src/components/Admin/addons/AiAdminConfig.test.tsx @@ -95,7 +95,8 @@ describe('the connection and the assistant', () => { expect(screen.getByText('Everyone uses the connection below unless they connect their own.')).toBeInTheDocument(); expect(screen.getByText('No connection yet.')).toBeInTheDocument(); expect(screen.getByRole('button', { name: /Own server/ })).toBeInTheDocument(); - expect(screen.queryByText('Sign in with ChatGPT')).not.toBeInTheDocument(); + // The admin may share their own ChatGPT plan with everyone, so its tile is offered too. + expect(screen.getByRole('button', { name: /Sign in with ChatGPT/ })).toBeInTheDocument(); await user.click(screen.getByRole('button', { name: 'AI Assistant' })); expect(screen.getByDisplayValue('50')).toBeInTheDocument(); expect(screen.getByText('Keep chats')).toBeInTheDocument(); diff --git a/client/src/components/Admin/addons/aiAddons.ts b/client/src/components/Admin/addons/aiAddons.ts index 2e597bad1..338e5c095 100644 --- a/client/src/components/Admin/addons/aiAddons.ts +++ b/client/src/components/Admin/addons/aiAddons.ts @@ -7,7 +7,7 @@ import type { Addon } from './addonModel'; * and MCP (TREK inside other AI apps). Their ids, rows and settings stay their * own; only the page groups them. Assistant and parsing share one connection. */ -export const AI_ADDON_IDS = ['ai_assistant', 'llm_parsing', 'mcp'] as const; +const AI_ADDON_IDS = ['ai_assistant', 'llm_parsing', 'mcp'] as const; export type AiAddonId = (typeof AI_ADDON_IDS)[number]; /** Each one's glyph, keyed by id: the icon column of an existing install still holds what it was seeded with. */ diff --git a/client/src/components/Assistant/AssistantChangeCard.test.tsx b/client/src/components/Assistant/AssistantChangeCard.test.tsx index 8c2a0c011..27db4bcd1 100644 --- a/client/src/components/Assistant/AssistantChangeCard.test.tsx +++ b/client/src/components/Assistant/AssistantChangeCard.test.tsx @@ -230,7 +230,8 @@ describe('AssistantChangeCard: on the map', () => { fireEvent.mouseEnter(screen.getByText('Ryoan-ji')); expect(useAssistantStore.getState().mapLink?.hoveredId).toBeNull(); - const row = screen.getByText('Pantheon').closest('[class*="py-2.5"]'); + const row = screen.getByText('Pantheon').closest('[class*="rounded-[12px]"]'); + expect(row).not.toBeNull(); expect(row).not.toHaveClass('bg-surface-hover'); act(() => useAssistantStore.setState({ mapLink: { tripId: TRIP, hoveredId: 'c1', source: 'map' } })); expect(row).toHaveClass('bg-surface-hover'); diff --git a/client/src/components/Assistant/AssistantChangeCard.tsx b/client/src/components/Assistant/AssistantChangeCard.tsx index 01d193301..c9d83f532 100644 --- a/client/src/components/Assistant/AssistantChangeCard.tsx +++ b/client/src/components/Assistant/AssistantChangeCard.tsx @@ -123,10 +123,12 @@ function KindTile({ ); } -/** The verb as a quiet line over the name of what it touches. */ +/** The key of a tool's own action name, for the tools TREK names one for. */ +const toolKey = (tool: string) => `assistant.tool.${tool}`; + /** What the change does: the tool's own action where TREK names it, else add, change or remove. */ function verbLabel(preview: AssistantChangePreview, t: (key: string) => string): string { - const own = preview.tool ? `assistant.tool.${preview.tool}` : null; + const own = preview.tool ? toolKey(preview.tool) : null; const named = own ? t(own) : null; return named && named !== own ? named : t(`assistant.change.verb.${preview.verbKey}`); } diff --git a/client/src/components/Assistant/useAssistantAttach.ts b/client/src/components/Assistant/useAssistantAttach.ts index d790aef07..50f29bec3 100644 --- a/client/src/components/Assistant/useAssistantAttach.ts +++ b/client/src/components/Assistant/useAssistantAttach.ts @@ -6,7 +6,7 @@ import { chatOf, useAssistantStore } from '../../store/assistantStore'; import { useToast } from '../shared/Toast'; /** What the booking import reads (the server checks again), and how big one file may be. */ -export const ATTACH_EXTENSIONS: readonly string[] = [ +const ATTACH_EXTENSIONS: readonly string[] = [ '.pdf', '.eml', '.pkpass', diff --git a/client/src/components/Settings/ai/AiConnectPanel.test.tsx b/client/src/components/Settings/ai/AiConnectPanel.test.tsx index 3664c4ef8..89fd60b92 100644 --- a/client/src/components/Settings/ai/AiConnectPanel.test.tsx +++ b/client/src/components/Settings/ai/AiConnectPanel.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-AI-CONNECT-PANEL-001 to FE-COMP-AI-CONNECT-PANEL-008: the two tiles, the key field, the ChatGPT paste step. +// FE-COMP-AI-CONNECT-PANEL-001 to FE-COMP-AI-CONNECT-PANEL-009: the mode tiles, the key field, the ChatGPT paste step. import userEvent from '@testing-library/user-event'; import type { AiStatusResponse } from '@trek/shared'; import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -63,7 +63,7 @@ describe('AiConnectPanel', () => { expect(repo.chatgptStart).not.toHaveBeenCalled(); }); - it('FE-COMP-AI-CONNECT-PANEL-002: the admin picks between a key and an own server, prefilled from AI parsing', () => { + it('FE-COMP-AI-CONNECT-PANEL-002: the admin picks between a key, their ChatGPT plan and an own server, prefilled from AI parsing', () => { render( { prefill={{ kind: 'custom', baseUrl: 'http://ollama:11434/v1', model: 'qwen3:8b' }} /> ); - expect(screen.queryByText('Sign in with ChatGPT')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Paste an API key/ })).toHaveAttribute('aria-pressed', 'false'); + expect(screen.getByRole('button', { name: /Sign in with ChatGPT/ })).toHaveAttribute('aria-pressed', 'false'); + expect(repo.chatgptStart).not.toHaveBeenCalled(); expect(screen.getByRole('button', { name: /Own server/ })).toHaveAttribute('aria-pressed', 'true'); expect(screen.getByText('Taken over from AI Parsing.')).toBeInTheDocument(); expect(screen.getByDisplayValue('http://ollama:11434/v1')).toBeInTheDocument(); @@ -183,4 +185,22 @@ describe('AiConnectPanel', () => { render(); expect(screen.getByRole('button', { name: 'Using Claude? Connect in Claude' })).toBeInTheDocument(); }); + + it('FE-COMP-AI-CONNECT-PANEL-009: the admin signing in with ChatGPT is told the plan is shared, and the sign-in is for everyone', async () => { + const user = userEvent.setup(); + const { unmount } = render(); + await waitFor(() => expect(repo.chatgptStart).toHaveBeenCalledWith('user')); + expect(screen.queryByRole('note')).not.toBeInTheDocument(); + unmount(); + + render(); + await user.click(screen.getByRole('button', { name: /Sign in with ChatGPT/ })); + expect(screen.getByRole('note')).toHaveTextContent( + 'You share your own ChatGPT plan with everyone on this server: their chats count against your plan.' + ); + await waitFor(() => + expect(screen.getByRole('link', { name: /Open ChatGPT sign-in/ })).toHaveAttribute('href', FLOW.authorizeUrl) + ); + expect(repo.chatgptStart).toHaveBeenLastCalledWith('instance'); + }); }); diff --git a/client/src/components/Settings/ai/AiConnectPanel.tsx b/client/src/components/Settings/ai/AiConnectPanel.tsx index cc30339ad..fa3ab218b 100644 --- a/client/src/components/Settings/ai/AiConnectPanel.tsx +++ b/client/src/components/Settings/ai/AiConnectPanel.tsx @@ -384,11 +384,11 @@ function prefillModeOf(prefill: AiParsingPrefill | null): AiConnectMode | null { } /** - * Connecting a provider in under a minute: two tiles, and the form of the one - * picked below them. A user picks between signing in with ChatGPT and pasting - * a key; the admin, setting up the connection for everyone, between a key and - * an own server. ChatGPT is never shared across an instance, an own server - * never named by a user (#1772): the server refuses both, so they are not offered. + * Connecting a provider in under a minute: a tile for each way, and the form + * of the one picked below them. A user picks between signing in with ChatGPT + * and pasting a key; the admin, setting up the connection for everyone, between + * a key, their own ChatGPT plan and an own server. An own server is never named + * by a user (#1772): the server refuses it, so it is not offered. */ export default function AiConnectPanel({ scope, onConnected, initialMode, prefill = null }: AiConnectPanelProps) { const { t } = useTranslation(); diff --git a/client/src/components/Settings/ai/aiConnectModel.ts b/client/src/components/Settings/ai/aiConnectModel.ts index f5382383c..fabebe033 100644 --- a/client/src/components/Settings/ai/aiConnectModel.ts +++ b/client/src/components/Settings/ai/aiConnectModel.ts @@ -128,8 +128,6 @@ export function svgDataUrl(svg: string): string { */ export function aiSectionText(assistant: boolean, parsing: boolean): { titleKey: string; hintKey: string } { if (!assistant) return { titleKey: 'settings.aiParsing.title', hintKey: 'assistant.connect.descriptionParsingOnly' }; - return { - titleKey: 'assistant.connect.title', - hintKey: parsing ? 'assistant.connect.descriptionWithParsing' : 'assistant.connect.description', - }; + if (parsing) return { titleKey: 'assistant.connect.title', hintKey: 'assistant.connect.descriptionWithParsing' }; + return { titleKey: 'assistant.connect.title', hintKey: 'assistant.connect.description' }; } diff --git a/client/src/components/Settings/ai/useAiConnect.test.ts b/client/src/components/Settings/ai/useAiConnect.test.ts index 6b3e24542..9e1ffb0c9 100644 --- a/client/src/components/Settings/ai/useAiConnect.test.ts +++ b/client/src/components/Settings/ai/useAiConnect.test.ts @@ -1,5 +1,5 @@ -// FE-HOOK-AI-CONNECT-001 to FE-HOOK-AI-CONNECT-012: pasting a key, an own server, the ChatGPT sign-in and the OpenRouter return. -import { act, renderHook, waitFor } from '@testing-library/react'; +// FE-HOOK-AI-CONNECT-001 to FE-HOOK-AI-CONNECT-013: pasting a key, an own server, the ChatGPT sign-in and the OpenRouter return. +import { act, fireEvent, renderHook, waitFor } from '@testing-library/react'; import { createElement, type ReactNode } from 'react'; import { MemoryRouter, useLocation } from 'react-router'; import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -218,6 +218,37 @@ describe('useChatgptConnect', () => { expect(onConnected).not.toHaveBeenCalled(); expect(repo.chatgptStart).toHaveBeenCalledTimes(1); }); + + it('FE-HOOK-AI-CONNECT-013: the clipboard fills the field where the browser lets it be read, else the field gets the cursor', async () => { + const readText = vi.fn(async () => 'not an address'); + Object.defineProperty(navigator, 'clipboard', { configurable: true, value: { readText } }); + Object.defineProperty(window, 'isSecureContext', { configurable: true, value: true }); + const { result } = renderHook(() => useChatgptConnect({ active: true, onConnected: vi.fn() }), { + wrapper: wrapperAt(), + }); + await waitFor(() => expect(result.current.flow).not.toBeNull()); + const input = document.createElement('input'); + document.body.append(input); + result.current.inputRef.current = input; + + act(() => result.current.pasteFromClipboard()); + await waitFor(() => expect(result.current.pasted).toBe('not an address')); + expect(repo.chatgptFinish).not.toHaveBeenCalled(); + + // Without a readable clipboard the button and the return from the sign-in window both focus the field. + Object.defineProperty(window, 'isSecureContext', { configurable: true, value: false }); + act(() => result.current.pasteFromClipboard()); + expect(document.activeElement).toBe(input); + input.blur(); + act(() => { + result.current.change(''); + result.current.markOpened(); + }); + fireEvent.focus(window); + expect(document.activeElement).toBe(input); + expect(readText).toHaveBeenCalledTimes(1); + input.remove(); + }); }); describe('OpenRouter', () => { diff --git a/client/src/components/Settings/ai/useAiConnect.ts b/client/src/components/Settings/ai/useAiConnect.ts index 676dfcabb..dc9c7d49d 100644 --- a/client/src/components/Settings/ai/useAiConnect.ts +++ b/client/src/components/Settings/ai/useAiConnect.ts @@ -150,6 +150,35 @@ export function useCustomConnect({ return { baseUrl, setBaseUrl, apiKey, setApiKey, model, setModel, host, ready, saving, error, submit }; } +/** + * Hands `onText` what the clipboard holds where the browser lets the page read + * it; where it does not, or the user refuses, `onUnreadable` runs instead. + */ +function readClipboard(onText: (text: string) => void, onUnreadable: () => void) { + if (!navigator.clipboard?.readText || !window.isSecureContext) { + onUnreadable(); + return; + } + navigator.clipboard.readText().then(onText, onUnreadable); +} + +/** Runs `onBack` each time the user comes back to this tab while `watching` holds. */ +function useOnReturnToTab(watching: boolean, onBack: () => void) { + const back = useEffectEvent(onBack); + useEffect(() => { + if (!watching) return; + const onVisible = () => { + if (document.visibilityState === 'visible') back(); + }; + document.addEventListener('visibilitychange', onVisible); + window.addEventListener('focus', onVisible); + return () => { + document.removeEventListener('visibilitychange', onVisible); + window.removeEventListener('focus', onVisible); + }; + }, [watching]); +} + /** * Signing in with ChatGPT (K3). The sign-in address is fetched as soon as the * step shows, so opening it is a plain link and no popup blocker interferes. @@ -241,47 +270,22 @@ export function useChatgptConnect({ return true; }; + const focusField = () => inputRef.current?.focus(); + /** Reads the address off the clipboard where the browser allows it, else hands the field the cursor. */ - const pasteFromClipboard = () => { - if (!navigator.clipboard?.readText || !window.isSecureContext) { - inputRef.current?.focus(); - return; - } - navigator.clipboard.readText().then( - (text) => { - if (!paste(text)) change(text); - }, - () => inputRef.current?.focus() - ); - }; + const pasteFromClipboard = () => + readClipboard((text) => { + if (!paste(text)) change(text); + }, focusField); // Back from the sign-in window: the address the user just copied is taken // off the clipboard where the browser allows it, else the field has the cursor. - const takeCopied = useEffectEvent(() => { + useOnReturnToTab(opened, () => { if (pasted.trim() || busy) return; - if (!navigator.clipboard?.readText || !window.isSecureContext) { - inputRef.current?.focus(); - return; - } - navigator.clipboard.readText().then( - (text) => { - if (!paste(text)) inputRef.current?.focus(); - }, - () => inputRef.current?.focus() - ); + readClipboard((text) => { + if (!paste(text)) focusField(); + }, focusField); }); - useEffect(() => { - if (!opened) return; - const onBack = () => { - if (document.visibilityState === 'visible') takeCopied(); - }; - document.addEventListener('visibilitychange', onBack); - window.addEventListener('focus', onBack); - return () => { - document.removeEventListener('visibilitychange', onBack); - window.removeEventListener('focus', onBack); - }; - }, [opened]); return { flow, diff --git a/server/src/db/migrations/Migration20200101046230_an_admin_can_share_a_chatgpt_plan.ts b/server/src/db/migrations/Migration20200101046230_an_admin_can_share_a_chatgpt_plan.ts new file mode 100644 index 000000000..08184270f --- /dev/null +++ b/server/src/db/migrations/Migration20200101046230_an_admin_can_share_a_chatgpt_plan.ts @@ -0,0 +1,55 @@ +import { Migration } from '@mikro-orm/migrations'; + +/** + * An admin may share their own ChatGPT sign-in with everyone, so a ChatGPT + * connection can be the instance connection too. The table refused that with + * `CHECK(kind <> 'chatgpt' OR scope = 'user')`, and saving the shared plan + * failed on it. + * + * SQLite cannot drop a CHECK in place, so the table is rebuilt without that + * one; every other check stays as it was. Nothing references + * `ai_connections`, so the old table can go before the new one takes its + * name. Ids are copied, so a connection keeps its id. + */ +export class Migration20200101046230_an_admin_can_share_a_chatgpt_plan extends Migration { + override name = 'Migration20200101046230_an_admin_can_share_a_chatgpt_plan'; + + override up(): void { + this.addSql(` + CREATE TABLE ai_connections_new ( + id INTEGER PRIMARY KEY, + scope TEXT NOT NULL CHECK(scope IN ('instance', 'user')), + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + kind TEXT NOT NULL CHECK(kind IN ('anthropic', 'openai', 'gemini', 'openrouter', 'chatgpt', 'custom')), + model TEXT NOT NULL, + base_url TEXT, + secrets TEXT, + secrets_version INTEGER NOT NULL DEFAULT 0, + hint TEXT, + account TEXT, + status TEXT NOT NULL DEFAULT 'ok' CHECK(status IN ('ok', 'needs_reauth', 'signed_out', 'error')), + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CHECK((scope = 'user') = (user_id IS NOT NULL)), + CHECK(kind <> 'custom' OR (scope = 'instance' AND base_url IS NOT NULL)), + CHECK(base_url IS NULL OR kind = 'custom') + ) + `); + this.addSql(` + INSERT INTO ai_connections_new ( + id, scope, user_id, kind, model, base_url, secrets, secrets_version, hint, account, status, created_at, updated_at + ) + SELECT + id, scope, user_id, kind, model, base_url, secrets, secrets_version, hint, account, status, created_at, updated_at + FROM ai_connections + `); + this.addSql('DROP TABLE ai_connections'); + this.addSql('ALTER TABLE ai_connections_new RENAME TO ai_connections'); + this.addSql( + 'CREATE UNIQUE INDEX IF NOT EXISTS idx_ai_connections_user ON ai_connections(user_id) WHERE user_id IS NOT NULL', + ); + this.addSql( + 'CREATE UNIQUE INDEX IF NOT EXISTS idx_ai_connections_instance ON ai_connections(scope) WHERE user_id IS NULL', + ); + } +} diff --git a/server/src/nest/assistant/assistant-booking-import.service.ts b/server/src/nest/assistant/assistant-booking-import.service.ts index a18261dcf..05bf11c25 100644 --- a/server/src/nest/assistant/assistant-booking-import.service.ts +++ b/server/src/nest/assistant/assistant-booking-import.service.ts @@ -3,7 +3,7 @@ import { BookingImportService } from '../booking-import/booking-import.service'; import { DemoService } from '../common/demo.service'; import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { TripAccessService } from '../trip-membership/trip-access.service'; -import { MAX_BOOKINGS_PER_FILE } from './assistant-tools.constants'; +import { MAX_WRITES_PER_STEP } from './assistant-tools.constants'; import type { ToolRun } from './assistant-tools.service'; import { Injectable } from '@nestjs/common'; import { bookingImportPreviewItemSchema, LLM_PHOTO_EXTENSIONS, type BookingImportPreviewItem } from '@trek/shared'; @@ -54,7 +54,8 @@ export class AssistantBookingImportService { async read(userId: number, file: Express.Multer.File): Promise { const mode = (await this.bookingImport.aiAvailable(userId)) ? 'fallback-on-empty' : 'no-ai'; const preview = await this.bookingImport.preview([file], mode, userId); - return preview.items.slice(0, MAX_BOOKINGS_PER_FILE); + // One file proposes no more bookings than one step may hold changes; the rest are left out. + return preview.items.slice(0, MAX_WRITES_PER_STEP); } /** diff --git a/server/src/nest/assistant/assistant-decisions.helpers.ts b/server/src/nest/assistant/assistant-decisions.helpers.ts index 6c5eac169..a05501a11 100644 --- a/server/src/nest/assistant/assistant-decisions.helpers.ts +++ b/server/src/nest/assistant/assistant-decisions.helpers.ts @@ -3,7 +3,7 @@ import type { AssistantMessagePart } from '@trek/shared'; /** What the model is told about a change the user discarded. */ export const DISCARDED = { applied: false, reason: 'The user discarded this change.' }; /** ... one the user left undecided by asking something else. */ -export const CANCELLED = { applied: false, reason: 'The user moved on without deciding.' }; +const CANCELLED = { applied: false, reason: 'The user moved on without deciding.' }; /** ... one that was approved but never ran. */ export const NOT_RUN = { applied: false, reason: 'The change could not be run.' }; diff --git a/server/src/nest/assistant/assistant-tools.constants.ts b/server/src/nest/assistant/assistant-tools.constants.ts index d77692f68..a58cc73c2 100644 --- a/server/src/nest/assistant/assistant-tools.constants.ts +++ b/server/src/nest/assistant/assistant-tools.constants.ts @@ -163,9 +163,6 @@ export const ASSISTANT_SERVER_TOOLS: ReadonlySet = new Set([ ...Object.keys(ASSISTANT_SERVER_WRITE_TOOLS), ]); -/** Bookings one attached file may propose; the rest are left out, as a step holds no more changes. */ -export const MAX_BOOKINGS_PER_FILE = MAX_WRITES_PER_STEP; - /** The largest file the assistant reads. */ export const MAX_ATTACHMENT_BYTES = 10 * 1024 * 1024; diff --git a/server/src/nest/assistant/tool-call-generic.helpers.ts b/server/src/nest/assistant/tool-call-generic.helpers.ts index 6c9ce0a91..daaa72768 100644 --- a/server/src/nest/assistant/tool-call-generic.helpers.ts +++ b/server/src/nest/assistant/tool-call-generic.helpers.ts @@ -79,7 +79,7 @@ function clip(value: string): string { } /** "update_vacay_plan" reads as "update vacay plan"; only shown where nothing better is known. */ -export function humanToolName(name: string): string { +function humanToolName(name: string): string { return name.replace(/_/g, ' '); } diff --git a/server/tests/helpers/factories/files.ts b/server/tests/helpers/factories/files.ts index 1c2af52b9..6b6b2286b 100644 --- a/server/tests/helpers/factories/files.ts +++ b/server/tests/helpers/factories/files.ts @@ -37,5 +37,5 @@ export function linkFile( fileId: number, target: Pick, 'reservation' | 'assignment' | 'place' | 'budgetItem'>, ): Promise { - return createRow(orm, FileLinks, { file: fileId, ...target }); + return createRow(orm, FileLinks, { file: fileId }, target); } diff --git a/server/tests/unit/db/legacy-baseline.test.ts b/server/tests/unit/db/legacy-baseline.test.ts index 761ce70f4..2186848da 100644 --- a/server/tests/unit/db/legacy-baseline.test.ts +++ b/server/tests/unit/db/legacy-baseline.test.ts @@ -89,6 +89,7 @@ const UNNUMBERED = [ 'Migration20200101046200_a_user_can_connect_an_ai_provider', 'Migration20200101046210_the_trip_assistant_remembers_its_conversations', 'Migration20200101046220_a_chat_message_can_say_it_came_from_the_assistant', + 'Migration20200101046230_an_admin_can_share_a_chatgpt_plan', ]; // The last step the positional runner ever had; planLegacyBaseline refuses a // schema_version past it. diff --git a/server/tests/unit/db/repositories/AiConnections.repository.test.ts b/server/tests/unit/db/repositories/AiConnections.repository.test.ts index 999e15071..cf09e8c4e 100644 --- a/server/tests/unit/db/repositories/AiConnections.repository.test.ts +++ b/server/tests/unit/db/repositories/AiConnections.repository.test.ts @@ -1,8 +1,8 @@ /** * AiConnectionsRepository and the table under it: one connection per user and * one for the instance, a save that replaces in place, the version check a - * token refresh writes under, and the checks that keep ChatGPT personal and a - * custom server the admin's. + * token refresh writes under, and the checks that keep a custom server the + * admin's while a ChatGPT sign-in may be either side's. */ import { AiConnections } from '../../../../src/db/entities/AiConnections.entity'; import { Users } from '../../../../src/db/entities/Users.entity'; @@ -80,13 +80,15 @@ describe('AiConnectionsRepository', () => { expect(await repo.findById(id)).toBeNull(); }); - it('AICREPO-003: the table refuses a ChatGPT instance connection, a custom user one and a stray base URL', async () => { + it('AICREPO-003: the table refuses a custom user connection and a stray base URL, takes a shared ChatGPT plan', async () => { const { user } = await makeUser(t); - await expect(repo.save(null, { ...KEY, kind: 'chatgpt' })).rejects.toThrow(/CHECK/); await expect(repo.save(user.id, { ...KEY, kind: 'custom', baseUrl: 'http://lan' })).rejects.toThrow(/CHECK/); await expect(repo.save(null, { ...KEY, baseUrl: 'http://lan' })).rejects.toThrow(/CHECK/); await expect(repo.save(null, { ...KEY, kind: 'custom' })).rejects.toThrow(/CHECK/); await expect(insertRow(t, AiConnections, { scope: 'user', kind: 'openai', model: 'm' })).rejects.toThrow(/CHECK/); + // An admin may share their own ChatGPT plan with everyone. + await repo.save(null, { ...KEY, kind: 'chatgpt' }); + expect(await repo.findInstance()).toMatchObject({ scope: 'instance', user_id: null, kind: 'chatgpt' }); }); it('AICREPO-004: a user deleted takes their connection along', async () => { diff --git a/server/tests/unit/nest/ai-connections/ai-connections.controller.test.ts b/server/tests/unit/nest/ai-connections/ai-connections.controller.test.ts index 5955f3c36..7ccae2565 100644 --- a/server/tests/unit/nest/ai-connections/ai-connections.controller.test.ts +++ b/server/tests/unit/nest/ai-connections/ai-connections.controller.test.ts @@ -23,6 +23,8 @@ import { GUARDS_METADATA, HTTP_CODE_METADATA, METHOD_METADATA, PATH_METADATA } f import { describe, expect, it, vi } from 'vitest'; const VIEW = { scope: 'user', kind: 'anthropic', model: 'm', hint: '…1234', status: 'ok', host: 'api.anthropic.com' }; +const INSTANCE_VIEW = { ...VIEW, scope: 'instance', kind: 'chatgpt', host: 'chatgpt.com' }; +const ADMIN_CONNECTION = { connection: INSTANCE_VIEW, prefill: null }; const user = { id: 7, role: 'user' } as User; function services() { @@ -35,16 +37,17 @@ function services() { removeOwn: vi.fn(async () => undefined), ackPrivacy: vi.fn(async () => undefined), ownView: vi.fn(async () => VIEW), + instanceView: vi.fn(async () => INSTANCE_VIEW), readSettings: vi.fn(async () => ({ policy: 'user' })), writeSettings: vi.fn(async () => ({ policy: 'user' })), - instanceConnection: vi.fn(async () => ({ connection: null, prefill: null })), + instanceConnection: vi.fn(async () => ADMIN_CONNECTION), saveInstance: vi.fn(async () => ({ connection: null, prefill: null })), removeInstance: vi.fn(async () => undefined), listInstanceModels: vi.fn(async () => []), }, chatgpt: { start: vi.fn(async () => ({ flowId: 'f', authorizeUrl: 'https://auth.openai.com', expiresAt: 'x' })), - finish: vi.fn(async () => undefined), + finish: vi.fn(async (): Promise<'user' | 'instance'> => 'user'), signOut: vi.fn(async () => undefined), }, openrouter: { @@ -136,10 +139,14 @@ describe('AiConnectionsController', () => { await controller.startChatgpt(user); expect(s.chatgpt.start).toHaveBeenCalledWith(7); const flowId = '7d3c2a64-0f1e-4c4c-9a39-7d2a7a0c5b11'; - expect( - await controller.finishChatgpt(user, { flowId, pastedUrl: 'http://127.0.0.1:1455/auth/callback?code=c' }), - ).toBe(VIEW); - expect(s.chatgpt.finish).toHaveBeenCalledWith(7, flowId, 'http://127.0.0.1:1455/auth/callback?code=c'); + const pastedUrl = 'http://127.0.0.1:1455/auth/callback?code=c'; + expect(await controller.finishChatgpt(user, { flowId, pastedUrl })).toBe(VIEW); + // The service gets the caller with their role, so it can refuse a sign-in for everyone to a non-admin. + expect(s.chatgpt.finish).toHaveBeenCalledWith(user, flowId, pastedUrl); + expect(s.connections.ownView).toHaveBeenCalledWith(7); + // A sign-in an admin started for everyone answers with the instance's connection. + s.chatgpt.finish.mockResolvedValueOnce('instance'); + expect(await controller.finishChatgpt(user, { flowId, pastedUrl })).toBe(INSTANCE_VIEW); expect(await controller.signOutChatgpt(user)).toBe(VIEW); expect(s.chatgpt.signOut).toHaveBeenCalledWith(7); }); @@ -162,6 +169,7 @@ describe('AiAdminController', () => { controller: new AiAdminController( s.connections as unknown as AiConnectionsService, s.openrouter as unknown as OpenrouterAuthService, + s.chatgpt as unknown as ChatgptAuthService, ), }; } @@ -174,9 +182,17 @@ describe('AiAdminController', () => { }); expect(Reflect.getMetadata(MANAGED_FORBIDDEN, AiAdminController)?.reason).toBeTruthy(); expect( - ['settings', 'saveSettings', 'connection', 'saveConnection', 'removeConnection', 'models', 'startOpenrouter'].map( - (name) => route(AiAdminController, name), - ), + [ + 'settings', + 'saveSettings', + 'connection', + 'saveConnection', + 'removeConnection', + 'models', + 'startOpenrouter', + 'startChatgpt', + 'signOutChatgpt', + ].map((name) => route(AiAdminController, name)), ).toEqual([ { method: RequestMethod.GET, path: 'settings', httpCode: undefined }, { method: RequestMethod.PUT, path: 'settings', httpCode: undefined }, @@ -185,6 +201,8 @@ describe('AiAdminController', () => { { method: RequestMethod.DELETE, path: 'connection', httpCode: 200 }, { method: RequestMethod.GET, path: 'models', httpCode: undefined }, { method: RequestMethod.POST, path: 'connect/openrouter/start', httpCode: 200 }, + { method: RequestMethod.POST, path: 'connect/chatgpt/start', httpCode: 200 }, + { method: RequestMethod.POST, path: 'connect/chatgpt/sign-out', httpCode: 200 }, ]); }); @@ -206,5 +224,10 @@ describe('AiAdminController', () => { expect(await controller.models(admin)).toEqual({ models: [] }); await controller.startOpenrouter(admin); expect(s.openrouter.start).toHaveBeenCalledWith(1, 'instance'); + // The admin's own ChatGPT plan, shared with everyone: started and signed out for the instance. + await controller.startChatgpt(admin); + expect(s.chatgpt.start).toHaveBeenCalledWith(1, 'instance'); + expect(await controller.signOutChatgpt(admin)).toBe(ADMIN_CONNECTION); + expect(s.chatgpt.signOut).toHaveBeenCalledWith(1, 'instance'); }); }); diff --git a/server/tests/unit/nest/ai-connections/chatgpt-auth.service.test.ts b/server/tests/unit/nest/ai-connections/chatgpt-auth.service.test.ts index 290986e76..0b7d2666c 100644 --- a/server/tests/unit/nest/ai-connections/chatgpt-auth.service.test.ts +++ b/server/tests/unit/nest/ai-connections/chatgpt-auth.service.test.ts @@ -24,7 +24,7 @@ import { OidcService } from '../../../../src/nest/oidc/oidc.service'; import { createSnapshotTestDb } from '../../../helpers/db-mock'; import { findRow } from '../../../helpers/factories/rows'; import { readAppSetting } from '../../../helpers/factories/settings'; -import { makeAdmin, makeUser } from '../../../helpers/factories/users'; +import { makeUser } from '../../../helpers/factories/users'; import { resetTestDb } from '../../../helpers/test-db'; import { createTestModule, type TestModule } from '../../../helpers/test-module'; import { MikroORM } from '@mikro-orm/core'; @@ -99,6 +99,12 @@ async function refusal(promise: Promise) { return { status: (err as DomainError).getStatus(), body: (err as DomainError).toBody() }; } +/** A stored user as the service takes the caller of a finished sign-in: the id and the role. */ +async function caller(role: 'user' | 'admin' = 'user'): Promise<{ id: number; role: string }> { + const { user } = await makeUser(orm(), { role }); + return { id: user.id, role }; +} + /** Starts a sign-in and answers its flow id with the state and nonce the authorize URL carried. */ async function started(userId: number, scope: 'user' | 'instance' = 'user') { const start = await service().start(userId, scope); @@ -161,7 +167,7 @@ describe('starting a sign-in', () => { describe('finishing a sign-in', () => { it('CGPT-010: exchanges the code, checks the ID token and saves the connection', async () => { - const { user } = await makeUser(orm()); + const user = await caller(); const flow = await started(user.id); const access = accessToken(Math.floor(Date.now() / 1000) + 3600); safeFetch.mockResolvedValueOnce( @@ -176,7 +182,7 @@ describe('finishing a sign-in', () => { verifyIdToken.mockResolvedValueOnce(claims({ nonce: flow.nonce })); await service().finish( - user.id, + user, flow.flowId, callbackUrl({ code: 'code-1', state: flow.state, client_id: 'oaiapp_new_1' }), ); @@ -207,8 +213,8 @@ describe('finishing a sign-in', () => { }); it('CGPT-030: an admin shares their plan with everyone; only an admin finishes that sign-in', async () => { - const { user: admin } = await makeAdmin(orm()); - const { user: member } = await makeUser(orm()); + const admin = await caller('admin'); + const member = await caller(); const shared = await started(admin.id, 'instance'); const url = callbackUrl({ code: 'c', state: shared.state, client_id: 'oaiapp_shared' }); // A flow belongs to whoever started it, so a member cannot even take it. @@ -219,7 +225,10 @@ describe('finishing a sign-in', () => { ); verifyIdToken.mockResolvedValueOnce(claims({ nonce: shared.nonce })); expect(await service().finish(admin, shared.flowId, url)).toBe('instance'); - expect(await findRow(orm(), AiConnections, { scope: 'instance' })).toMatchObject({ kind: 'chatgpt', user: null }); + expect(await findRow(orm(), AiConnections, { scope: 'instance' })).toMatchObject({ + kind: 'chatgpt', + user_id: null, + }); expect(await findRow(orm(), AiConnections, { user: admin.id })).toBeNull(); // An admin who is no longer one cannot finish a sign-in for everyone they started before. @@ -230,7 +239,7 @@ describe('finishing a sign-in', () => { }); it('CGPT-011: a wrong address is refused before the flow is used, so the right one still works', async () => { - const { user } = await makeUser(orm()); + const user = await caller(); const flow = await started(user.id); const wrong = await Promise.all( ['not a url', 'http://localhost:1455/auth/callback?code=x', 'https://chatgpt.com/'].map((pasted) => @@ -251,8 +260,8 @@ describe('finishing a sign-in', () => { }); it('CGPT-012: another state, an unknown or foreign flow, and a used one read as expired', async () => { - const { user } = await makeUser(orm()); - const { user: other } = await makeUser(orm()); + const user = await caller(); + const other = await caller(); const flow = await started(user.id); const url = callbackUrl({ code: 'c', state: flow.state, client_id: 'oaiapp_x' }); expect((await refusal(service().finish(other, flow.flowId, url))).status).toBe(410); @@ -270,7 +279,7 @@ describe('finishing a sign-in', () => { }); it('CGPT-013: a client id that is not an issued one, named twice, or another than the saved one is refused', async () => { - const { user } = await makeUser(orm()); + const user = await caller(); const refused = await Promise.all( ['&client_id=evil', '&client_id=oaiapp_a&client_id=oaiapp_b'].map(async (extra) => { const flow = await started(user.id); @@ -285,18 +294,14 @@ describe('finishing a sign-in', () => { expect( ( await refusal( - service().finish( - user.id, - flow.flowId, - callbackUrl({ code: 'c', state: flow.state, client_id: 'oaiapp_other' }), - ), + service().finish(user, flow.flowId, callbackUrl({ code: 'c', state: flow.state, client_id: 'oaiapp_other' })), ) ).body, ).toMatchObject({ code: 'wrong_url' }); }); it('CGPT-014: a nonce that does not match, another account and a plan without the scope are refused', async () => { - const { user } = await makeUser(orm()); + const user = await caller(); const answer = () => tokenAnswer(200, { access_token: 'at', refresh_token: 'rt', id_token: 'id', scope: PLAN_SCOPES }); @@ -334,7 +339,7 @@ describe('finishing a sign-in', () => { }); it('CGPT-015: a refused code reads as expired, a token endpoint that is down as provider_unavailable', async () => { - const { user } = await makeUser(orm()); + const user = await caller(); let flow = await started(user.id); safeFetch.mockResolvedValueOnce(tokenAnswer(400, { error: 'invalid_grant' })); expect( diff --git a/server/tests/unit/nest/assistant/assistant-booking-import.service.test.ts b/server/tests/unit/nest/assistant/assistant-booking-import.service.test.ts index 1c7dfc510..5657b2c07 100644 --- a/server/tests/unit/nest/assistant/assistant-booking-import.service.test.ts +++ b/server/tests/unit/nest/assistant/assistant-booking-import.service.test.ts @@ -5,7 +5,7 @@ * applies it, the same an MCP write runs. */ import { AssistantBookingImportService } from '../../../../src/nest/assistant/assistant-booking-import.service'; -import { MAX_BOOKINGS_PER_FILE } from '../../../../src/nest/assistant/assistant-tools.constants'; +import { MAX_WRITES_PER_STEP } from '../../../../src/nest/assistant/assistant-tools.constants'; import { BookingImportService } from '../../../../src/nest/booking-import/booking-import.service'; import { DemoService } from '../../../../src/nest/common/demo.service'; import { DomainError } from '../../../../src/nest/common/domain-error'; @@ -42,7 +42,7 @@ async function make(opts: Opts = {}) { aiAvailable: vi.fn(async () => opts.ai === true), readsImages: vi.fn(async () => opts.images === true), preview: vi.fn(async (_files: Express.Multer.File[], _mode: string, _userId: number) => ({ - items: Array.from({ length: MAX_BOOKINGS_PER_FILE + 2 }, (_, index) => ({ ...ITEM, title: `Leg ${index}` })), + items: Array.from({ length: MAX_WRITES_PER_STEP + 2 }, (_, index) => ({ ...ITEM, title: `Leg ${index}` })), warnings: [], })), confirm: vi.fn(async () => ({ created: [{ id: 81, title: 'ICE 123', type: 'train', day_id: 31 }] })), @@ -88,7 +88,7 @@ describe('AssistantBookingImportService', () => { const items = await plain.service.read(7, file('a.pdf')); const given = expect.objectContaining({ originalname: 'a.pdf' }); expect(plain.bookingImport.preview).toHaveBeenCalledWith([given], 'no-ai', 7); - expect(items).toHaveLength(MAX_BOOKINGS_PER_FILE); + expect(items).toHaveLength(MAX_WRITES_PER_STEP); const ai = await make({ ai: true }); await ai.service.read(7, file('a.pdf')); expect(ai.bookingImport.preview.mock.calls[0][1]).toBe('fallback-on-empty');