refactor(ai): unify provider-settings allowlist + stronger chatApiStyle tests (#177 review)

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>
This commit is contained in:
claude code agent 227
2026-06-24 23:18:31 +03:00
parent 59190148db
commit 6edbbab43b
6 changed files with 169 additions and 17 deletions

View File

@@ -10,6 +10,29 @@ import {
import { ExpressionBuilder, sql } from 'kysely';
import { DB, Workspaces } from '@docmost/db/types/db';
/**
* Writable `settings.ai.provider` keys, enforced at this generic SQL layer. This
* repo cannot import AI-feature types, so this list is its own copy; a parity
* test (ai-provider-settings-keys.spec.ts) asserts it equals
* PROVIDER_SETTINGS_KEYS in ai.types so a future drift fails in CI rather than
* silently dropping a field at this boundary.
*/
export const AI_PROVIDER_SETTINGS_ALLOWED: readonly string[] = [
'driver',
'chatModel',
'chatApiStyle',
'embeddingModel',
'baseUrl',
'embeddingBaseUrl',
'sttModel',
'sttBaseUrl',
'sttApiStyle',
'sttLanguage',
'systemPrompt',
'publicShareChatModel',
'publicShareAssistantRoleId',
];
@Injectable()
export class WorkspaceRepo {
public baseFields: Array<keyof Workspaces> = [
@@ -239,9 +262,8 @@ export class WorkspaceRepo {
// is a real jsonb object, never a double-encoded string. The CASE self-heals
// workspaces whose settings.ai.provider was previously corrupted into an
// array/string.
const ALLOWED = ['driver', 'chatModel', 'chatApiStyle', 'embeddingModel', 'baseUrl', 'embeddingBaseUrl', 'sttModel', 'sttBaseUrl', 'sttApiStyle', 'sttLanguage', 'systemPrompt', 'publicShareChatModel', 'publicShareAssistantRoleId'];
const entries = Object.entries(provider).filter(
([k, v]) => v !== undefined && ALLOWED.includes(k),
([k, v]) => v !== undefined && AI_PROVIDER_SETTINGS_ALLOWED.includes(k),
);
const patch = entries.length
? sql`jsonb_build_object(${sql.join(