diff --git a/.changeset/service-settings-typecheck-gate.md b/.changeset/service-settings-typecheck-gate.md new file mode 100644 index 0000000000..b10671d73a --- /dev/null +++ b/.changeset/service-settings-typecheck-gate.md @@ -0,0 +1,47 @@ +--- +"@objectstack/service-settings": patch +--- + +fix(service-settings): wire the `typecheck` script so turbo stops silently no-opping the gate, and clear the 14 type errors it was hiding (#7925) + +`packages/services/service-settings/package.json` declared only `build` and +`test`. `turbo run typecheck --filter=@objectstack/service-settings` therefore +exited **0 while never running a typecheck task at all** — turbo no-ops a +package that has no such script, and reports success. "typecheck green" for +this package was a claim nothing enforced. + +Adding the one-line script (mirroring its sibling `service-messaging`) makes +the CI-equivalent command execute a real `service-settings:typecheck` task — +7 tasks where there were 6 — and it immediately surfaced 14 pre-existing +errors across five test files. Every one was a **stale test**, not a defect in +the source; no `service-settings/src/*.ts` non-test file changed. + +- `sms.manifest.test.ts` (3), `storage.manifest.test.ts` (4), and + `ai.manifest.test.ts` (5, previously behind `as any`) invoked their action + handlers with a partial input. `SettingsActionHandler` takes + `{ namespace, actionId, values, payload?, ctx }` and the service always + passes all of it (`settings-service.ts:1809`); the tests had drifted to the + older two-field shape. They now call handlers the way the service does — and + the `as any` casts that were hiding the same drift in the `ai` tests are + gone rather than extended to the other two files. +- `settings-service.test.ts` passed `record: (e) => events.push(e)` for an + audit sink declared `Promise | void`; the expression-bodied arrow + returned `Array.push`'s number. +- `settings-translation-coverage.test.ts` filtered the manifests barrel + through a hand-rolled structural `Manifest` type that had drifted from the + real one (`label` is `string | Record`, not `string`), + making its type predicate unassignable to the exports it narrowed. It now + narrows to the spec's own `SettingsManifest`. + +`aiTestEmbedderActionHandler` was imported by `ai.manifest.test.ts` and never +used — the unused import the compiler flagged. Rather than delete the import, +the two cases it was there for are now tested: the manifest declares a +`test_embedder` action button whose handler had no coverage. + +No error was silenced: no `any` was added, no `@ts-expect-error`, and the +package tsconfig's `include` is unchanged (its tests live under `src`, so they +were always inside the program — only the script that reads them was missing). + +The package's entry in the `check:type-check-coverage` DEBT ledger is deleted, +since it graduated: the gate goes from 63/77 workspace packages type-checked to +64/77, and the frozen raw-error total from 455 to 442. diff --git a/packages/services/service-settings/package.json b/packages/services/service-settings/package.json index 0982310e42..bd336a4538 100644 --- a/packages/services/service-settings/package.json +++ b/packages/services/service-settings/package.json @@ -15,6 +15,7 @@ }, "scripts": { "build": "tsup --config ../../../tsup.config.ts", + "typecheck": "tsc --noEmit", "test": "vitest run" }, "dependencies": { 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 1b619afbab..5118f212e4 100644 --- a/packages/services/service-settings/src/manifests/ai.manifest.test.ts +++ b/packages/services/service-settings/src/manifests/ai.manifest.test.ts @@ -57,41 +57,62 @@ describe('aiSettingsManifest', () => { }); }); +/** Mirror how the service invokes a handler: full input, never a partial. */ +const runTest = (values: Record) => + aiTestActionHandler({ namespace: 'ai', actionId: 'test', values, ctx: {} }); +const runTestEmbedder = (values: Record) => + aiTestEmbedderActionHandler({ namespace: 'ai', actionId: 'test_embedder', values, ctx: {} }); + describe('aiTestActionHandler', () => { it('returns warning for memory provider (no external call to validate)', async () => { - const r = await aiTestActionHandler({ values: { provider: 'memory' } } as any); + const r = await runTest({ provider: 'memory' }); expect(r.ok).toBe(true); expect(r.severity).toBe('warning'); }); it('rejects gateway provider without gateway_model', async () => { - const r = await aiTestActionHandler({ values: { provider: 'gateway' } } as any); + const r = await runTest({ provider: 'gateway' }); expect(r.ok).toBe(false); expect(r.severity).toBe('error'); }); it('rejects openai provider without api key', async () => { - const r = await aiTestActionHandler({ values: { provider: 'openai' } } as any); + const r = await runTest({ provider: 'openai' }); expect(r.ok).toBe(false); expect(r.severity).toBe('error'); }); it('accepts openai provider with api key and reports the model', async () => { - const r = await aiTestActionHandler({ - values: { provider: 'openai', openai_api_key: 'sk-test', openai_model: 'gpt-4o' }, - } as any); + const r = await runTest({ + provider: 'openai', + openai_api_key: 'sk-test', + openai_model: 'gpt-4o', + }); expect(r.ok).toBe(true); expect(r.message).toContain('gpt-4o'); }); it('accepts anthropic with api key', async () => { - const r = await aiTestActionHandler({ - values: { provider: 'anthropic', anthropic_api_key: 'sk-ant-test' }, - } as any); + const r = await runTest({ provider: 'anthropic', anthropic_api_key: 'sk-ant-test' }); expect(r.ok).toBe(true); }); }); +describe('aiTestEmbedderActionHandler', () => { + it('warns when the embedder is disabled', async () => { + const r = await runTestEmbedder({ embedder_provider: 'none' }); + expect(r.ok).toBe(false); + expect(r.severity).toBe('warning'); + }); + + it('requires an api key for non-ollama providers', async () => { + const r = await runTestEmbedder({ embedder_provider: 'openai' }); + expect(r.ok).toBe(false); + expect(r.severity).toBe('error'); + expect(r.message).toContain('API key'); + }); +}); + /** * #6550 — `temperature` declares a window, not a grid. Since #6199 a declared * `step` binds as a value constraint on both doors, and temperature's true diff --git a/packages/services/service-settings/src/manifests/sms.manifest.test.ts b/packages/services/service-settings/src/manifests/sms.manifest.test.ts index ea06981607..612fc14aab 100644 --- a/packages/services/service-settings/src/manifests/sms.manifest.test.ts +++ b/packages/services/service-settings/src/manifests/sms.manifest.test.ts @@ -44,20 +44,32 @@ describe('sms settings manifest', () => { describe('smsTestActionHandler (fallback)', () => { it('accepts the log provider', async () => { - const r = await smsTestActionHandler({ values: { provider: 'log' }, ctx: {} as any }); + const r = await smsTestActionHandler({ + namespace: 'sms', + actionId: 'test', + values: { provider: 'log' }, + ctx: {}, + }); expect(r.ok).toBe(true); }); it('requires aliyun credentials', async () => { - const r = await smsTestActionHandler({ values: { provider: 'aliyun' }, ctx: {} as any }); + const r = await smsTestActionHandler({ + namespace: 'sms', + actionId: 'test', + values: { provider: 'aliyun' }, + ctx: {}, + }); expect(r.ok).toBe(false); expect(r.message).toMatch(/AccessKey/); }); it('requires a twilio sender', async () => { const r = await smsTestActionHandler({ + namespace: 'sms', + actionId: 'test', values: { provider: 'twilio', twilio_account_sid: 'AC1', twilio_auth_token: 't' }, - ctx: {} as any, + ctx: {}, }); expect(r.ok).toBe(false); expect(r.message).toMatch(/From number|Messaging Service/); diff --git a/packages/services/service-settings/src/manifests/storage.manifest.test.ts b/packages/services/service-settings/src/manifests/storage.manifest.test.ts index 23ee1602a5..cdf58c8979 100644 --- a/packages/services/service-settings/src/manifests/storage.manifest.test.ts +++ b/packages/services/service-settings/src/manifests/storage.manifest.test.ts @@ -52,23 +52,32 @@ describe('storageSettingsManifest', () => { describe('storageTestActionHandler (fallback)', () => { it('rejects local adapter without local_root', async () => { - const r = await storageTestActionHandler({ values: { adapter: 'local' }, ctx: {} as any }); + const r = await storageTestActionHandler({ + namespace: 'storage', + actionId: 'test', + values: { adapter: 'local' }, + ctx: {}, + }); expect(r.ok).toBe(false); expect(r.severity).toBe('error'); }); it('accepts local adapter when local_root is set', async () => { const r = await storageTestActionHandler({ + namespace: 'storage', + actionId: 'test', values: { adapter: 'local', local_root: './uploads' }, - ctx: {} as any, + ctx: {}, }); expect(r.ok).toBe(true); }); it('rejects s3 adapter when credentials are missing', async () => { const r = await storageTestActionHandler({ + namespace: 'storage', + actionId: 'test', values: { adapter: 's3', s3_bucket: 'x' }, - ctx: {} as any, + ctx: {}, }); expect(r.ok).toBe(false); expect(r.message).toMatch(/s3_region|s3_access_key_id|s3_secret_access_key/); @@ -76,6 +85,8 @@ describe('storageTestActionHandler (fallback)', () => { it('accepts a fully-specified s3 config', async () => { const r = await storageTestActionHandler({ + namespace: 'storage', + actionId: 'test', values: { adapter: 's3', s3_bucket: 'b', @@ -83,7 +94,7 @@ describe('storageTestActionHandler (fallback)', () => { s3_access_key_id: 'A', s3_secret_access_key: 'S', }, - ctx: {} as any, + ctx: {}, }); expect(r.ok).toBe(true); }); diff --git a/packages/services/service-settings/src/settings-service.test.ts b/packages/services/service-settings/src/settings-service.test.ts index 651f7328c7..0aa45ed325 100644 --- a/packages/services/service-settings/src/settings-service.test.ts +++ b/packages/services/service-settings/src/settings-service.test.ts @@ -140,7 +140,11 @@ describe('SettingsService — audit sink', () => { const events: any[] = []; const svc = new SettingsService({ env: {}, - audit: { record: (e) => events.push(e) }, + audit: { + record: (e) => { + events.push(e); + }, + }, }); svc.registerManifest(mailSettingsManifest); await svc.setMany('mail', { provider: 'resend', api_key: 'top-secret', from_email: 'a@b.com' }); diff --git a/packages/services/service-settings/src/translations/settings-translation-coverage.test.ts b/packages/services/service-settings/src/translations/settings-translation-coverage.test.ts index d5a7f20188..6c127cb364 100644 --- a/packages/services/service-settings/src/translations/settings-translation-coverage.test.ts +++ b/packages/services/service-settings/src/translations/settings-translation-coverage.test.ts @@ -18,15 +18,18 @@ */ import { describe, it, expect } from 'vitest'; +import type { SettingsManifest } from '@objectstack/spec/system'; import * as manifestsModule from '../manifests/index.js'; import { zhCN, jaJP, esES } from './index.js'; -type Specifier = { type?: string; id?: string; key?: string; label?: string; description?: string }; -type Manifest = { namespace: string; description?: string; specifiers: Specifier[] }; - +// The manifests barrel also exports action handlers and the aggregate array; +// keep only the manifest objects. const manifests = Object.values(manifestsModule).filter( - (v): v is Manifest => - !!v && typeof v === 'object' && 'namespace' in v && Array.isArray((v as Manifest).specifiers), + (v): v is SettingsManifest => + !!v && + typeof v === 'object' && + 'namespace' in v && + Array.isArray((v as SettingsManifest).specifiers), ); const LOCALES: Array<[string, { settings?: Record }]> = [ @@ -35,7 +38,7 @@ const LOCALES: Array<[string, { settings?: Record }]> = [ ['es-ES', esES], ]; -function missingFor(data: { settings?: Record }, m: Manifest): string[] { +function missingFor(data: { settings?: Record }, m: SettingsManifest): string[] { const tr = (data.settings ?? {})[m.namespace]; const missing: string[] = []; if (tr?.title == null) missing.push('title'); diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index d5bec28680..7970e7f6ed 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -395,10 +395,6 @@ const DEBT = { + '5ab08428, up from 8; code-tier is unchanged at 3, so the +2 is config-tier/noise. 8 of the 10 are ' + 'in __tests__/knowledge-service.test.ts.', }, - '@objectstack/service-settings': { - errors: 13, - note: 'code-tier 12 (TS2345 x7: manifest action handlers called without `namespace`/`actionId`; TS2322) + 1 noise. Was ledgered at 44 with "no code-tier finding" -- wrong in both directions: 31 of those 44 were unresolved imports (see the NodeNext note at the top of this ledger), and the resolution they were blocking is what made the 12 real ones visible.', - }, '@objectstack/service-storage': { errors: 52, note: 'code-tier 8 (TS2339 x4, TS2347 x4); config-tier 21 (TS2835); noise 13 (TS7006 x11, TS6196, '