From da9011125cac9df348db4990518d66efad8684c1 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 17 Jul 2026 17:03:40 +0800 Subject: [PATCH 1/7] feat: add typed SMS configuration flow --- .../__snapshots__/merge.test.ts.snap | 9 ++ graphql/env/__tests__/merge.test.ts | 114 +++++++++++++++++- graphql/env/src/env.ts | 44 ++++++- graphql/env/src/index.ts | 1 + graphql/env/src/merge.ts | 3 +- graphql/types/src/constructive.ts | 8 +- graphql/types/src/index.ts | 8 ++ graphql/types/src/sms.ts | 31 +++++ 8 files changed, 214 insertions(+), 4 deletions(-) create mode 100644 graphql/types/src/sms.ts diff --git a/graphql/env/__tests__/__snapshots__/merge.test.ts.snap b/graphql/env/__tests__/__snapshots__/merge.test.ts.snap index 3e27125d04..531cd80ff8 100644 --- a/graphql/env/__tests__/__snapshots__/merge.test.ts.snap +++ b/graphql/env/__tests__/__snapshots__/merge.test.ts.snap @@ -120,6 +120,15 @@ exports[`getEnvOptions merges pgpm defaults, graphql defaults, config, env, and "strictAuth": false, "trustProxy": false, }, + "sms": { + "devsms": { + "baseUrl": "http://env-devsms:4000", + }, + "dryRun": true, + "provider": "devsms", + "requestTimeoutMs": 9000, + "senderId": "OverrideSender", + }, "smtp": { "debug": false, "logger": false, diff --git a/graphql/env/__tests__/merge.test.ts b/graphql/env/__tests__/merge.test.ts index edbc3f6df6..b3b0ad6eea 100644 --- a/graphql/env/__tests__/merge.test.ts +++ b/graphql/env/__tests__/merge.test.ts @@ -1,4 +1,5 @@ import { getEnvOptions } from '../src/merge'; +import { getGraphQLEnvVars } from '../src/env'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -37,6 +38,15 @@ describe('getEnvOptions', () => { enableServicesApi: false, isPublic: false, metaSchemas: ['config_meta'] + }, + sms: { + provider: 'devsms', + senderId: 'ConfigSender', + requestTimeoutMs: 3000, + dryRun: false, + devsms: { + baseUrl: 'http://config-devsms:4000' + } } }); @@ -52,7 +62,12 @@ describe('getEnvOptions', () => { API_META_SCHEMAS: 'env_meta1,env_meta2', API_ANON_ROLE: 'env_anon', API_ROLE_NAME: 'env_role', - API_DEFAULT_DATABASE_ID: 'env_db' + API_DEFAULT_DATABASE_ID: 'env_db', + SMS_PROVIDER: 'devsms', + SMS_SENDER_ID: 'EnvSender', + SMS_REQUEST_TIMEOUT_MS: '4000', + SEND_SMS_DRY_RUN: 'true', + DEVSMS_BASE_URL: 'http://env-devsms:4000' }; const result = getEnvOptions( @@ -75,6 +90,10 @@ describe('getEnvOptions', () => { api: { enableServicesApi: false, defaultDatabaseId: 'override_db' + }, + sms: { + senderId: 'OverrideSender', + requestTimeoutMs: 9000 } }, tempDir, @@ -121,4 +140,97 @@ describe('getEnvOptions', () => { expect(result.api?.exposedSchemas).toEqual(['public', 'override_schema']); expect(result.api?.metaSchemas).toEqual(['env_meta', 'override_meta']); }); + + it('parses SMS environment variables into typed options', () => { + const result = getGraphQLEnvVars({ + SMS_PROVIDER: 'devsms', + SMS_SENDER_ID: 'LocalSender', + SMS_REQUEST_TIMEOUT_MS: '2500', + SEND_SMS_DRY_RUN: 'true', + DEVSMS_BASE_URL: 'http://localhost:4000' + }); + + expect(result.sms).toEqual({ + provider: 'devsms', + senderId: 'LocalSender', + requestTimeoutMs: 2500, + dryRun: true, + devsms: { + baseUrl: 'http://localhost:4000' + } + }); + }); + + it('honors defaults, config, env, and runtime override priority for SMS', () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'graphql-env-sms-')); + writeConfig(tempDir, { + sms: { + provider: 'devsms', + senderId: 'ConfigSender', + requestTimeoutMs: 3000, + dryRun: false, + devsms: { + baseUrl: 'http://config-devsms:4000' + } + } + }); + + const result = getEnvOptions( + { + sms: { + requestTimeoutMs: 9000 + } + }, + tempDir, + { + SMS_SENDER_ID: 'EnvSender', + SEND_SMS_DRY_RUN: 'true', + DEVSMS_BASE_URL: 'http://env-devsms:4000' + } + ); + + expect(result.sms).toEqual({ + provider: 'devsms', + senderId: 'EnvSender', + requestTimeoutMs: 9000, + dryRun: true, + devsms: { + baseUrl: 'http://env-devsms:4000' + } + }); + }); + + it('uses the injected env object instead of global process.env for SMS', () => { + const previousSmsProvider = process.env.SMS_PROVIDER; + process.env.SMS_PROVIDER = 'twilio'; + + try { + const result = getEnvOptions({}, process.cwd(), { + SMS_PROVIDER: 'devsms' + }); + + expect(result.sms?.provider).toBe('devsms'); + } finally { + if (previousSmsProvider === undefined) { + delete process.env.SMS_PROVIDER; + } else { + process.env.SMS_PROVIDER = previousSmsProvider; + } + } + }); + + it('keeps SMS provider and DevSms base URL optional while applying defaults', () => { + const result = getEnvOptions({}, process.cwd(), {}); + + expect(result.sms).toEqual({ + requestTimeoutMs: 5000, + dryRun: false + }); + }); + + it('throws on invalid SMS_REQUEST_TIMEOUT_MS values', () => { + expect(() => getGraphQLEnvVars({ + SMS_REQUEST_TIMEOUT_MS: '5s' + })).toThrow('SMS_REQUEST_TIMEOUT_MS must be an integer'); + }); }); diff --git a/graphql/env/src/env.ts b/graphql/env/src/env.ts index 2ea2ad5c71..c96709bf74 100644 --- a/graphql/env/src/env.ts +++ b/graphql/env/src/env.ts @@ -1,6 +1,27 @@ -import { ConstructiveOptions } from '@constructive-io/graphql-types'; +import type { ConstructiveOptions, SmsProviderName } from '@constructive-io/graphql-types'; import { parseEnvBoolean } from '12factor-env'; +const parseEnvInteger = (name: string, val?: string): number | undefined => { + if (val === undefined) return undefined; + const trimmed = val.trim(); + if (!/^\d+$/.test(trimmed)) { + throw new Error(`${name} must be an integer`); + } + const parsed = Number.parseInt(trimmed, 10); + if (!Number.isSafeInteger(parsed) || parsed <= 0) { + throw new Error(`${name} must be a positive safe integer`); + } + return parsed; +}; + +const parseSmsProvider = (val?: string): SmsProviderName | undefined => { + if (val === undefined) return undefined; + if (val === 'devsms' || val === 'twilio' || val === 'sns') { + return val; + } + throw new Error('SMS_PROVIDER must be one of: devsms, twilio, sns'); +}; + /** * @param env - Environment object to read from (defaults to process.env for backwards compatibility) */ @@ -26,6 +47,12 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial CHAT_PROVIDER, CHAT_MODEL, CHAT_BASE_URL, + + SMS_PROVIDER, + SMS_SENDER_ID, + SMS_REQUEST_TIMEOUT_MS, + SEND_SMS_DRY_RUN, + DEVSMS_BASE_URL, } = env; return { @@ -68,5 +95,20 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial }), }, }), + ...((SMS_PROVIDER || SMS_SENDER_ID || SMS_REQUEST_TIMEOUT_MS || SEND_SMS_DRY_RUN || DEVSMS_BASE_URL) && { + sms: { + ...(SMS_PROVIDER && { provider: parseSmsProvider(SMS_PROVIDER) }), + ...(SMS_SENDER_ID && { senderId: SMS_SENDER_ID }), + ...(SMS_REQUEST_TIMEOUT_MS && { + requestTimeoutMs: parseEnvInteger('SMS_REQUEST_TIMEOUT_MS', SMS_REQUEST_TIMEOUT_MS) + }), + ...(SEND_SMS_DRY_RUN && { dryRun: parseEnvBoolean(SEND_SMS_DRY_RUN) }), + ...(DEVSMS_BASE_URL && { + devsms: { + baseUrl: DEVSMS_BASE_URL + } + }), + }, + }), }; }; diff --git a/graphql/env/src/index.ts b/graphql/env/src/index.ts index cfe0e18f11..6a8ef9ec7e 100644 --- a/graphql/env/src/index.ts +++ b/graphql/env/src/index.ts @@ -1,3 +1,4 @@ // Export Constructive-specific env functions export { getEnvOptions, getConstructiveEnvOptions } from './merge'; export { getGraphQLEnvVars } from './env'; +export type { DevSmsOptions, SmsOptions, SmsProviderName } from '@constructive-io/graphql-types'; diff --git a/graphql/env/src/merge.ts b/graphql/env/src/merge.ts index 669c75269f..ef661777cb 100644 --- a/graphql/env/src/merge.ts +++ b/graphql/env/src/merge.ts @@ -30,7 +30,7 @@ export const getEnvOptions = ( const graphqlEnvOptions = getGraphQLEnvVars(env); // Load config again to get any GraphQL-specific config - // Config files can contain Constructive options (graphile, features, api) + // Config files can contain Constructive options (graphile, features, api, sms) // even though loadConfigSync returns PgpmOptions type const configOptions = loadConfigSync(cwd) as Partial; @@ -43,6 +43,7 @@ export const getEnvOptions = ( ...(configOptions.graphile && { graphile: configOptions.graphile }), ...(configOptions.features && { features: configOptions.features }), ...(configOptions.api && { api: configOptions.api }), + ...(configOptions.sms && { sms: configOptions.sms }), }, graphqlEnvOptions, overrides diff --git a/graphql/types/src/constructive.ts b/graphql/types/src/constructive.ts index fc64c547c0..b3d0e5747f 100644 --- a/graphql/types/src/constructive.ts +++ b/graphql/types/src/constructive.ts @@ -19,6 +19,7 @@ import { apiDefaults } from './graphile'; import { LlmOptions } from './llm'; +import { SmsOptions, smsDefaults } from './sms'; /** * GraphQL-specific options for Constructive @@ -30,6 +31,8 @@ export interface ConstructiveGraphQLOptions { features?: GraphileFeatureOptions; /** API configuration options */ api?: ApiOptions; + /** SMS provider configuration */ + sms?: SmsOptions; } /** @@ -59,6 +62,8 @@ export interface ConstructiveOptions extends PgpmOptions, ConstructiveGraphQLOpt jobs?: JobsConfig; /** LLM provider configuration (embeddings, chat, RAG) */ llm?: LlmOptions; + /** SMS provider configuration */ + sms?: SmsOptions; } /** @@ -67,7 +72,8 @@ export interface ConstructiveOptions extends PgpmOptions, ConstructiveGraphQLOpt export const constructiveGraphqlDefaults: ConstructiveGraphQLOptions = { graphile: graphileDefaults, features: graphileFeatureDefaults, - api: apiDefaults + api: apiDefaults, + sms: smsDefaults }; /** diff --git a/graphql/types/src/index.ts b/graphql/types/src/index.ts index a66eb0bf63..8b5647dff7 100644 --- a/graphql/types/src/index.ts +++ b/graphql/types/src/index.ts @@ -29,3 +29,11 @@ export { LlmEmbedderOptions, LlmChatOptions } from './llm'; + +// Export SMS types +export { + SmsProviderName, + SmsOptions, + DevSmsOptions, + smsDefaults +} from './sms'; diff --git a/graphql/types/src/sms.ts b/graphql/types/src/sms.ts new file mode 100644 index 0000000000..90c7833a7b --- /dev/null +++ b/graphql/types/src/sms.ts @@ -0,0 +1,31 @@ +/** + * SMS provider configuration options for Constructive runtimes. + * + * Production providers are intentionally configuration-only here. Runtime + * packages decide which providers they implement and validate that required + * provider-specific values are present before sending. + */ +export type SmsProviderName = 'devsms' | 'twilio' | 'sns'; + +export interface DevSmsOptions { + /** Base URL for the local DevSms API, e.g. http://localhost:4000 */ + baseUrl?: string; +} + +export interface SmsOptions { + /** SMS provider implementation to use. */ + provider?: SmsProviderName; + /** Optional sender ID/default source address for providers that support it. */ + senderId?: string; + /** Outbound provider HTTP timeout in milliseconds. */ + requestTimeoutMs?: number; + /** Validate/render messages without sending them to the provider. */ + dryRun?: boolean; + /** DevSms local provider options. */ + devsms?: DevSmsOptions; +} + +export const smsDefaults: SmsOptions = { + requestTimeoutMs: 5000, + dryRun: false +}; From ca0143f5e4716f998dc7ba248f9e77a2b3cede7e Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 21 Jul 2026 08:55:55 +0800 Subject: [PATCH 2/7] refactor: allow custom SMS provider names --- graphql/env/__tests__/merge.test.ts | 8 ++++++++ graphql/env/src/env.ts | 12 ++---------- graphql/env/src/index.ts | 2 +- graphql/types/src/index.ts | 1 - graphql/types/src/sms.ts | 6 ++---- 5 files changed, 13 insertions(+), 16 deletions(-) diff --git a/graphql/env/__tests__/merge.test.ts b/graphql/env/__tests__/merge.test.ts index b3b0ad6eea..db49bb5f78 100644 --- a/graphql/env/__tests__/merge.test.ts +++ b/graphql/env/__tests__/merge.test.ts @@ -161,6 +161,14 @@ describe('getEnvOptions', () => { }); }); + it('accepts custom SMS provider names', () => { + const result = getGraphQLEnvVars({ + SMS_PROVIDER: 'custom-sms-gateway' + }); + + expect(result.sms?.provider).toBe('custom-sms-gateway'); + }); + it('honors defaults, config, env, and runtime override priority for SMS', () => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'graphql-env-sms-')); writeConfig(tempDir, { diff --git a/graphql/env/src/env.ts b/graphql/env/src/env.ts index c96709bf74..de938ce9ae 100644 --- a/graphql/env/src/env.ts +++ b/graphql/env/src/env.ts @@ -1,4 +1,4 @@ -import type { ConstructiveOptions, SmsProviderName } from '@constructive-io/graphql-types'; +import type { ConstructiveOptions } from '@constructive-io/graphql-types'; import { parseEnvBoolean } from '12factor-env'; const parseEnvInteger = (name: string, val?: string): number | undefined => { @@ -14,14 +14,6 @@ const parseEnvInteger = (name: string, val?: string): number | undefined => { return parsed; }; -const parseSmsProvider = (val?: string): SmsProviderName | undefined => { - if (val === undefined) return undefined; - if (val === 'devsms' || val === 'twilio' || val === 'sns') { - return val; - } - throw new Error('SMS_PROVIDER must be one of: devsms, twilio, sns'); -}; - /** * @param env - Environment object to read from (defaults to process.env for backwards compatibility) */ @@ -97,7 +89,7 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial }), ...((SMS_PROVIDER || SMS_SENDER_ID || SMS_REQUEST_TIMEOUT_MS || SEND_SMS_DRY_RUN || DEVSMS_BASE_URL) && { sms: { - ...(SMS_PROVIDER && { provider: parseSmsProvider(SMS_PROVIDER) }), + ...(SMS_PROVIDER && { provider: SMS_PROVIDER }), ...(SMS_SENDER_ID && { senderId: SMS_SENDER_ID }), ...(SMS_REQUEST_TIMEOUT_MS && { requestTimeoutMs: parseEnvInteger('SMS_REQUEST_TIMEOUT_MS', SMS_REQUEST_TIMEOUT_MS) diff --git a/graphql/env/src/index.ts b/graphql/env/src/index.ts index 6a8ef9ec7e..b199376324 100644 --- a/graphql/env/src/index.ts +++ b/graphql/env/src/index.ts @@ -1,4 +1,4 @@ // Export Constructive-specific env functions export { getEnvOptions, getConstructiveEnvOptions } from './merge'; export { getGraphQLEnvVars } from './env'; -export type { DevSmsOptions, SmsOptions, SmsProviderName } from '@constructive-io/graphql-types'; +export type { DevSmsOptions, SmsOptions } from '@constructive-io/graphql-types'; diff --git a/graphql/types/src/index.ts b/graphql/types/src/index.ts index 8b5647dff7..9b51f15325 100644 --- a/graphql/types/src/index.ts +++ b/graphql/types/src/index.ts @@ -32,7 +32,6 @@ export { // Export SMS types export { - SmsProviderName, SmsOptions, DevSmsOptions, smsDefaults diff --git a/graphql/types/src/sms.ts b/graphql/types/src/sms.ts index 90c7833a7b..18ba95f503 100644 --- a/graphql/types/src/sms.ts +++ b/graphql/types/src/sms.ts @@ -5,16 +5,14 @@ * packages decide which providers they implement and validate that required * provider-specific values are present before sending. */ -export type SmsProviderName = 'devsms' | 'twilio' | 'sns'; - export interface DevSmsOptions { /** Base URL for the local DevSms API, e.g. http://localhost:4000 */ baseUrl?: string; } export interface SmsOptions { - /** SMS provider implementation to use. */ - provider?: SmsProviderName; + /** SMS provider implementation to use; runtimes may register custom names. */ + provider?: string; /** Optional sender ID/default source address for providers that support it. */ senderId?: string; /** Outbound provider HTTP timeout in milliseconds. */ From c8c5d933bcd42fb6b8f8470bbeee9c4bd3817985 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 21 Jul 2026 09:08:10 +0800 Subject: [PATCH 3/7] reuse shared env number parser --- graphql/env/__tests__/merge.test.ts | 8 +++++--- graphql/env/src/env.ts | 17 ++--------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/graphql/env/__tests__/merge.test.ts b/graphql/env/__tests__/merge.test.ts index db49bb5f78..7670e7c395 100644 --- a/graphql/env/__tests__/merge.test.ts +++ b/graphql/env/__tests__/merge.test.ts @@ -236,9 +236,11 @@ describe('getEnvOptions', () => { }); }); - it('throws on invalid SMS_REQUEST_TIMEOUT_MS values', () => { - expect(() => getGraphQLEnvVars({ + it('uses shared number parsing behavior for invalid SMS_REQUEST_TIMEOUT_MS values', () => { + const result = getGraphQLEnvVars({ SMS_REQUEST_TIMEOUT_MS: '5s' - })).toThrow('SMS_REQUEST_TIMEOUT_MS must be an integer'); + }); + + expect(result.sms?.requestTimeoutMs).toBeUndefined(); }); }); diff --git a/graphql/env/src/env.ts b/graphql/env/src/env.ts index de938ce9ae..c2c7c7dd06 100644 --- a/graphql/env/src/env.ts +++ b/graphql/env/src/env.ts @@ -1,18 +1,5 @@ import type { ConstructiveOptions } from '@constructive-io/graphql-types'; -import { parseEnvBoolean } from '12factor-env'; - -const parseEnvInteger = (name: string, val?: string): number | undefined => { - if (val === undefined) return undefined; - const trimmed = val.trim(); - if (!/^\d+$/.test(trimmed)) { - throw new Error(`${name} must be an integer`); - } - const parsed = Number.parseInt(trimmed, 10); - if (!Number.isSafeInteger(parsed) || parsed <= 0) { - throw new Error(`${name} must be a positive safe integer`); - } - return parsed; -}; +import { parseEnvBoolean, parseEnvNumber } from '12factor-env'; /** * @param env - Environment object to read from (defaults to process.env for backwards compatibility) @@ -92,7 +79,7 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial ...(SMS_PROVIDER && { provider: SMS_PROVIDER }), ...(SMS_SENDER_ID && { senderId: SMS_SENDER_ID }), ...(SMS_REQUEST_TIMEOUT_MS && { - requestTimeoutMs: parseEnvInteger('SMS_REQUEST_TIMEOUT_MS', SMS_REQUEST_TIMEOUT_MS) + requestTimeoutMs: parseEnvNumber(SMS_REQUEST_TIMEOUT_MS) }), ...(SEND_SMS_DRY_RUN && { dryRun: parseEnvBoolean(SEND_SMS_DRY_RUN) }), ...(DEVSMS_BASE_URL && { From 57b49dec9698e08c263e900c1deb8ff12c295ef5 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Thu, 23 Jul 2026 11:16:29 +0800 Subject: [PATCH 4/7] refactor: preserve SMS config precedence --- graphql/env/__tests__/merge.test.ts | 113 +++++++++++++++++----------- graphql/env/src/env.ts | 57 ++++++++++---- graphql/types/src/sms.ts | 8 +- 3 files changed, 119 insertions(+), 59 deletions(-) diff --git a/graphql/env/__tests__/merge.test.ts b/graphql/env/__tests__/merge.test.ts index 7670e7c395..2f389c3e17 100644 --- a/graphql/env/__tests__/merge.test.ts +++ b/graphql/env/__tests__/merge.test.ts @@ -5,7 +5,10 @@ import * as os from 'os'; import * as path from 'path'; const writeConfig = (dir: string, config: Record): void => { - fs.writeFileSync(path.join(dir, 'pgpm.json'), JSON.stringify(config, null, 2)); + fs.writeFileSync( + path.join(dir, 'pgpm.json'), + JSON.stringify(config, null, 2) + ); }; describe('getEnvOptions', () => { @@ -23,21 +26,21 @@ describe('getEnvOptions', () => { writeConfig(tempDir, { pg: { host: 'config-host', - database: 'config-db' + database: 'config-db', }, server: { - port: 4000 + port: 4000, }, graphile: { - schema: ['config_schema'] + schema: ['config_schema'], }, features: { - simpleInflection: false + simpleInflection: false, }, api: { enableServicesApi: false, isPublic: false, - metaSchemas: ['config_meta'] + metaSchemas: ['config_meta'], }, sms: { provider: 'devsms', @@ -45,9 +48,9 @@ describe('getEnvOptions', () => { requestTimeoutMs: 3000, dryRun: false, devsms: { - baseUrl: 'http://config-devsms:4000' - } - } + baseUrl: 'http://config-devsms:4000', + }, + }, }); const testEnv: NodeJS.ProcessEnv = { @@ -67,34 +70,34 @@ describe('getEnvOptions', () => { SMS_SENDER_ID: 'EnvSender', SMS_REQUEST_TIMEOUT_MS: '4000', SEND_SMS_DRY_RUN: 'true', - DEVSMS_BASE_URL: 'http://env-devsms:4000' + DEVSMS_BASE_URL: 'http://env-devsms:4000', }; const result = getEnvOptions( { db: { - cwd: '' + cwd: '', }, pg: { - host: 'override-host' + host: 'override-host', }, server: { - port: 5000 + port: 5000, }, graphile: { - schema: ['override_schema'] + schema: ['override_schema'], }, features: { - oppositeBaseNames: false + oppositeBaseNames: false, }, api: { enableServicesApi: false, - defaultDatabaseId: 'override_db' + defaultDatabaseId: 'override_db', }, sms: { senderId: 'OverrideSender', - requestTimeoutMs: 9000 - } + requestTimeoutMs: 9000, + }, }, tempDir, testEnv @@ -107,36 +110,39 @@ describe('getEnvOptions', () => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'graphql-env-replace-')); writeConfig(tempDir, { graphile: { - schema: ['config_schema', 'shared_schema'] + schema: ['config_schema', 'shared_schema'], }, api: { exposedSchemas: ['public', 'shared'], - metaSchemas: ['metaschema_public', 'services_public', 'config_meta'] - } + metaSchemas: ['metaschema_public', 'services_public', 'config_meta'], + }, }); const testEnv: NodeJS.ProcessEnv = { GRAPHILE_SCHEMA: 'shared_schema,env_schema', API_EXPOSED_SCHEMAS: 'shared,env_schema', - API_META_SCHEMAS: 'services_public,env_meta' + API_META_SCHEMAS: 'services_public,env_meta', }; const result = getEnvOptions( { graphile: { - schema: ['override_schema', 'shared_schema'] + schema: ['override_schema', 'shared_schema'], }, api: { exposedSchemas: ['public', 'override_schema'], - metaSchemas: ['env_meta', 'override_meta'] - } + metaSchemas: ['env_meta', 'override_meta'], + }, }, tempDir, testEnv ); // Arrays are replaced, not merged - overrides win completely - expect(result.graphile?.schema).toEqual(['override_schema', 'shared_schema']); + expect(result.graphile?.schema).toEqual([ + 'override_schema', + 'shared_schema', + ]); expect(result.api?.exposedSchemas).toEqual(['public', 'override_schema']); expect(result.api?.metaSchemas).toEqual(['env_meta', 'override_meta']); }); @@ -147,7 +153,7 @@ describe('getEnvOptions', () => { SMS_SENDER_ID: 'LocalSender', SMS_REQUEST_TIMEOUT_MS: '2500', SEND_SMS_DRY_RUN: 'true', - DEVSMS_BASE_URL: 'http://localhost:4000' + DEVSMS_BASE_URL: 'http://localhost:4000', }); expect(result.sms).toEqual({ @@ -156,14 +162,14 @@ describe('getEnvOptions', () => { requestTimeoutMs: 2500, dryRun: true, devsms: { - baseUrl: 'http://localhost:4000' - } + baseUrl: 'http://localhost:4000', + }, }); }); it('accepts custom SMS provider names', () => { const result = getGraphQLEnvVars({ - SMS_PROVIDER: 'custom-sms-gateway' + SMS_PROVIDER: 'custom-sms-gateway', }); expect(result.sms?.provider).toBe('custom-sms-gateway'); @@ -178,22 +184,22 @@ describe('getEnvOptions', () => { requestTimeoutMs: 3000, dryRun: false, devsms: { - baseUrl: 'http://config-devsms:4000' - } - } + baseUrl: 'http://config-devsms:4000', + }, + }, }); const result = getEnvOptions( { sms: { - requestTimeoutMs: 9000 - } + requestTimeoutMs: 9000, + }, }, tempDir, { SMS_SENDER_ID: 'EnvSender', SEND_SMS_DRY_RUN: 'true', - DEVSMS_BASE_URL: 'http://env-devsms:4000' + DEVSMS_BASE_URL: 'http://env-devsms:4000', } ); @@ -203,8 +209,8 @@ describe('getEnvOptions', () => { requestTimeoutMs: 9000, dryRun: true, devsms: { - baseUrl: 'http://env-devsms:4000' - } + baseUrl: 'http://env-devsms:4000', + }, }); }); @@ -214,7 +220,7 @@ describe('getEnvOptions', () => { try { const result = getEnvOptions({}, process.cwd(), { - SMS_PROVIDER: 'devsms' + SMS_PROVIDER: 'devsms', }); expect(result.sms?.provider).toBe('devsms'); @@ -232,15 +238,36 @@ describe('getEnvOptions', () => { expect(result.sms).toEqual({ requestTimeoutMs: 5000, - dryRun: false + dryRun: false, }); }); - it('uses shared number parsing behavior for invalid SMS_REQUEST_TIMEOUT_MS values', () => { + it('omits an invalid SMS timeout from partial env overrides', () => { const result = getGraphQLEnvVars({ - SMS_REQUEST_TIMEOUT_MS: '5s' + SMS_REQUEST_TIMEOUT_MS: '5s', }); - expect(result.sms?.requestTimeoutMs).toBeUndefined(); + expect(result.sms).toBeUndefined(); + }); + + it('does not let absent or invalid SMS env values override config', () => { + tempDir = fs.mkdtempSync( + path.join(os.tmpdir(), 'graphql-env-sms-defaults-') + ); + writeConfig(tempDir, { + sms: { + requestTimeoutMs: 3000, + dryRun: true, + }, + }); + + const result = getEnvOptions({}, tempDir, { + SMS_REQUEST_TIMEOUT_MS: '5s', + }); + + expect(result.sms).toEqual({ + requestTimeoutMs: 3000, + dryRun: true, + }); }); }); diff --git a/graphql/env/src/env.ts b/graphql/env/src/env.ts index c2c7c7dd06..1cf035d746 100644 --- a/graphql/env/src/env.ts +++ b/graphql/env/src/env.ts @@ -4,7 +4,9 @@ import { parseEnvBoolean, parseEnvNumber } from '12factor-env'; /** * @param env - Environment object to read from (defaults to process.env for backwards compatibility) */ -export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial => { +export const getGraphQLEnvVars = ( + env: NodeJS.ProcessEnv = process.env +): Partial => { const { GRAPHILE_SCHEMA, @@ -34,27 +36,52 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial DEVSMS_BASE_URL, } = env; + // Keep this function as a partial env-override parser. Defaults are applied + // before config in constructiveGraphqlDefaults; injecting them here would + // incorrectly let an absent env var overwrite pgpm.json. + const smsRequestTimeoutMs = parseEnvNumber(SMS_REQUEST_TIMEOUT_MS); + const smsDryRun = parseEnvBoolean(SEND_SMS_DRY_RUN); + const hasSmsEnvOverrides = Boolean( + SMS_PROVIDER || + SMS_SENDER_ID || + smsRequestTimeoutMs !== undefined || + smsDryRun !== undefined || + DEVSMS_BASE_URL + ); + return { graphile: { ...(GRAPHILE_SCHEMA && { schema: GRAPHILE_SCHEMA.includes(',') - ? GRAPHILE_SCHEMA.split(',').map(s => s.trim()) - : GRAPHILE_SCHEMA + ? GRAPHILE_SCHEMA.split(',').map((s) => s.trim()) + : GRAPHILE_SCHEMA, }), }, features: { - ...(FEATURES_SIMPLE_INFLECTION && { simpleInflection: parseEnvBoolean(FEATURES_SIMPLE_INFLECTION) }), - ...(FEATURES_OPPOSITE_BASE_NAMES && { oppositeBaseNames: parseEnvBoolean(FEATURES_OPPOSITE_BASE_NAMES) }), + ...(FEATURES_SIMPLE_INFLECTION && { + simpleInflection: parseEnvBoolean(FEATURES_SIMPLE_INFLECTION), + }), + ...(FEATURES_OPPOSITE_BASE_NAMES && { + oppositeBaseNames: parseEnvBoolean(FEATURES_OPPOSITE_BASE_NAMES), + }), ...(FEATURES_POSTGIS && { postgis: parseEnvBoolean(FEATURES_POSTGIS) }), }, api: { - ...(API_ENABLE_SERVICES && { enableServicesApi: parseEnvBoolean(API_ENABLE_SERVICES) }), + ...(API_ENABLE_SERVICES && { + enableServicesApi: parseEnvBoolean(API_ENABLE_SERVICES), + }), ...(API_IS_PUBLIC && { isPublic: parseEnvBoolean(API_IS_PUBLIC) }), - ...(API_EXPOSED_SCHEMAS && { exposedSchemas: API_EXPOSED_SCHEMAS.split(',').map(s => s.trim()) }), - ...(API_META_SCHEMAS && { metaSchemas: API_META_SCHEMAS.split(',').map(s => s.trim()) }), + ...(API_EXPOSED_SCHEMAS && { + exposedSchemas: API_EXPOSED_SCHEMAS.split(',').map((s) => s.trim()), + }), + ...(API_META_SCHEMAS && { + metaSchemas: API_META_SCHEMAS.split(',').map((s) => s.trim()), + }), ...(API_ANON_ROLE && { anonRole: API_ANON_ROLE }), ...(API_ROLE_NAME && { roleName: API_ROLE_NAME }), - ...(API_DEFAULT_DATABASE_ID && { defaultDatabaseId: API_DEFAULT_DATABASE_ID }), + ...(API_DEFAULT_DATABASE_ID && { + defaultDatabaseId: API_DEFAULT_DATABASE_ID, + }), }, ...((EMBEDDER_PROVIDER || CHAT_PROVIDER) && { llm: { @@ -74,18 +101,18 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial }), }, }), - ...((SMS_PROVIDER || SMS_SENDER_ID || SMS_REQUEST_TIMEOUT_MS || SEND_SMS_DRY_RUN || DEVSMS_BASE_URL) && { + ...(hasSmsEnvOverrides && { sms: { ...(SMS_PROVIDER && { provider: SMS_PROVIDER }), ...(SMS_SENDER_ID && { senderId: SMS_SENDER_ID }), - ...(SMS_REQUEST_TIMEOUT_MS && { - requestTimeoutMs: parseEnvNumber(SMS_REQUEST_TIMEOUT_MS) + ...(smsRequestTimeoutMs !== undefined && { + requestTimeoutMs: smsRequestTimeoutMs, }), - ...(SEND_SMS_DRY_RUN && { dryRun: parseEnvBoolean(SEND_SMS_DRY_RUN) }), + ...(smsDryRun !== undefined && { dryRun: smsDryRun }), ...(DEVSMS_BASE_URL && { devsms: { - baseUrl: DEVSMS_BASE_URL - } + baseUrl: DEVSMS_BASE_URL, + }, }), }, }), diff --git a/graphql/types/src/sms.ts b/graphql/types/src/sms.ts index 18ba95f503..5d55780b25 100644 --- a/graphql/types/src/sms.ts +++ b/graphql/types/src/sms.ts @@ -23,7 +23,13 @@ export interface SmsOptions { devsms?: DevSmsOptions; } +/** + * Honest fallbacks for every environment (12factor-env class 1). + * + * These live in the domain-default layer rather than the env parser so config + * files can override them when the corresponding env variables are absent. + */ export const smsDefaults: SmsOptions = { requestTimeoutMs: 5000, - dryRun: false + dryRun: false, }; From 39bf565d11c04ac69c20c3d0aeaaec396cb0d9bb Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 24 Jul 2026 11:08:12 +0800 Subject: [PATCH 5/7] refactor: leave SMS defaults to consumers --- graphql/env/__tests__/merge.test.ts | 9 +++------ graphql/env/src/env.ts | 6 +++--- graphql/types/src/constructive.ts | 5 ++--- graphql/types/src/index.ts | 3 +-- graphql/types/src/sms.ts | 11 ----------- 5 files changed, 9 insertions(+), 25 deletions(-) diff --git a/graphql/env/__tests__/merge.test.ts b/graphql/env/__tests__/merge.test.ts index 2f389c3e17..7ebf2c73e0 100644 --- a/graphql/env/__tests__/merge.test.ts +++ b/graphql/env/__tests__/merge.test.ts @@ -175,7 +175,7 @@ describe('getEnvOptions', () => { expect(result.sms?.provider).toBe('custom-sms-gateway'); }); - it('honors defaults, config, env, and runtime override priority for SMS', () => { + it('honors config, env, and runtime override priority for SMS', () => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'graphql-env-sms-')); writeConfig(tempDir, { sms: { @@ -233,13 +233,10 @@ describe('getEnvOptions', () => { } }); - it('keeps SMS provider and DevSms base URL optional while applying defaults', () => { + it('keeps SMS absent when it is not configured', () => { const result = getEnvOptions({}, process.cwd(), {}); - expect(result.sms).toEqual({ - requestTimeoutMs: 5000, - dryRun: false, - }); + expect(result.sms).toBeUndefined(); }); it('omits an invalid SMS timeout from partial env overrides', () => { diff --git a/graphql/env/src/env.ts b/graphql/env/src/env.ts index 1cf035d746..b2677e239b 100644 --- a/graphql/env/src/env.ts +++ b/graphql/env/src/env.ts @@ -36,9 +36,9 @@ export const getGraphQLEnvVars = ( DEVSMS_BASE_URL, } = env; - // Keep this function as a partial env-override parser. Defaults are applied - // before config in constructiveGraphqlDefaults; injecting them here would - // incorrectly let an absent env var overwrite pgpm.json. + // Keep this function as a partial env-override parser. SMS runtime defaults + // belong to the consuming application; injecting them here would incorrectly + // let an absent env var overwrite pgpm.json or consumer-specific values. const smsRequestTimeoutMs = parseEnvNumber(SMS_REQUEST_TIMEOUT_MS); const smsDryRun = parseEnvBoolean(SEND_SMS_DRY_RUN); const hasSmsEnvOverrides = Boolean( diff --git a/graphql/types/src/constructive.ts b/graphql/types/src/constructive.ts index b3d0e5747f..cd4849407b 100644 --- a/graphql/types/src/constructive.ts +++ b/graphql/types/src/constructive.ts @@ -19,7 +19,7 @@ import { apiDefaults } from './graphile'; import { LlmOptions } from './llm'; -import { SmsOptions, smsDefaults } from './sms'; +import { SmsOptions } from './sms'; /** * GraphQL-specific options for Constructive @@ -72,8 +72,7 @@ export interface ConstructiveOptions extends PgpmOptions, ConstructiveGraphQLOpt export const constructiveGraphqlDefaults: ConstructiveGraphQLOptions = { graphile: graphileDefaults, features: graphileFeatureDefaults, - api: apiDefaults, - sms: smsDefaults + api: apiDefaults }; /** diff --git a/graphql/types/src/index.ts b/graphql/types/src/index.ts index 9b51f15325..c64934e7e0 100644 --- a/graphql/types/src/index.ts +++ b/graphql/types/src/index.ts @@ -33,6 +33,5 @@ export { // Export SMS types export { SmsOptions, - DevSmsOptions, - smsDefaults + DevSmsOptions } from './sms'; diff --git a/graphql/types/src/sms.ts b/graphql/types/src/sms.ts index 5d55780b25..10d34d4bf5 100644 --- a/graphql/types/src/sms.ts +++ b/graphql/types/src/sms.ts @@ -22,14 +22,3 @@ export interface SmsOptions { /** DevSms local provider options. */ devsms?: DevSmsOptions; } - -/** - * Honest fallbacks for every environment (12factor-env class 1). - * - * These live in the domain-default layer rather than the env parser so config - * files can override them when the corresponding env variables are absent. - */ -export const smsDefaults: SmsOptions = { - requestTimeoutMs: 5000, - dryRun: false, -}; From 0f515a08dc9fc992ff1a99af014cdb8136094dd5 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 24 Jul 2026 13:15:59 +0800 Subject: [PATCH 6/7] chore: minimize SMS env review diff --- graphql/env/__tests__/merge.test.ts | 46 +++++++++++++---------------- graphql/env/src/env.ts | 34 +++++++-------------- 2 files changed, 30 insertions(+), 50 deletions(-) diff --git a/graphql/env/__tests__/merge.test.ts b/graphql/env/__tests__/merge.test.ts index 7ebf2c73e0..b6c3bf61a9 100644 --- a/graphql/env/__tests__/merge.test.ts +++ b/graphql/env/__tests__/merge.test.ts @@ -5,10 +5,7 @@ import * as os from 'os'; import * as path from 'path'; const writeConfig = (dir: string, config: Record): void => { - fs.writeFileSync( - path.join(dir, 'pgpm.json'), - JSON.stringify(config, null, 2) - ); + fs.writeFileSync(path.join(dir, 'pgpm.json'), JSON.stringify(config, null, 2)); }; describe('getEnvOptions', () => { @@ -26,21 +23,21 @@ describe('getEnvOptions', () => { writeConfig(tempDir, { pg: { host: 'config-host', - database: 'config-db', + database: 'config-db' }, server: { - port: 4000, + port: 4000 }, graphile: { - schema: ['config_schema'], + schema: ['config_schema'] }, features: { - simpleInflection: false, + simpleInflection: false }, api: { enableServicesApi: false, isPublic: false, - metaSchemas: ['config_meta'], + metaSchemas: ['config_meta'] }, sms: { provider: 'devsms', @@ -76,23 +73,23 @@ describe('getEnvOptions', () => { const result = getEnvOptions( { db: { - cwd: '', + cwd: '' }, pg: { - host: 'override-host', + host: 'override-host' }, server: { - port: 5000, + port: 5000 }, graphile: { - schema: ['override_schema'], + schema: ['override_schema'] }, features: { - oppositeBaseNames: false, + oppositeBaseNames: false }, api: { enableServicesApi: false, - defaultDatabaseId: 'override_db', + defaultDatabaseId: 'override_db' }, sms: { senderId: 'OverrideSender', @@ -110,39 +107,36 @@ describe('getEnvOptions', () => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'graphql-env-replace-')); writeConfig(tempDir, { graphile: { - schema: ['config_schema', 'shared_schema'], + schema: ['config_schema', 'shared_schema'] }, api: { exposedSchemas: ['public', 'shared'], - metaSchemas: ['metaschema_public', 'services_public', 'config_meta'], - }, + metaSchemas: ['metaschema_public', 'services_public', 'config_meta'] + } }); const testEnv: NodeJS.ProcessEnv = { GRAPHILE_SCHEMA: 'shared_schema,env_schema', API_EXPOSED_SCHEMAS: 'shared,env_schema', - API_META_SCHEMAS: 'services_public,env_meta', + API_META_SCHEMAS: 'services_public,env_meta' }; const result = getEnvOptions( { graphile: { - schema: ['override_schema', 'shared_schema'], + schema: ['override_schema', 'shared_schema'] }, api: { exposedSchemas: ['public', 'override_schema'], - metaSchemas: ['env_meta', 'override_meta'], - }, + metaSchemas: ['env_meta', 'override_meta'] + } }, tempDir, testEnv ); // Arrays are replaced, not merged - overrides win completely - expect(result.graphile?.schema).toEqual([ - 'override_schema', - 'shared_schema', - ]); + expect(result.graphile?.schema).toEqual(['override_schema', 'shared_schema']); expect(result.api?.exposedSchemas).toEqual(['public', 'override_schema']); expect(result.api?.metaSchemas).toEqual(['env_meta', 'override_meta']); }); diff --git a/graphql/env/src/env.ts b/graphql/env/src/env.ts index b2677e239b..8640a16c54 100644 --- a/graphql/env/src/env.ts +++ b/graphql/env/src/env.ts @@ -1,12 +1,10 @@ -import type { ConstructiveOptions } from '@constructive-io/graphql-types'; +import { ConstructiveOptions } from '@constructive-io/graphql-types'; import { parseEnvBoolean, parseEnvNumber } from '12factor-env'; /** * @param env - Environment object to read from (defaults to process.env for backwards compatibility) */ -export const getGraphQLEnvVars = ( - env: NodeJS.ProcessEnv = process.env -): Partial => { +export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial => { const { GRAPHILE_SCHEMA, @@ -53,35 +51,23 @@ export const getGraphQLEnvVars = ( graphile: { ...(GRAPHILE_SCHEMA && { schema: GRAPHILE_SCHEMA.includes(',') - ? GRAPHILE_SCHEMA.split(',').map((s) => s.trim()) - : GRAPHILE_SCHEMA, + ? GRAPHILE_SCHEMA.split(',').map(s => s.trim()) + : GRAPHILE_SCHEMA }), }, features: { - ...(FEATURES_SIMPLE_INFLECTION && { - simpleInflection: parseEnvBoolean(FEATURES_SIMPLE_INFLECTION), - }), - ...(FEATURES_OPPOSITE_BASE_NAMES && { - oppositeBaseNames: parseEnvBoolean(FEATURES_OPPOSITE_BASE_NAMES), - }), + ...(FEATURES_SIMPLE_INFLECTION && { simpleInflection: parseEnvBoolean(FEATURES_SIMPLE_INFLECTION) }), + ...(FEATURES_OPPOSITE_BASE_NAMES && { oppositeBaseNames: parseEnvBoolean(FEATURES_OPPOSITE_BASE_NAMES) }), ...(FEATURES_POSTGIS && { postgis: parseEnvBoolean(FEATURES_POSTGIS) }), }, api: { - ...(API_ENABLE_SERVICES && { - enableServicesApi: parseEnvBoolean(API_ENABLE_SERVICES), - }), + ...(API_ENABLE_SERVICES && { enableServicesApi: parseEnvBoolean(API_ENABLE_SERVICES) }), ...(API_IS_PUBLIC && { isPublic: parseEnvBoolean(API_IS_PUBLIC) }), - ...(API_EXPOSED_SCHEMAS && { - exposedSchemas: API_EXPOSED_SCHEMAS.split(',').map((s) => s.trim()), - }), - ...(API_META_SCHEMAS && { - metaSchemas: API_META_SCHEMAS.split(',').map((s) => s.trim()), - }), + ...(API_EXPOSED_SCHEMAS && { exposedSchemas: API_EXPOSED_SCHEMAS.split(',').map(s => s.trim()) }), + ...(API_META_SCHEMAS && { metaSchemas: API_META_SCHEMAS.split(',').map(s => s.trim()) }), ...(API_ANON_ROLE && { anonRole: API_ANON_ROLE }), ...(API_ROLE_NAME && { roleName: API_ROLE_NAME }), - ...(API_DEFAULT_DATABASE_ID && { - defaultDatabaseId: API_DEFAULT_DATABASE_ID, - }), + ...(API_DEFAULT_DATABASE_ID && { defaultDatabaseId: API_DEFAULT_DATABASE_ID }), }, ...((EMBEDDER_PROVIDER || CHAT_PROVIDER) && { llm: { From cf75880112fb8498e6612afc04248ed617e9b27e Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 24 Jul 2026 13:23:12 +0800 Subject: [PATCH 7/7] refactor: avoid duplicate SMS option declaration --- graphql/types/src/constructive.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/graphql/types/src/constructive.ts b/graphql/types/src/constructive.ts index cd4849407b..708969413e 100644 --- a/graphql/types/src/constructive.ts +++ b/graphql/types/src/constructive.ts @@ -31,8 +31,6 @@ export interface ConstructiveGraphQLOptions { features?: GraphileFeatureOptions; /** API configuration options */ api?: ApiOptions; - /** SMS provider configuration */ - sms?: SmsOptions; } /**