Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions .changeset/service-settings-typecheck-gate.md
Original file line numberDiff line numberDiff line change
@@ -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> | 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<string, string>`, 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.
1 change: 1 addition & 0 deletions packages/services/service-settings/package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,7 @@
},
"scripts": {
"build": "tsup --config ../../../tsup.config.ts",
"typecheck": "tsc --noEmit",
"test": "vitest run"
},
"dependencies": {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -57,41 +57,62 @@ describe('aiSettingsManifest', () => {
});
});

/** Mirror how the service invokes a handler: full input, never a partial. */
const runTest = (values: Record<string, unknown>) =>
aiTestActionHandler({ namespace: 'ai', actionId: 'test', values, ctx: {} });
const runTestEmbedder = (values: Record<string, unknown>) =>
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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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/);
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -52,38 +52,49 @@ 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/);
});

it('accepts a fully-specified s3 config', async () => {
const r = await storageTestActionHandler({
namespace: 'storage',
actionId: 'test',
values: {
adapter: 's3',
s3_bucket: 'b',
s3_region: 'us-east-1',
s3_access_key_id: 'A',
s3_secret_access_key: 'S',
},
ctx: {} as any,
ctx: {},
});
expect(r.ok).toBe(true);
});
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' });
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<string, any> }]> = [
Expand All@@ -35,7 +38,7 @@ const LOCALES: Array<[string, { settings?: Record<string, any> }]> = [
['es-ES', esES],
];

function missingFor(data: { settings?: Record<string, any> }, m: Manifest): string[] {
function missingFor(data: { settings?: Record<string, any> }, m: SettingsManifest): string[] {
const tr = (data.settings ?? {})[m.namespace];
const missing: string[] = [];
if (tr?.title == null) missing.push('title');
Expand Down
4 changes: 0 additions & 4 deletions scripts/check-type-check-coverage.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -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, '
Expand Down
Loading