mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 22:46:32 +00:00
fix(assistant): green CI round after the shared ChatGPT plan
- Tests that still knew the ChatGPT sign-in as personal only follow the admin's shared plan: the service takes the caller with their role, the admin tiles show ChatGPT, the controller tests cover the admin routes. - The table refused an instance ChatGPT connection with a CHECK, so sharing the plan failed. A new migration rebuilds ai_connections without that check; the shipped migration stays as it was. - useChatgptConnect grew past the function size limit: reading the clipboard and noticing the return to the tab are helpers of their own. - The file link factory hands its overrides over apart from the row, like every other factory, which ends the deep type instantiation. - Knip: exports only their own file used are private again, and the booking cap reads MAX_WRITES_PER_STEP instead of an alias. - lint:i18n-keys reaches the tools' own action names through a key builder and the connection hints through literal keys. - MMehrSheet stays in the assistant hints pair only.
This commit is contained in:
Maurice
@@ -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).",
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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}`);
|
||||
}
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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(
|
||||
<AiConnectPanel
|
||||
scope="instance"
|
||||
@@ -71,7 +71,9 @@ describe('AiConnectPanel', () => {
|
||||
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(<AiConnectPanel scope="user" onConnected={vi.fn()} />);
|
||||
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(<AiConnectPanel scope="user" onConnected={vi.fn()} initialMode="chatgpt" />);
|
||||
await waitFor(() => expect(repo.chatgptStart).toHaveBeenCalledWith('user'));
|
||||
expect(screen.queryByRole('note')).not.toBeInTheDocument();
|
||||
unmount();
|
||||
|
||||
render(<AiConnectPanel scope="instance" onConnected={vi.fn()} />);
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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' };
|
||||
}
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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',
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -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<BookingImportPreviewItem[]> {
|
||||
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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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.' };
|
||||
|
||||
|
||||
@@ -163,9 +163,6 @@ export const ASSISTANT_SERVER_TOOLS: ReadonlySet<string> = 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;
|
||||
|
||||
|
||||
@@ -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, ' ');
|
||||
}
|
||||
|
||||
|
||||
@@ -37,5 +37,5 @@ export function linkFile(
|
||||
fileId: number,
|
||||
target: Pick<EntityData<FileLinks>, 'reservation' | 'assignment' | 'place' | 'budgetItem'>,
|
||||
): Promise<FileLinkRow> {
|
||||
return createRow(orm, FileLinks, { file: fileId, ...target });
|
||||
return createRow(orm, FileLinks, { file: fileId }, target);
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<unknown>) {
|
||||
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(
|
||||
|
||||
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user