diff --git a/.changeset/24769-settings-test-handlers.md b/.changeset/24769-settings-test-handlers.md new file mode 100644 index 00000000000..9bec90aa0cc --- /dev/null +++ b/.changeset/24769-settings-test-handlers.md @@ -0,0 +1,12 @@ +--- +'@objectstack/service-settings': patch +--- + +fix(service-settings): Setup's AI "Test embedder" and Knowledge "Test connection" buttons are answered, and their answers say they checked the form only + +Clause-②: no + +- **Before.** Both buttons answered `400 SETTINGS_ACTION_FAILED` with "No handler registered for action 'test_embedder' in 'ai'." and "No handler registered for action 'test' in 'knowledge'." The manifests declared both buttons and the package exported both handlers (`aiTestEmbedderActionHandler`, `knowledgeTestActionHandler`), but `SettingsServicePlugin`'s default `actionHandlers` map did not register them. +- **Now.** The default map registers both, beside `mail.test`, `sms.test`, `storage.test` and `ai.test`. A host that passes its own `actionHandlers` still replaces the whole default map, and a plugin can still replace any one handler with `SettingsService.registerAction`. +- **What each button checks.** Both are form checks and send nothing over the network. "Test embedder" checks that a provider is chosen, that an API key is set (Ollama excepted) and that `custom` / `azure` have a Base URL. "Test connection" checks that a managed `libsql://` Turso URL has an auth token and that RAGFlow has a Base URL and an API key. The in-memory adapter answers with a warning that there is no external service to probe. +- **Answer text.** A passing "Test embedder" now ends "This checks the form only — no embedding request was sent." Every passing "Test connection" on Turso or RAGFlow now says "This checks the form only — no connection was attempted." The old "Mount @objectstack/embedder-openai + a knowledge adapter to exercise live calls" and "Mount @objectstack/knowledge-ragflow to exercise live calls" are gone: neither package registers these actions, so mounting it never made either check live. diff --git a/packages/services/service-settings/src/manifests/ai.manifest.test.ts b/packages/services/service-settings/src/manifests/ai.manifest.test.ts index d3688da1982..4d6d55ab75a 100644 --- a/packages/services/service-settings/src/manifests/ai.manifest.test.ts +++ b/packages/services/service-settings/src/manifests/ai.manifest.test.ts @@ -354,18 +354,35 @@ describe('aiTestActionHandler — live-call hint carries the cloud boundary (#11 }); } - // Contrast control, so the assertions above cannot be satisfied by blanket- - // rewording every hint in this file: the embedder hint names - // `@objectstack/embedder-openai`, which IS built in this repo (8 path hits - // under `git ls-tree -r --name-only HEAD | grep -cF /embedder-openai/`, against - // 0 for `/service-ai/`), so that instruction is followable as written and stays - // a plain mount line. Asserted here, deliberately not edited — commit 99ccbb9c8 fences - // this site out by name. - it('leaves the embedder hint a plain mount line — its package IS built here', async () => { - const r = await runTestEmbedder({ embedder_provider: 'openai', embedder_api_key: 'sk-test' }); + // The embedder hint used to stand here as this block's contrast control — a + // plain "Mount @objectstack/embedder-openai + a knowledge adapter to exercise + // live calls", kept because that package IS built in this repo. Followable, + // but measured false for the button it answered (#24769): no package in this + // repo registers `ai/test_embedder`, so mounting it never made "Test + // embedder" live. Its replacement is pinned with the embedder handler below; + // the three branches above are still exercised one by one, so a blanket + // rewording cannot stand in for them. +}); + +/** + * #24769 — "Test embedder" is now answered (the handler is in the plugin's + * default map), and what it answers is a form check: no embedding request is + * sent and no package in this repo replaces it with a live probe. A green + * answer must say so, and must not name a package whose mounting would make the + * check live. + */ +describe('aiTestEmbedderActionHandler — a form check says it is one (#24769)', () => { + it('states that no embedding request was sent, and promises no live call', async () => { + const r = await runTestEmbedder({ + embedder_provider: 'openai', + embedder_api_key: 'sk-test', + embedder_model: 'text-embedding-3-small', + }); expect(r.ok).toBe(true); - expect(String(r.message)).toContain( - 'Mount @objectstack/embedder-openai + a knowledge adapter to exercise live calls', - ); + expect(r.severity).toBe('info'); + const message = String(r.message); + expect(message).toContain('text-embedding-3-small'); + expect(message).toMatch(/no embedding request was sent/); + expect(message).not.toMatch(/to exercise live calls/); }); }); diff --git a/packages/services/service-settings/src/manifests/ai.manifest.ts b/packages/services/service-settings/src/manifests/ai.manifest.ts index 79900f6f6e3..bb1ad034d9a 100644 --- a/packages/services/service-settings/src/manifests/ai.manifest.ts +++ b/packages/services/service-settings/src/manifests/ai.manifest.ts @@ -427,12 +427,18 @@ export const aiTestActionHandler: SettingsActionHandler = async ({ values, paylo }; /** - * Built-in fallback handler for `ai/test_embedder`. Real implementation - * with a live `embed()` round-trip lives in `@objectstack/service-ai` or - * `@objectstack/service-knowledge` and overrides this stub at runtime. + * Handler for `ai/test_embedder` ("Test embedder"), registered by + * `SettingsServicePlugin`'s default `actionHandlers` map (#24769). * - * This fallback validates form completeness only — no network call — - * so the button is useful even when no embedder plugin is mounted. + * It checks form completeness only — a provider is chosen, an API key is + * present (Ollama excepted), a Base URL is present for `custom` / `azure` — + * and sends NO embedding request. Nothing in this repository replaces it with + * a live probe: neither `@objectstack/embedder-openai` nor + * `@objectstack/service-knowledge` registers `ai/test_embedder` (a plugin that + * did would call `SettingsService.registerAction`, which replaces this + * handler). So its success message says what was checked and that no request + * was sent, and names no package whose mounting would make this check live — + * none does. */ export const aiTestEmbedderActionHandler: SettingsActionHandler = async ({ values, payload }) => { const overrides = extractOverrides(payload); @@ -469,8 +475,8 @@ export const aiTestEmbedderActionHandler: SettingsActionHandler = async ({ value ok: true, severity: 'info', message: - `${provider} embedder configured (model=${model}). ` + - 'Mount @objectstack/embedder-openai + a knowledge adapter to exercise live calls.', + `${provider} embedder settings are complete (model=${model}). ` + + 'This checks the form only — no embedding request was sent.', }; }; diff --git a/packages/services/service-settings/src/manifests/knowledge.manifest.test.ts b/packages/services/service-settings/src/manifests/knowledge.manifest.test.ts index 5a24c275054..4fb9eb061e2 100644 --- a/packages/services/service-settings/src/manifests/knowledge.manifest.test.ts +++ b/packages/services/service-settings/src/manifests/knowledge.manifest.test.ts @@ -145,20 +145,28 @@ describe('knowledgeTestActionHandler', () => { expect(message).not.toMatch(/Mount @objectstack\/knowledge-turso to exercise live calls/); }); - // Contrast, so the assertion above cannot be satisfied by blanket-rewording every - // hint: ragflow's plugin IS built here, and its hint stays the plain mount line. - it('ragflow live-call hint stays a plain mount line (its plugin is built here)', async () => { - const r = await knowledgeTestActionHandler({ - values: { - adapter: 'ragflow', - ragflow_base_url: 'http://localhost:9380', - ragflow_api_key: 'k', - }, - } as any); - expect(r.ok).toBe(true); - expect(String(r.message)).toContain( - 'Mount @objectstack/knowledge-ragflow to exercise live calls', - ); + // #24769 — "Test connection" is now answered (the handler is in the plugin's + // default map), and what it answers is a form check: no package in this repo + // reads this namespace or registers `knowledge/test`, so nothing replaces it + // with a live probe. Every green answer on a remote adapter must say no + // connection was attempted. Ragflow's hint used to be "Mount + // @objectstack/knowledge-ragflow to exercise live calls" — followable (the + // plugin IS built here) but false for this button, which mounting it never + // made live; no answer may carry that promise. + it('every green answer on a remote adapter says no connection was attempted', async () => { + const remote: Array<[string, Record]> = [ + ['turso, no URL', { adapter: 'turso' }], + ['turso, local file', { adapter: 'turso', turso_url: 'file:./knowledge.db' }], + ['turso, managed', { adapter: 'turso', turso_url: 'libsql://x.turso.io', turso_auth_token: 't' }], + ['ragflow', { adapter: 'ragflow', ragflow_base_url: 'http://localhost:9380', ragflow_api_key: 'k' }], + ]; + for (const [label, values] of remote) { + const r = await knowledgeTestActionHandler({ values } as any); + expect(r.ok, label).toBe(true); + const message = String(r.message); + expect(message, label).toMatch(/no connection was attempted/); + expect(message, label).not.toMatch(/to exercise live calls/); + } }); it('rejects ragflow without base URL', async () => { diff --git a/packages/services/service-settings/src/manifests/knowledge.manifest.ts b/packages/services/service-settings/src/manifests/knowledge.manifest.ts index 8e41db0e17f..396195582b0 100644 --- a/packages/services/service-settings/src/manifests/knowledge.manifest.ts +++ b/packages/services/service-settings/src/manifests/knowledge.manifest.ts @@ -137,12 +137,24 @@ const manifest = { export const knowledgeSettingsManifest = manifest as unknown as SettingsManifest; /** - * Built-in fallback handler for `knowledge/test`. The real probe with - * a live `healthCheck()` round-trip lives in `@objectstack/service-knowledge` - * and overrides this stub at runtime (mirrors the AI / storage patterns). + * The sentence every answer on a remote adapter carries, so a green "Test + * connection" is never read as a connection that succeeded. + */ +const FORM_CHECK_ONLY = 'This checks the form only — no connection was attempted.'; + +/** + * Handler for `knowledge/test` ("Test connection"), registered by + * `SettingsServicePlugin`'s default `actionHandlers` map (#24769). * - * This fallback only validates form completeness so the button is - * usable when no knowledge adapter plugin is mounted. + * It checks form completeness only — a Turso auth token beside a managed + * `libsql://` URL, a RAGFlow Base URL and API key — and opens NO connection. + * Nothing in this repository replaces it with a live probe: + * `@objectstack/service-knowledge`, `@objectstack/knowledge-memory` and + * `@objectstack/knowledge-ragflow` neither read this namespace nor register + * `knowledge/test` (a plugin that did would call + * `SettingsService.registerAction`, which replaces this handler). So every + * answer on a remote adapter says no connection was attempted, and none names + * a package whose mounting would make this check live — none does. */ export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values, payload }) => { const overrides = extractOverrides(payload); @@ -163,7 +175,9 @@ export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values return { ok: true, severity: 'info', - message: 'No URL configured — adapter will reuse the tenant\'s primary libSQL connection at runtime.', + message: + 'No URL configured — adapter will reuse the tenant\'s primary libSQL connection at runtime. ' + + FORM_CHECK_ONLY, }; } const u = String(url); @@ -178,7 +192,7 @@ export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values ok: true, severity: 'info', message: - `Turso adapter configured (${u}). Live calls need @objectstack/knowledge-turso, ` + + `Turso adapter configured (${u}). ${FORM_CHECK_ONLY} Live calls need @objectstack/knowledge-turso, ` + 'which this platform does not ship: it is built in the ObjectStack Cloud monorepo ' + 'and published on its own version track, so check that it offers a release ' + 'compatible with your platform version before mounting it.', @@ -195,7 +209,7 @@ export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values return { ok: true, severity: 'info', - message: `RAGFlow adapter configured (${merged.ragflow_base_url}). Mount @objectstack/knowledge-ragflow to exercise live calls.`, + message: `RAGFlow adapter configured (${merged.ragflow_base_url}). ${FORM_CHECK_ONLY}`, }; } diff --git a/packages/services/service-settings/src/settings-default-action-handlers.pin.test.ts b/packages/services/service-settings/src/settings-default-action-handlers.pin.test.ts new file mode 100644 index 00000000000..7e6f128e63f --- /dev/null +++ b/packages/services/service-settings/src/settings-default-action-handlers.pin.test.ts @@ -0,0 +1,197 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Every `action_button` a bundled settings manifest declares against the + * settings action route is answered by the plugin's DEFAULT composition + * (#24769). + * + * ## The defect + * + * `ai.test_embedder` ("Test embedder") and `knowledge.test` ("Test connection") + * were declared in their manifests, so Setup rendered both buttons, and their + * handlers (`aiTestEmbedderActionHandler`, `knowledgeTestActionHandler`) were + * exported beside the manifests — but `SettingsServicePlugin`'s default + * `actionHandlers` map registered only `mail.test`, `sms.test`, `storage.test` + * and `ai.test`. Both buttons answered `400 SETTINGS_ACTION_FAILED`, "No + * handler registered". Nothing registers either action later: at a43405d59 the + * only `registerAction` calls against a settings namespace outside this + * package are plugin-email (`mail`), service-sms (`sms`) and service-storage + * (`storage`). + * + * ## What is pinned + * + * The CLASS, not the two instances: each bundled manifest's `http`-kind + * `action_button` whose URL is the settings action route is driven through + * `runAction` on the service the default-constructed plugin registers, and must + * reach a handler. "Reached" is observed, not inferred from message text: every + * handler the plugin registers is wrapped to record its own invocation, so an + * action is honoured when a registered handler ran, or when `runAction` + * answered `ok` without one — its built-in `reset` fallback, the one action it + * serves for every namespace. The no-handler answer is neither: no handler + * runs and it is `ok: false`. A new button declared without its handler, or a + * handler dropped from the default map, goes red here by name. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { PluginContext } from '@objectstack/core'; +import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts'; +import { SettingsServicePlugin } from './settings-service-plugin.js'; +import { SettingsService } from './settings-service.js'; +import type { SettingsActionHandler } from './settings-service.types.js'; +import { registerSettingsRoutes } from './settings-routes.js'; +import { builtinSettingsManifests } from './manifests/index.js'; + +/** The settings action route under the plugin's default base path. */ +const SETTINGS_ACTION_ROUTE = /^\/api\/settings\/([^/]+)\/([^/]+)$/; + +interface DeclaredRouteAction { + manifestNamespace: string; + buttonId: string; + namespace: string; + actionId: string; +} + +/** Every bundled `action_button` whose handler POSTs to the settings action route. */ +function declaredRouteActions(): DeclaredRouteAction[] { + const out: DeclaredRouteAction[] = []; + for (const manifest of builtinSettingsManifests) { + for (const spec of manifest.specifiers as any[]) { + if (spec.type !== 'action_button' || spec.handler?.kind !== 'http') continue; + const m = SETTINGS_ACTION_ROUTE.exec(String(spec.handler.url)); + if (!m) continue; + out.push({ manifestNamespace: manifest.namespace, buttonId: spec.id, namespace: m[1], actionId: m[2] }); + } + } + return out; +} + +/** + * Construct the plugin with NO options — the composition a host gets by + * default — and capture the service its `init()` registers. Every handler the + * plugin registers is wrapped so the test can see which one ran. + */ +async function bootDefaultPlugin() { + const invoked = new Set(); + const register = SettingsService.prototype.registerAction; + vi.spyOn(SettingsService.prototype, 'registerAction').mockImplementation(function ( + this: SettingsService, + namespace: string, + actionId: string, + handler: SettingsActionHandler, + ) { + register.call(this, namespace, actionId, (input) => { + invoked.add(`${namespace}/${actionId}`); + return handler(input); + }); + }); + let service: SettingsService | undefined; + const ctx = { + logger: { info() {}, warn() {}, error() {}, debug() {} }, + registerService: (name: string, svc: unknown) => { + if (name === 'settings') service = svc as SettingsService; + }, + getService: (name: string) => { + throw new Error(`fixture: no '${name}' service`); + }, + } as unknown as PluginContext; + await new SettingsServicePlugin({ env: {} }).init(ctx); + if (!service) throw new Error('fixture: the plugin registered no settings service'); + return { service, invoked }; +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('[#24769] the default composition answers every declared settings action button', () => { + it('enumerates the declared population — the two buttons this card is about among it', () => { + const declared = declaredRouteActions().map((a) => `${a.namespace}/${a.actionId}`); + expect(declared).toEqual( + expect.arrayContaining([ + 'ai/test', + 'ai/reset', + 'ai/test_embedder', + 'knowledge/test', + 'mail/test', + 'sms/test', + 'storage/test', + ]), + ); + }); + + it('each button targets its own namespace and its own id on the route', () => { + for (const a of declaredRouteActions()) { + expect(a.namespace, `${a.manifestNamespace}.${a.buttonId}`).toBe(a.manifestNamespace); + expect(a.actionId, `${a.manifestNamespace}.${a.buttonId}`).toBe(a.buttonId); + } + }); + + it('every declared action reaches a handler — none answers "no handler registered"', async () => { + const { service, invoked } = await bootDefaultPlugin(); + const unanswered: string[] = []; + for (const { namespace, actionId } of declaredRouteActions()) { + const result = await service.runAction(namespace, actionId, {}, {}); + if (!invoked.has(`${namespace}/${actionId}`) && !result.ok) unanswered.push(`${namespace}/${actionId}`); + } + expect(unanswered).toEqual([]); + }); +}); + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +async function post(http: MockHttp, namespace: string, actionId: string, body: unknown) { + const req = { params: { namespace, actionId }, query: {}, body, headers: {}, method: 'POST', path: '/' } as IHttpRequest; + const state: { status: number; body?: any } = { status: 200 }; + const res = { + json: (data: unknown) => { state.body = data; }, + send: () => {}, + status: (code: number) => { state.status = code; return res; }, + header: () => res, + } as unknown as IHttpResponse; + await http.routes.get('POST /api/settings/:namespace/:actionId')!(req, res); + return state; +} + +describe('[#24769] the two buttons the card measured, at the HTTP door', () => { + // A platform admin: both namespaces gate reads and writes (and so actions) on + // `manage_platform_settings`. + const admin = () => ({ enforced: true, permissions: ['manage_platform_settings'], userId: 'usr_admin' }); + + it('POST /api/settings/ai/test_embedder is answered by the embedder check (200, not 400)', async () => { + const { service } = await bootDefaultPlugin(); + const http = new MockHttp(); + registerSettingsRoutes(http, service, { contextFromRequest: admin }); + const answer = await post(http, 'ai', 'test_embedder', { + values: { embedder_provider: 'openai', embedder_api_key: 'sk-test', embedder_model: 'text-embedding-3-small' }, + }); + expect(answer.status).toBe(200); + expect(answer.body?.data?.ok).toBe(true); + expect(answer.body?.data?.message).toContain('text-embedding-3-small'); + }); + + it('POST /api/settings/knowledge/test is answered by the adapter check (200, not 400)', async () => { + const { service } = await bootDefaultPlugin(); + const http = new MockHttp(); + registerSettingsRoutes(http, service, { contextFromRequest: admin }); + const answer = await post(http, 'knowledge', 'test', { + values: { adapter: 'ragflow', ragflow_base_url: 'http://localhost:9380', ragflow_api_key: 'k' }, + }); + expect(answer.status).toBe(200); + expect(answer.body?.data?.ok).toBe(true); + expect(answer.body?.data?.message).toContain('localhost:9380'); + }); +}); diff --git a/packages/services/service-settings/src/settings-service-plugin.ts b/packages/services/service-settings/src/settings-service-plugin.ts index 958f95af684..716f8a5cda1 100644 --- a/packages/services/service-settings/src/settings-service-plugin.ts +++ b/packages/services/service-settings/src/settings-service-plugin.ts @@ -46,6 +46,8 @@ import { smsTestActionHandler, storageTestActionHandler, aiTestActionHandler, + aiTestEmbedderActionHandler, + knowledgeTestActionHandler, } from './manifests/index.js'; import { settingsBuiltinTranslations } from './translations/index.js'; @@ -79,8 +81,12 @@ export interface SettingsServicePluginOptions { env?: Record; /** * Action handlers to register at boot, keyed by namespace and action - * id. The bundled `mail.test` handler is registered automatically - * unless this object is provided. + * id. When omitted, the bundled handler of every `action_button` the + * builtin manifests declare is registered (`mail.test`, `sms.test`, + * `storage.test`, `ai.test`, `ai.test_embedder`, `knowledge.test`; + * `reset` is served by the service for every namespace). Providing this + * object replaces that whole default map. A plugin may still replace any + * one handler later with `SettingsService.registerAction`. */ actionHandlers?: Record>; } @@ -121,11 +127,16 @@ export class SettingsServicePlugin implements Plugin { this.opts = { ...opts, manifests: opts.manifests ?? builtinSettingsManifests, + // One entry per `action_button` a builtin manifest declares against the + // settings action route — a declared button with no entry here answers + // "No handler registered" (#24769). `settings-default-action-handlers.pin.test.ts` + // enumerates the manifests and holds this map to them. actionHandlers: opts.actionHandlers ?? { mail: { test: mailTestActionHandler }, sms: { test: smsTestActionHandler }, storage: { test: storageTestActionHandler }, - ai: { test: aiTestActionHandler }, + ai: { test: aiTestActionHandler, test_embedder: aiTestEmbedderActionHandler }, + knowledge: { test: knowledgeTestActionHandler }, }, }; }