Skip to content

Commit 7697ddc

Browse files
fix(service-settings): register the ai.test_embedder and knowledge.test handlers by default (#24830)
Fixes #24769 Clause-②: no ## What was wrong Setup's AI "Test embedder" and Knowledge "Test connection" buttons both 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 declare both buttons, and `aiTestEmbedderActionHandler` and `knowledgeTestActionHandler` are exported beside them. But `SettingsServicePlugin`'s default `actionHandlers` map registered only `mail.test`, `sms.test`, `storage.test` and `ai.test`. Nothing registers either action later. At `a43405d59`, `git grep "registerAction("` finds only three registrations against a settings namespace outside this package: plugin-email (`mail`), service-sms (`sms`) and service-storage (`storage`). `@objectstack/service-knowledge`, `@objectstack/knowledge-memory`, `@objectstack/knowledge-ragflow` and `@objectstack/embedder-openai` never reference the settings service. So the PM's mechanism assumption M2 holds on the "no override" half: in this repository the stubs are the only possible handlers. `@objectstack/service-ai` is `edition: 'cloud'` and not in this repository, so whether it overrides `ai/test_embedder` is NOT MEASURED. Landing: `packages/services/service-settings/src/settings-service-plugin.ts`, as the card expected. The defect is in the producer of the default map, not in a consumer. ## The fix 1. **Registration.** The default map now carries `ai: { test, test_embedder }` and `knowledge: { test }`. Override semantics are unchanged (M3, measured): a host's own `actionHandlers` still replaces the whole default map (`opts.actionHandlers ?? DEFAULTS`), and `SettingsService.registerAction` (a `Map.set`) still lets a plugin replace any one handler later. The option's docblock used to say only "the bundled `mail.test` handler is registered automatically". It now names the default set and says that passing the option replaces the whole map. 2. **Answer text (M2's second half).** No answer claimed outright that a connection or round trip was tested. Two answers did promise something false, though: "Mount @objectstack/embedder-openai + a knowledge adapter to exercise live calls" and "Mount @objectstack/knowledge-ragflow to exercise live calls". In this file that phrase follows the storage stub's pattern ("Mount @objectstack/service-storage to exercise live I/O"), and there it is true: service-storage overrides `storage/test` with a live probe. Neither embedder-openai nor knowledge-ragflow overrides anything, so mounting them never made either button live. Both phrases are removed. Every passing answer on a remote backend now says what it is: "This checks the form only — no embedding request was sent." for the embedder, and "This checks the form only — no connection was attempted." for Turso and RAGFlow. Two handler docblocks claimed a live-probe override "in `@objectstack/service-knowledge`". That is false, and they now state what was measured. ## What each button checks after this change Neither button touches the network. Each merges the posted form state (a nested `values` object or a bare key map) over the saved values, then: **AI → "Test embedder"** (`POST /api/settings/ai/test_embedder`) - `embedder_provider` unset or `none`: `ok: false`, warning, "Embedder is disabled. Pick a provider to enable knowledge / RAG." - any provider except `ollama` with no `embedder_api_key`: `ok: false`, error. - `custom` / `azure` with no `embedder_base_url`: `ok: false`, error. - otherwise: `ok: true`, info, "PROVIDER embedder settings are complete (model=MODEL). This checks the form only — no embedding request was sent." **Knowledge → "Test connection"** (`POST /api/settings/knowledge/test`) - `memory`: `ok: true`, warning, "In-memory adapter — no external service to probe. …" (unchanged). - `turso` with no URL: `ok: true`, info, the reuse-the-tenant's-connection note, plus "This checks the form only — no connection was attempted." - `turso` with a `libsql://` URL and no auth token: `ok: false`, error. - `turso` with any other URL: `ok: true`, info, "Turso adapter configured (URL). This checks the form only — no connection was attempted." The existing sentence about `@objectstack/knowledge-turso` follows, unchanged. - `ragflow` with no Base URL or no API key: `ok: false`, error. - `ragflow` complete: `ok: true`, info, "RAGFlow adapter configured (URL). This checks the form only — no connection was attempted." - any other adapter: `ok: false`, error. The route is unchanged: `ok: true` answers 200 with the result, and `ok: false` answers 400 `SETTINGS_ACTION_FAILED` with the result under `error.details`. ## The pin, and the class it closes `src/settings-default-action-handlers.pin.test.ts` reads every builtin manifest's `http`-kind `action_button` whose URL is the settings action route. It constructs the plugin with no options, runs `init()`, and drives each declared action through `runAction` on the service the plugin registered. Every handler the plugin registers is wrapped to record its own invocation. An action counts as honoured when a registered handler ran, or when `runAction` answered `ok` without one (its built-in `reset`, served for every namespace). The no-handler answer is neither. No message text is used as the discriminator. The file holds a floor: the population must contain all seven current pairs. A second assertion checks that each button's route names its own namespace and id. Two HTTP-door cases replay the card's own two requests through `registerSettingsRoutes`. A new button declared without its handler, or a handler dropped from the default map, now goes red by name. ### Red before, green after Run with the test commit `a66a74166` on the unchanged plugin (base `a43405d59`): `Tests 3 failed | 2 passed (5)`. ``` AssertionError: expected [ Array(2) ] to deeply equal [] + "ai/test_embedder", + "knowledge/test", AssertionError: expected 400 to be 200 (POST ai/test_embedder) AssertionError: expected 400 to be 200 (POST knowledge/test) ``` With the fix, on `62b892d56` (the final head, `origin/main` `307ce4c4b` merged in): `pnpm --filter @objectstack/service-settings exec vitest run --maxWorkers=2` reports `Test Files 54 passed (54)`, `Tests 899 passed (899)`. `pnpm --filter @objectstack/service-settings typecheck` exits 0. `tsc --listFiles` includes all three touched test files. ### Ablations (one-off, run at `c8dbf1395`; nothing permanent was added) All three mutations went through `scripts/ablation-replace.mjs` in WRAP mode. Each anchor hit once, the blob changed, and the restore was proven: blob equal to HEAD, and `git diff HEAD` empty. The tests resolve this package's own source through relative imports, and `@objectstack/*` through the vitest source aliases, so no `dist/` is involved. Every ablation went red, the expected direction. - **A: drop `knowledge: { test: knowledgeTestActionHandler }` from the default map.** `Tests 2 failed | 3 passed (5)`. The failures were `expected [ 'knowledge/test' ] to deeply equal []` and, at the HTTP door, `expected 400 to be 200`. Restored blob `716f8a5cda1d` equals HEAD. - **B: append the old "Mount @objectstack/embedder-openai + a knowledge adapter to exercise live calls." to the embedder answer.** `Tests 1 failed | 27 passed (28)`: `expected 'openai embedder settings are complete…' not to match /to exercise live calls/`. Restored blob `bb1ad034d9ad` equals HEAD. - **C: append the old "Mount @objectstack/knowledge-ragflow to exercise live calls." to the RAGFlow answer.** `Tests 1 failed | 24 passed (25)`: `ragflow: expected 'RAGFlow adapter configured (http://lo…' not to match /to exercise live calls/`. Restored blob `396195582b05` equals HEAD. B and C keep the "no … was sent / attempted" sentence and add the old promise, so the negative pin fires on its own. ## Gates (local subset, on `62b892d56`) - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 67 commands from the 7 changed paths, against merge base `307ce4c4b`. Each was run with its exit code captured before any pipe. - 66 exited 0. `node scripts/check-plugin-teardown-shape.mjs --self-test` first exited 3 because the shallow clone could not reach its pinned fixture commit. Once that commit was present it re-ran: exit 0, "48 cases pass". - `pnpm check:dual-build-cjs-loads` exited 3, `PREREQUISITE NOT MET`. It reads every package's `dist/`, and this worktree has built only the service-settings closure. NOT MEASURED locally; CI's `Lint & Repo Gates` runs it. - `--ran` reconciliation: "67 derived famil(ies) accounted for — 66 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)", 0 UNRUN. - Dependency closure built with `turbo run build --filter='@objectstack/service-settings...'` (16 tasks, exit 0). - `test:repo` was not run locally; it belongs to CI (`Test Core`). ## Acceptance notes - **Nothing in this repository reads the settings these buttons check.** That is the `knowledge` namespace and the `ai.embedder_*` keys: the only readers are service-settings' own manifests, tests and visibility evaluator. `@objectstack/knowledge-ragflow` and `@objectstack/embedder-openai` take their configuration from plugin constructor options. Whether a cloud runtime reads these values was not measured. Not filed: this is a read-only inference with no public-door measurement. Carrier: none. - **`aiTestActionHandler`'s docblock is unverified.** It still says the real `ai/test` lives in `@objectstack/service-ai` and overrides the stub on `kernel:ready`. That package is cloud-edition, so this could not be measured here. Left untouched, because `ai.test` was already registered and is not this card's button. - **Compatibility edge.** `init()` registers each default handler through `registerAction`, which throws `UnknownNamespaceError` for an unregistered namespace. A host that passes its own `manifests` without `knowledge`, and does not pass `actionHandlers`, would now fail at boot. The same edge already existed for `mail` / `sms` / `storage` / `ai`. Every in-repo site that passes `manifests` also passes `actionHandlers: {}`; the sites that pass no `manifests` get the builtin set, which includes `knowledge`. - **Two fixture comments name an incomplete handler list.** `config-change-audit.test.ts` and `settings-admission-tenancy-posture.test.ts` describe the bundled handlers as `mail`/`sms`/`storage`/`ai`. The reason those comments give still holds; they are outside this card's file surface and were not edited. - **Two contrast-control tests were replaced.** The embedder test in `ai.manifest.test.ts` and the RAGFlow test in `knowledge.manifest.test.ts` pinned the two removed "Mount … to exercise live calls" sentences, kept because those packages are built here. The packages do exist, but the promise was false for these buttons. The replacements pin the new property, and the AI live-call branches are still each exercised on their own. ## Risk and rollback The change is additive registration plus answer text. Rollback is a revert of `c8dbf1395`: the two buttons go back to answering 400. Changeset: `.changeset/24769-settings-test-handlers.md`, `@objectstack/service-settings` patch. --- _Generated by [Claude Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0c910d5 commit 7697ddc

7 files changed

Lines changed: 309 additions & 44 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/service-settings': patch
3+
---
4+
5+
fix(service-settings): Setup's AI "Test embedder" and Knowledge "Test connection" buttons are answered, and their answers say they checked the form only
6+
7+
Clause-②: no
8+
9+
- **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.
10+
- **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`.
11+
- **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.
12+
- **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.

‎packages/services/service-settings/src/manifests/ai.manifest.test.ts‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -354,18 +354,35 @@ describe('aiTestActionHandler — live-call hint carries the cloud boundary (#11
354354
});
355355
}
356356

357-
// Contrast control, so the assertions above cannot be satisfied by blanket-
358-
// rewording every hint in this file: the embedder hint names
359-
// `@objectstack/embedder-openai`, which IS built in this repo (8 path hits
360-
// under `git ls-tree -r --name-only HEAD | grep -cF /embedder-openai/`, against
361-
// 0 for `/service-ai/`), so that instruction is followable as written and stays
362-
// a plain mount line. Asserted here, deliberately not edited — commit 99ccbb9c8 fences
363-
// this site out by name.
364-
it('leaves the embedder hint a plain mount line — its package IS built here', async () => {
365-
const r = await runTestEmbedder({ embedder_provider: 'openai', embedder_api_key: 'sk-test' });
357+
// The embedder hint used to stand here as this block's contrast control — a
358+
// plain "Mount @objectstack/embedder-openai + a knowledge adapter to exercise
359+
// live calls", kept because that package IS built in this repo. Followable,
360+
// but measured false for the button it answered (#24769): no package in this
361+
// repo registers `ai/test_embedder`, so mounting it never made "Test
362+
// embedder" live. Its replacement is pinned with the embedder handler below;
363+
// the three branches above are still exercised one by one, so a blanket
364+
// rewording cannot stand in for them.
365+
});
366+
367+
/**
368+
* #24769 — "Test embedder" is now answered (the handler is in the plugin's
369+
* default map), and what it answers is a form check: no embedding request is
370+
* sent and no package in this repo replaces it with a live probe. A green
371+
* answer must say so, and must not name a package whose mounting would make the
372+
* check live.
373+
*/
374+
describe('aiTestEmbedderActionHandler — a form check says it is one (#24769)', () => {
375+
it('states that no embedding request was sent, and promises no live call', async () => {
376+
const r = await runTestEmbedder({
377+
embedder_provider: 'openai',
378+
embedder_api_key: 'sk-test',
379+
embedder_model: 'text-embedding-3-small',
380+
});
366381
expect(r.ok).toBe(true);
367-
expect(String(r.message)).toContain(
368-
'Mount @objectstack/embedder-openai + a knowledge adapter to exercise live calls',
369-
);
382+
expect(r.severity).toBe('info');
383+
const message = String(r.message);
384+
expect(message).toContain('text-embedding-3-small');
385+
expect(message).toMatch(/no embedding request was sent/);
386+
expect(message).not.toMatch(/to exercise live calls/);
370387
});
371388
});

‎packages/services/service-settings/src/manifests/ai.manifest.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -427,12 +427,18 @@ export const aiTestActionHandler: SettingsActionHandler = async ({ values, paylo
427427
};
428428

429429
/**
430-
* Built-in fallback handler for `ai/test_embedder`. Real implementation
431-
* with a live `embed()` round-trip lives in `@objectstack/service-ai` or
432-
* `@objectstack/service-knowledge` and overrides this stub at runtime.
430+
* Handler for `ai/test_embedder` ("Test embedder"), registered by
431+
* `SettingsServicePlugin`'s default `actionHandlers` map (#24769).
433432
*
434-
* This fallback validates form completeness only — no network call —
435-
* so the button is useful even when no embedder plugin is mounted.
433+
* It checks form completeness only — a provider is chosen, an API key is
434+
* present (Ollama excepted), a Base URL is present for `custom` / `azure` —
435+
* and sends NO embedding request. Nothing in this repository replaces it with
436+
* a live probe: neither `@objectstack/embedder-openai` nor
437+
* `@objectstack/service-knowledge` registers `ai/test_embedder` (a plugin that
438+
* did would call `SettingsService.registerAction`, which replaces this
439+
* handler). So its success message says what was checked and that no request
440+
* was sent, and names no package whose mounting would make this check live —
441+
* none does.
436442
*/
437443
export const aiTestEmbedderActionHandler: SettingsActionHandler = async ({ values, payload }) => {
438444
const overrides = extractOverrides(payload);
@@ -469,8 +475,8 @@ export const aiTestEmbedderActionHandler: SettingsActionHandler = async ({ value
469475
ok: true,
470476
severity: 'info',
471477
message:
472-
`${provider} embedder configured (model=${model}). ` +
473-
'Mount @objectstack/embedder-openai + a knowledge adapter to exercise live calls.',
478+
`${provider} embedder settings are complete (model=${model}). ` +
479+
'This checks the form only — no embedding request was sent.',
474480
};
475481
};
476482

‎packages/services/service-settings/src/manifests/knowledge.manifest.test.ts‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -145,20 +145,28 @@ describe('knowledgeTestActionHandler', () => {
145145
expect(message).not.toMatch(/Mount @objectstack\/knowledge-turso to exercise live calls/);
146146
});
147147

148-
// Contrast, so the assertion above cannot be satisfied by blanket-rewording every
149-
// hint: ragflow's plugin IS built here, and its hint stays the plain mount line.
150-
it('ragflow live-call hint stays a plain mount line (its plugin is built here)', async () => {
151-
const r = await knowledgeTestActionHandler({
152-
values: {
153-
adapter: 'ragflow',
154-
ragflow_base_url: 'http://localhost:9380',
155-
ragflow_api_key: 'k',
156-
},
157-
} as any);
158-
expect(r.ok).toBe(true);
159-
expect(String(r.message)).toContain(
160-
'Mount @objectstack/knowledge-ragflow to exercise live calls',
161-
);
148+
// #24769 — "Test connection" is now answered (the handler is in the plugin's
149+
// default map), and what it answers is a form check: no package in this repo
150+
// reads this namespace or registers `knowledge/test`, so nothing replaces it
151+
// with a live probe. Every green answer on a remote adapter must say no
152+
// connection was attempted. Ragflow's hint used to be "Mount
153+
// @objectstack/knowledge-ragflow to exercise live calls" — followable (the
154+
// plugin IS built here) but false for this button, which mounting it never
155+
// made live; no answer may carry that promise.
156+
it('every green answer on a remote adapter says no connection was attempted', async () => {
157+
const remote: Array<[string, Record<string, unknown>]> = [
158+
['turso, no URL', { adapter: 'turso' }],
159+
['turso, local file', { adapter: 'turso', turso_url: 'file:./knowledge.db' }],
160+
['turso, managed', { adapter: 'turso', turso_url: 'libsql://x.turso.io', turso_auth_token: 't' }],
161+
['ragflow', { adapter: 'ragflow', ragflow_base_url: 'http://localhost:9380', ragflow_api_key: 'k' }],
162+
];
163+
for (const [label, values] of remote) {
164+
const r = await knowledgeTestActionHandler({ values } as any);
165+
expect(r.ok, label).toBe(true);
166+
const message = String(r.message);
167+
expect(message, label).toMatch(/no connection was attempted/);
168+
expect(message, label).not.toMatch(/to exercise live calls/);
169+
}
162170
});
163171

164172
it('rejects ragflow without base URL', async () => {

‎packages/services/service-settings/src/manifests/knowledge.manifest.ts‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,24 @@ const manifest = {
137137
export const knowledgeSettingsManifest = manifest as unknown as SettingsManifest;
138138

139139
/**
140-
* Built-in fallback handler for `knowledge/test`. The real probe with
141-
* a live `healthCheck()` round-trip lives in `@objectstack/service-knowledge`
142-
* and overrides this stub at runtime (mirrors the AI / storage patterns).
140+
* The sentence every answer on a remote adapter carries, so a green "Test
141+
* connection" is never read as a connection that succeeded.
142+
*/
143+
const FORM_CHECK_ONLY = 'This checks the form only — no connection was attempted.';
144+
145+
/**
146+
* Handler for `knowledge/test` ("Test connection"), registered by
147+
* `SettingsServicePlugin`'s default `actionHandlers` map (#24769).
143148
*
144-
* This fallback only validates form completeness so the button is
145-
* usable when no knowledge adapter plugin is mounted.
149+
* It checks form completeness only — a Turso auth token beside a managed
150+
* `libsql://` URL, a RAGFlow Base URL and API key — and opens NO connection.
151+
* Nothing in this repository replaces it with a live probe:
152+
* `@objectstack/service-knowledge`, `@objectstack/knowledge-memory` and
153+
* `@objectstack/knowledge-ragflow` neither read this namespace nor register
154+
* `knowledge/test` (a plugin that did would call
155+
* `SettingsService.registerAction`, which replaces this handler). So every
156+
* answer on a remote adapter says no connection was attempted, and none names
157+
* a package whose mounting would make this check live — none does.
146158
*/
147159
export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values, payload }) => {
148160
const overrides = extractOverrides(payload);
@@ -163,7 +175,9 @@ export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values
163175
return {
164176
ok: true,
165177
severity: 'info',
166-
message: 'No URL configured — adapter will reuse the tenant\'s primary libSQL connection at runtime.',
178+
message:
179+
'No URL configured — adapter will reuse the tenant\'s primary libSQL connection at runtime. ' +
180+
FORM_CHECK_ONLY,
167181
};
168182
}
169183
const u = String(url);
@@ -178,7 +192,7 @@ export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values
178192
ok: true,
179193
severity: 'info',
180194
message:
181-
`Turso adapter configured (${u}). Live calls need @objectstack/knowledge-turso, ` +
195+
`Turso adapter configured (${u}). ${FORM_CHECK_ONLY} Live calls need @objectstack/knowledge-turso, ` +
182196
'which this platform does not ship: it is built in the ObjectStack Cloud monorepo ' +
183197
'and published on its own version track, so check that it offers a release ' +
184198
'compatible with your platform version before mounting it.',
@@ -195,7 +209,7 @@ export const knowledgeTestActionHandler: SettingsActionHandler = async ({ values
195209
return {
196210
ok: true,
197211
severity: 'info',
198-
message: `RAGFlow adapter configured (${merged.ragflow_base_url}). Mount @objectstack/knowledge-ragflow to exercise live calls.`,
212+
message: `RAGFlow adapter configured (${merged.ragflow_base_url}). ${FORM_CHECK_ONLY}`,
199213
};
200214
}
201215

0 commit comments

Comments
 (0)