Addresses the second #177 review: - Architecture (the silent allowlist drift): the writable provider-setting keys were maintained by hand in two TS-uncheckable places — the key-loop in ai-settings.service and the SQL ALLOWED list in the generic workspace repo (a miss there silently dropped a field on persist, exactly what bit chatApiStyle). Introduce one typed source of truth PROVIDER_SETTINGS_KEYS in ai.types (`satisfies readonly (keyof AiProviderSettings)[]`), have the service consume it, and keep the repo's own copy (it can't import AI types) guarded by a parity test so any future drift fails in CI. - Tests: - ai.service.include-usage.spec: mocks @ai-sdk/openai-compatible and asserts the factory is called with { includeUsage: true, baseURL, apiKey, fetch, name } — `.provider` alone could not catch a dropped includeUsage (the token-usage zeroing regression); also asserts the 'openai' style does NOT use it. - ai-provider-settings-keys.spec: the allowlist parity check + DTO validation for chatApiStyle (@IsIn accepts both values, rejects garbage, optional). - CHANGELOG: [Unreleased] entries for the new "Protocol" / chatApiStyle setting and the default provider change (openai -> openai-compatible). (#175, #177) server + client tsc clean; 42 ai/settings specs green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
44 lines
1.8 KiB
TypeScript
44 lines
1.8 KiB
TypeScript
import { validate } from 'class-validator';
|
|
import { plainToInstance } from 'class-transformer';
|
|
import { PROVIDER_SETTINGS_KEYS } from './ai.types';
|
|
import { AI_PROVIDER_SETTINGS_ALLOWED } from '@docmost/db/repos/workspace/workspace.repo';
|
|
import { UpdateAiSettingsDto } from './dto/update-ai-settings.dto';
|
|
|
|
/**
|
|
* Drift guard: the writable provider-settings keys are maintained in two layers
|
|
* that TypeScript cannot cross-check — PROVIDER_SETTINGS_KEYS (ai.types, used by
|
|
* the settings service) and AI_PROVIDER_SETTINGS_ALLOWED (the generic workspace
|
|
* repo's SQL boundary). A key missing from the repo copy silently drops the field
|
|
* on persist (exactly what happened to chatApiStyle), so this asserts they match.
|
|
*/
|
|
describe('provider-settings key allowlist parity', () => {
|
|
it('the repo SQL allowlist equals PROVIDER_SETTINGS_KEYS', () => {
|
|
expect([...AI_PROVIDER_SETTINGS_ALLOWED].sort()).toEqual(
|
|
[...PROVIDER_SETTINGS_KEYS].sort(),
|
|
);
|
|
});
|
|
});
|
|
|
|
/** DTO validation for the new chatApiStyle field (@IsIn(CHAT_API_STYLES)). */
|
|
describe('UpdateAiSettingsDto.chatApiStyle', () => {
|
|
const errorsFor = async (chatApiStyle: unknown) =>
|
|
validate(plainToInstance(UpdateAiSettingsDto, { chatApiStyle }));
|
|
|
|
it('accepts both valid values', async () => {
|
|
for (const v of ['openai-compatible', 'openai']) {
|
|
const errs = await errorsFor(v);
|
|
expect(errs.find((e) => e.property === 'chatApiStyle')).toBeUndefined();
|
|
}
|
|
});
|
|
|
|
it('rejects an unknown value', async () => {
|
|
const errs = await errorsFor('definitely-not-a-style');
|
|
expect(errs.find((e) => e.property === 'chatApiStyle')).toBeDefined();
|
|
});
|
|
|
|
it('accepts the field being omitted (optional)', async () => {
|
|
const errs = await validate(plainToInstance(UpdateAiSettingsDto, {}));
|
|
expect(errs.find((e) => e.property === 'chatApiStyle')).toBeUndefined();
|
|
});
|
|
});
|