From ab46bd268b258bedab9eae9ced6e491660531018 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 17 Jul 2026 17:03:40 +0800 Subject: [PATCH 01/12] feat: add SMS verification code flow --- CLAUDE.md | 9 +- DEVELOPMENT.md | 32 ++- docker-compose.yml | 6 + functions/send-sms/README.md | 19 ++ functions/send-sms/__tests__/devsms.test.ts | 134 ++++++++++++ functions/send-sms/__tests__/handler.test.ts | 196 ++++++++++++++++++ functions/send-sms/__tests__/phone.test.ts | 25 +++ functions/send-sms/config.ts | 25 +++ functions/send-sms/handler.json | 12 ++ functions/send-sms/handler.ts | 141 +++++++++++++ functions/send-sms/phone.ts | 86 ++++++++ functions/send-sms/providers/devsms.ts | 103 +++++++++ functions/send-sms/providers/factory.ts | 50 +++++ functions/send-sms/providers/types.ts | 22 ++ functions/send-sms/templates.ts | 9 + job/service/package.json | 1 + .../constructive/knative-job-service.yaml | 4 +- k8s/base/functions/send-sms.yaml | 67 ++++++ k8s/base/kustomization.yaml | 1 + k8s/overlays/local-simple/config.yaml | 4 + k8s/overlays/local-simple/devsms-local.yaml | 42 ++++ k8s/overlays/local-simple/kustomization.yaml | 1 + k8s/overlays/local/constructive/config.yaml | 3 + k8s/overlays/local/devsms-local.yaml | 42 ++++ k8s/overlays/local/kustomization.yaml | 35 ++++ package.json | 1 + pnpm-lock.yaml | 97 +++++++++ scripts/dev.ts | 3 + scripts/generate.ts | 48 ++++- skaffold.yaml | 55 +++++ 30 files changed, 1255 insertions(+), 18 deletions(-) create mode 100644 functions/send-sms/README.md create mode 100644 functions/send-sms/__tests__/devsms.test.ts create mode 100644 functions/send-sms/__tests__/handler.test.ts create mode 100644 functions/send-sms/__tests__/phone.test.ts create mode 100644 functions/send-sms/config.ts create mode 100644 functions/send-sms/handler.json create mode 100644 functions/send-sms/handler.ts create mode 100644 functions/send-sms/phone.ts create mode 100644 functions/send-sms/providers/devsms.ts create mode 100644 functions/send-sms/providers/factory.ts create mode 100644 functions/send-sms/providers/types.ts create mode 100644 functions/send-sms/templates.ts create mode 100644 k8s/base/functions/send-sms.yaml create mode 100644 k8s/overlays/local-simple/devsms-local.yaml create mode 100644 k8s/overlays/local/devsms-local.yaml diff --git a/CLAUDE.md b/CLAUDE.md index 8ab94dc9d..21974314f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,6 +1,6 @@ # Constructive Functions -Serverless function workloads (send-email, send-verification-link) with a job queue system deployed via Kubernetes. +Serverless function workloads (send-email, send-verification-link, send-sms) with a job queue system deployed via Kubernetes. ## Project Structure @@ -93,6 +93,9 @@ Edit `functions//handler.ts` → Skaffold syncs the file into the containe | Job Service | 8080 | | send-email | 8081 | | send-verification-link | 8082 | +| send-sms | 8086 | +| DevSms API | 4000 | +| DevSms UI | 5153 | ## Debugging K8s Pods @@ -113,6 +116,7 @@ kubectl logs -n constructive-functions -l app=knative-job-service -f # Function logs kubectl logs -n constructive-functions -l app=send-email -f kubectl logs -n constructive-functions -l app=send-verification-link -f +kubectl logs -n constructive-functions -l app=send-sms -f # Constructive server logs kubectl logs -n constructive-functions -l app=constructive-server -f @@ -135,6 +139,9 @@ kubectl port-forward -n constructive-functions svc/postgres 5432:5432 kubectl port-forward -n constructive-functions svc/knative-job-service 8080:8080 kubectl port-forward -n constructive-functions svc/send-email 8081:80 kubectl port-forward -n constructive-functions svc/send-verification-link 8082:80 +kubectl port-forward -n constructive-functions svc/send-sms 8086:80 +kubectl port-forward -n constructive-functions svc/devsms 4000:4000 +kubectl port-forward -n constructive-functions svc/devsms 5153:5153 kubectl port-forward -n constructive-functions svc/constructive-server 3002:3000 ``` diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 90db15ba1..149684e85 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -1,6 +1,6 @@ # Development Guide -Local development setup for running functions against real infrastructure (Postgres, GraphQL, Mailpit). +Local development setup for running functions against real infrastructure (Postgres, GraphQL, Mailpit, DevSms). ## Prerequisites @@ -32,7 +32,7 @@ pnpm install # 3. Build everything (packages, job service, generated functions) pnpm build -# 4. Start infrastructure (Postgres, DB migrations, GraphQL server, Mailpit) +# 4. Start infrastructure (Postgres, DB migrations, GraphQL server, Mailpit, DevSms) make dev # 5. Wait for db-setup to finish (watch logs) @@ -60,6 +60,7 @@ After this you should have built artifacts in: |---------|--------| | `generated/send-verification-link/dist/` | Send-verification-link function server | | `generated/send-email/dist/` | Send-email function server | +| `generated/send-sms/dist/` | Send SMS verification code function server | | `generated/example/dist/` | knative-job-example function server | | `generated/python-example/dist/` | Python example function server | | `job/service/dist/` | Knative job service (worker + scheduler) | @@ -80,6 +81,7 @@ This runs `docker compose up -d` which starts: | **db-setup** | One-shot: creates DB, bootstraps roles, deploys pgpm packages | (exits on completion) | | **graphql-server** | Constructive admin GraphQL API (header-based routing) | 3002 | | **mailpit** | SMTP capture server with web UI | 1025 (SMTP), 8025 (UI) | +| **devsms** | Local SMS inbox/API for development verification codes | 4000 (API), 5153 (UI) | The `db-setup` container must finish before `graphql-server` starts (enforced by `service_completed_successfully`). Watch progress: @@ -100,6 +102,7 @@ You should see: - `db-setup` — exited (0) - `graphql-server` — running - `mailpit` — running +- `devsms` — running ### 3. Start Functions Locally @@ -114,6 +117,7 @@ This runs `scripts/dev.ts` which spawns local Node processes with env vars point | **job-service** | 8080 | `job/service/dist/run.js` | | **send-email** | 8081 | `generated/send-email/dist/index.js` | | **send-verification-link** | 8082 | `generated/send-verification-link/dist/index.js` | +| **send-sms** | 8086 | `generated/send-sms/dist/index.js` | | **knative-job-example** | 8083 | `generated/example/dist/index.js` | | **python-example** | 8084 | `generated/python-example/...` (python entrypoint) | @@ -136,6 +140,21 @@ curl -X POST http://localhost:8082 \ Check captured emails at http://localhost:8025 (Mailpit UI). +Send a request to `send-sms` and check captured SMS at http://localhost:5153 (DevSms UI): + +```bash +curl -X POST http://localhost:8086 \ + -H 'Content-Type: application/json' \ + -H 'X-Database-Id: constructive' \ + -d '{"sms_type":"sms_otp_code","phone":"+14155550123","code":"012345"}' +``` + +Query DevSms messages through its API: + +```bash +curl "http://localhost:4000/api/sms?limit=10" +``` + Query the GraphQL API directly: ```bash @@ -175,9 +194,12 @@ make dev-down # Stop Docker infrastructure | GraphQL API | 3002 | | Mailpit SMTP | 1025 | | Mailpit UI | 8025 | +| DevSms API | 4000 | +| DevSms UI | 5153 | | Job Service | 8080 | | send-email | 8081 | | send-verification-link | 8082 | +| send-sms | 8086 | | knative-job-example | 8083 | | python-example | 8084 | @@ -187,11 +209,13 @@ make dev-down # Stop Docker infrastructure Docker Compose (infrastructure): postgres -> db-setup (migrations) -> graphql-server mailpit + devsms Local Node processes (functions): job/service/dist/run.js (port 8080) - generated/send-email/dist/index.js (port 8081) - generated/send-verification-link/dist/index.js (port 8082) + generated/send-email/dist/index.js (port 8081) + generated/send-verification-link/dist/index.js (port 8082) + generated/send-sms/dist/index.js (port 8086) ``` Infrastructure runs in Docker. Functions run as local Node processes from `generated/` — no Docker rebuild needed when function code changes. Edit `functions/*/handler.ts`, rebuild (`pnpm build`), restart `make dev-fn`. diff --git a/docker-compose.yml b/docker-compose.yml index 0717362c4..ef72718dc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -93,5 +93,11 @@ services: - "1025:1025" # SMTP - "8025:8025" # Web UI + devsms: + image: ghcr.io/mrmeaow/devsms:latest + ports: + - "4000:4000" # API + - "5153:5153" # Web UI + volumes: pgdata: diff --git a/functions/send-sms/README.md b/functions/send-sms/README.md new file mode 100644 index 000000000..71392e47d --- /dev/null +++ b/functions/send-sms/README.md @@ -0,0 +1,19 @@ +# send-sms + +Handles `sms:send_verification_code` jobs by validating the job payload, normalizing the recipient phone number to E.164, rendering the verification SMS body, and sending it through the configured SMS provider. + +Current provider support is intentionally local-only: + +- `SMS_PROVIDER=devsms` +- `DEVSMS_BASE_URL=http://localhost:4000` for Docker Compose local development +- DevSms endpoint: `POST /api/sms/send/twilio` + +All SMS configuration is loaded through `@constructive-io/graphql-env` via `getEnvOptions({}, process.cwd(), context.env)`. The handler must not read `SMS_*` or `DEVSMS_*` values directly. + +## Retry and idempotency + +The job worker may retry a job after a timeout or provider error. The `SmsSendRequest.metadata` includes `jobId` and `databaseId` for future idempotency support, but DevSms does not currently expose an idempotency key. A retried job can therefore create duplicate local SMS messages. The handler intentionally does not implement its own retry loop; timeout and transient failures are left to the existing job retry mechanism. + +## Logging + +Logs include job metadata, SMS type, provider, provider message ID, status, and a masked phone number only. They must not include the OTP code, full SMS body, full phone number, or provider secrets. diff --git a/functions/send-sms/__tests__/devsms.test.ts b/functions/send-sms/__tests__/devsms.test.ts new file mode 100644 index 000000000..62005f1d0 --- /dev/null +++ b/functions/send-sms/__tests__/devsms.test.ts @@ -0,0 +1,134 @@ +import { DevSmsProvider } from '../providers/devsms'; +import type { SmsSendRequest } from '../providers/types'; + +const request: SmsSendRequest = { + to: '+14155550123', + body: 'Your sign-in code is 123456. Do not share this code.', + senderId: 'TestSender', + metadata: { + jobId: 'job-1', + databaseId: 'db-1', + purpose: 'sign_in_otp' + } +}; + +const jsonResponse = (body: unknown, status = 201): Response => + new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' } + }); + +describe('DevSmsProvider', () => { + it('sends the correct URL, method, headers, and request body', async () => { + const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ + id: 'row_1', + provider_message_id: 'SM123', + status: 'queued' + })); + const provider = new DevSmsProvider({ + baseUrl: 'http://devsms:4000', + requestTimeoutMs: 5000, + fetchImpl: fetchImpl as unknown as typeof fetch + }); + + await provider.send(request); + + expect(fetchImpl).toHaveBeenCalledWith( + 'http://devsms:4000/api/sms/send/twilio', + expect.objectContaining({ + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + From: 'TestSender', + To: '+14155550123', + Body: 'Your sign-in code is 123456. Do not share this code.' + }) + }) + ); + }); + + it('maps provider responses to SmsSendResult', async () => { + const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ + id: 'row_1', + provider_message_id: 'SM123', + status: 'sent' + })); + const provider = new DevSmsProvider({ + baseUrl: 'http://devsms:4000/', + requestTimeoutMs: 5000, + fetchImpl: fetchImpl as unknown as typeof fetch + }); + + await expect(provider.send(request)).resolves.toEqual({ + provider: 'devsms', + messageId: 'SM123', + status: 'sent' + }); + }); + + it('throws for non-2xx responses without exposing the response body', async () => { + const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ + error: 'OTP 123456 failed for +14155550123' + }, 400)); + const provider = new DevSmsProvider({ + baseUrl: 'http://devsms:4000', + requestTimeoutMs: 5000, + fetchImpl: fetchImpl as unknown as typeof fetch + }); + + const error = await provider.send(request).then( + () => undefined, + (cause: unknown) => cause as Error + ); + + expect(error).toBeInstanceOf(Error); + expect(error?.message).toBe('DevSmsProvider request failed with 400'); + expect(error?.message).not.toContain('123456'); + expect(error?.message).not.toContain('+14155550123'); + }); + + it('throws on timeout', async () => { + jest.useFakeTimers(); + const fetchImpl = jest.fn((_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => { + const error = new Error('aborted'); + error.name = 'AbortError'; + reject(error); + }); + }) + ); + const provider = new DevSmsProvider({ + baseUrl: 'http://devsms:4000', + requestTimeoutMs: 10, + fetchImpl: fetchImpl as unknown as typeof fetch + }); + + const promise = provider.send(request); + jest.advanceTimersByTime(10); + await expect(promise).rejects.toThrow('DevSmsProvider timed out after 10ms'); + jest.useRealTimers(); + }); + + it('throws for invalid JSON responses', async () => { + const fetchImpl = jest.fn().mockResolvedValue(new Response('not-json', { status: 201 })); + const provider = new DevSmsProvider({ + baseUrl: 'http://devsms:4000', + requestTimeoutMs: 5000, + fetchImpl: fetchImpl as unknown as typeof fetch + }); + + await expect(provider.send(request)).rejects.toThrow('DevSmsProvider returned invalid JSON'); + }); + + it('throws when the response is missing a message ID', async () => { + const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ status: 'queued' })); + const provider = new DevSmsProvider({ + baseUrl: 'http://devsms:4000', + requestTimeoutMs: 5000, + fetchImpl: fetchImpl as unknown as typeof fetch + }); + + await expect(provider.send(request)).rejects.toThrow('DevSmsProvider response missing message ID'); + }); +}); diff --git a/functions/send-sms/__tests__/handler.test.ts b/functions/send-sms/__tests__/handler.test.ts new file mode 100644 index 000000000..b79b9a527 --- /dev/null +++ b/functions/send-sms/__tests__/handler.test.ts @@ -0,0 +1,196 @@ +let handler: any; +import { createMockContext } from '../../../tests/helpers/mock-context'; + +const mockGetEnvOptions = jest.fn(); + +jest.mock('@constructive-io/graphql-env', () => ({ + getEnvOptions: mockGetEnvOptions +}), { virtual: true }); + +const smsConfig = (overrides: Record = {}) => ({ + provider: 'devsms', + senderId: 'TestSender', + requestTimeoutMs: 5000, + dryRun: false, + devsms: { + baseUrl: 'http://devsms:4000' + }, + ...overrides +}); + +const mockSuccessfulFetch = () => { + const fetchMock = jest.fn().mockResolvedValue( + new Response(JSON.stringify({ + id: 'row_123', + provider: 'twilio', + provider_message_id: 'SM123', + status: 'queued' + }), { + status: 201, + headers: { 'content-type': 'application/json' } + }) + ); + global.fetch = fetchMock as unknown as typeof fetch; + return fetchMock; +}; + +describe('send-sms handler', () => { + beforeEach(() => { + jest.resetModules(); + jest.clearAllMocks(); + mockGetEnvOptions.mockReturnValue({ sms: smsConfig() }); + mockSuccessfulFetch(); + // eslint-disable-next-line @typescript-eslint/no-require-imports -- reload after jest.resetModules() so the env mock is applied per test. + handler = require('../handler').default; + }); + + it('sends sms_otp_code payloads through DevSmsProvider', async () => { + const ctx = createMockContext({ + env: { + SMS_PROVIDER: 'devsms', + DEVSMS_BASE_URL: 'http://devsms:4000' + } + }); + + const result = await handler({ + sms_type: 'sms_otp_code', + phone: '+1 (415) 555-0123', + code: '123456' + }, ctx as any); + + expect(result).toEqual({ + complete: true, + provider: 'devsms', + messageId: 'SM123', + status: 'queued' + }); + expect(mockGetEnvOptions).toHaveBeenCalledWith({}, expect.any(String), ctx.env); + expect(global.fetch).toHaveBeenCalledWith( + 'http://devsms:4000/api/sms/send/twilio', + expect.objectContaining({ + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + From: 'TestSender', + To: '+14155550123', + Body: 'Your sign-in code is 123456. Do not share this code.' + }) + }) + ); + }); + + it('sends mfa_verification_code payloads through DevSmsProvider', async () => { + const result = await handler({ + sms_type: 'mfa_verification_code', + user_id: 'user-1', + phone_cc: '44', + phone_number: '020 7946 0018', + code: '654321' + }, createMockContext() as any); + + expect(result).toEqual(expect.objectContaining({ + complete: true, + provider: 'devsms', + messageId: 'SM123' + })); + expect(global.fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: JSON.stringify({ + From: 'TestSender', + To: '+442079460018', + Body: 'Your verification code is 654321. Do not share this code.' + }) + }) + ); + }); + + it('preserves a six-digit code with leading zero', async () => { + await handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '012345' + }, createMockContext() as any); + + expect(global.fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: expect.stringContaining('012345') + }) + ); + }); + + it('throws for unknown sms_type', async () => { + await expect(handler({ + sms_type: 'unknown', + phone: '+14155550123', + code: '123456' + } as any, createMockContext() as any)).rejects.toThrow('Unsupported sms_type: unknown'); + }); + + it('throws for missing code', async () => { + await expect(handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123' + } as any, createMockContext() as any)).rejects.toThrow('code must be a six-digit string'); + }); + + it('throws for invalid phone numbers', async () => { + await expect(handler({ + sms_type: 'sms_otp_code', + phone: 'not-a-phone', + code: '123456' + }, createMockContext() as any)).rejects.toThrow('Invalid phone number'); + }); + + it('throws when provider is missing outside dry-run', async () => { + mockGetEnvOptions.mockReturnValue({ sms: { requestTimeoutMs: 5000, dryRun: false } }); + + await expect(handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, createMockContext() as any)).rejects.toThrow('Missing SMS provider configuration'); + }); + + it('rejects devsms in production', async () => { + const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); + + await expect(handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, ctx as any)).rejects.toThrow('SMS_PROVIDER=devsms is not allowed'); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('dry-run validates and renders without sending', async () => { + mockGetEnvOptions.mockReturnValue({ sms: { dryRun: true } }); + + const result = await handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, createMockContext() as any); + + expect(result).toEqual({ complete: true, dryRun: true }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('does not log code, body, or full phone number', async () => { + mockGetEnvOptions.mockReturnValue({ sms: { dryRun: true } }); + const ctx = createMockContext(); + + await handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '012345' + }, ctx as any); + + const logOutput = JSON.stringify((ctx.log.info as jest.Mock).mock.calls); + expect(logOutput).not.toContain('012345'); + expect(logOutput).not.toContain('Your sign-in code'); + expect(logOutput).not.toContain('+14155550123'); + expect(logOutput).toContain('+1415***0123'); + }); +}); diff --git a/functions/send-sms/__tests__/phone.test.ts b/functions/send-sms/__tests__/phone.test.ts new file mode 100644 index 000000000..c0d9f390e --- /dev/null +++ b/functions/send-sms/__tests__/phone.test.ts @@ -0,0 +1,25 @@ +import { normalizePhone, normalizePhoneParts } from '../phone'; + +describe('phone normalization', () => { + it('normalizes international phone strings to E.164', () => { + expect(normalizePhone('+1 (415) 555-0123')).toBe('+14155550123'); + }); + + it('normalizes phone_cc + national phone_number with trunk prefix', () => { + expect(normalizePhoneParts('+44', '020 7946 0018')).toBe('+442079460018'); + }); + + it('normalizes another country code with a leading national zero', () => { + expect(normalizePhoneParts('81', '03-1234-5678')).toBe('+81312345678'); + }); + + it('rejects empty phone values', () => { + expect(() => normalizePhone('')).toThrow('Missing required field: phone'); + expect(() => normalizePhoneParts('1', '')).toThrow('Missing required field: phone_number'); + }); + + it('rejects invalid phone characters', () => { + expect(() => normalizePhone('+1 415 CALL NOW')).toThrow('Invalid phone number'); + expect(() => normalizePhoneParts('1', '415-555-ABCD')).toThrow('Invalid phone number'); + }); +}); diff --git a/functions/send-sms/config.ts b/functions/send-sms/config.ts new file mode 100644 index 000000000..3201f283a --- /dev/null +++ b/functions/send-sms/config.ts @@ -0,0 +1,25 @@ +import { getEnvOptions } from '@constructive-io/graphql-env'; + +export type SmsProviderName = 'devsms' | 'twilio' | 'sns'; + +export type SmsOptions = { + provider?: SmsProviderName; + senderId?: string; + requestTimeoutMs?: number; + dryRun?: boolean; + devsms?: { + baseUrl?: string; + }; +}; + +type ConstructiveOptionsWithSms = { + sms?: SmsOptions; +}; + +export const loadSmsOptions = ( + env: Record, + cwd: string = process.cwd() +): SmsOptions => { + const options = getEnvOptions({}, cwd, env) as ConstructiveOptionsWithSms; + return options.sms ?? {}; +}; diff --git a/functions/send-sms/handler.json b/functions/send-sms/handler.json new file mode 100644 index 000000000..20c1f2ce4 --- /dev/null +++ b/functions/send-sms/handler.json @@ -0,0 +1,12 @@ +{ + "name": "send-sms", + "version": "1.0.0", + "type": "node-graphql", + "port": 8086, + "taskIdentifier": "sms:send_verification_code", + "description": "Sends SMS verification codes from job payloads", + "dependencies": { + "@constructive-io/graphql-env": "^3.13.0", + "libphonenumber-js": "^1.13.8" + } +} diff --git a/functions/send-sms/handler.ts b/functions/send-sms/handler.ts new file mode 100644 index 000000000..b16b5da0f --- /dev/null +++ b/functions/send-sms/handler.ts @@ -0,0 +1,141 @@ +import type { FunctionHandler } from '@constructive-io/fn-runtime'; + +import { loadSmsOptions } from './config'; +import { maskPhone, normalizePhone, normalizePhoneParts } from './phone'; +import { assertProviderAllowed, createSmsProvider } from './providers/factory'; +import type { SmsPurpose, SmsSendRequest } from './providers/types'; +import { renderSmsBody, type SmsType } from './templates'; + +type SendSmsParams = + | { + sms_type: 'sms_otp_code'; + phone: string; + code: string; + } + | { + sms_type: 'mfa_verification_code'; + user_id: string; + phone_cc: string; + phone_number: string; + code: string; + }; + +type NormalizedSmsJob = { + smsType: SmsType; + to: string; + code: string; + purpose: SmsPurpose; +}; + +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + +const requireSixDigitCode = (value: unknown): string => { + if (typeof value !== 'string' || !/^\d{6}$/.test(value)) { + throw new Error('Invalid SMS verification code: code must be a six-digit string'); + } + return value; +}; + +const normalizePayload = (params: unknown): NormalizedSmsJob => { + if (!isRecord(params)) { + throw new Error('Invalid send-sms payload'); + } + + switch (params.sms_type) { + case 'sms_otp_code': + return { + smsType: 'sms_otp_code', + to: normalizePhone(params.phone), + code: requireSixDigitCode(params.code), + purpose: 'sign_in_otp' + }; + case 'mfa_verification_code': + return { + smsType: 'mfa_verification_code', + to: normalizePhoneParts(params.phone_cc, params.phone_number), + code: requireSixDigitCode(params.code), + purpose: 'mfa_verification' + }; + default: + throw new Error(`Unsupported sms_type: ${String(params.sms_type)}`); + } +}; + +const buildSmsRequest = ( + normalized: NormalizedSmsJob, + senderId: string | undefined, + jobId: string | undefined, + databaseId: string | undefined +): SmsSendRequest => ({ + to: normalized.to, + body: renderSmsBody(normalized.smsType, normalized.code), + ...(senderId && { senderId }), + metadata: { + jobId, + databaseId, + purpose: normalized.purpose + } +}); + +const handler: FunctionHandler = async (params, context) => { + const normalized = normalizePayload(params); + const options = loadSmsOptions(context.env); + const jobId = context.job?.jobId; + const databaseId = context.job?.databaseId; + const maskedPhone = maskPhone(normalized.to); + const providerName = options.provider; + + assertProviderAllowed(options, { nodeEnv: context.env.NODE_ENV }); + + context.log.info('[send-sms] Processing request', { + jobId, + databaseId, + smsType: normalized.smsType, + maskedPhone, + provider: providerName + }); + + const request = buildSmsRequest(normalized, options.senderId, jobId, databaseId); + + if (options.dryRun) { + context.log.info('[send-sms] Dry run complete; provider send skipped', { + jobId, + databaseId, + smsType: normalized.smsType, + maskedPhone, + provider: providerName, + dryRun: true + }); + return { + complete: true, + dryRun: true + }; + } + + const provider = createSmsProvider(options, { + nodeEnv: context.env.NODE_ENV + }); + const result = await provider.send(request); + + context.log.info('[send-sms] Provider accepted message', { + jobId, + databaseId, + smsType: normalized.smsType, + maskedPhone, + provider: result.provider, + providerMessageId: result.messageId, + status: result.status + }); + + return { + complete: true, + provider: result.provider, + messageId: result.messageId, + status: result.status + }; +}; + +export { buildSmsRequest, normalizePayload }; +export type { SendSmsParams }; +export default handler; diff --git a/functions/send-sms/phone.ts b/functions/send-sms/phone.ts new file mode 100644 index 000000000..83c6e1b16 --- /dev/null +++ b/functions/send-sms/phone.ts @@ -0,0 +1,86 @@ +import type { CountryCode } from 'libphonenumber-js'; +import { + getCountries, + getCountryCallingCode, + parsePhoneNumberFromString +} from 'libphonenumber-js'; + +const hasInvalidPhoneCharacters = (value: string): boolean => + /[^\d+().\s-]/.test(value); + +const normalizeCountryCallingCode = (phoneCc: unknown): string => { + if (typeof phoneCc !== 'string' || phoneCc.trim().length === 0) { + throw new Error('Missing required field: phone_cc'); + } + + const digits = phoneCc.replace(/\D/g, ''); + if (!/^[1-9]\d{0,2}$/.test(digits)) { + throw new Error('Invalid phone country code'); + } + return digits; +}; + +const countryForCallingCode = (callingCode: string): CountryCode | undefined => + getCountries().find((country) => getCountryCallingCode(country) === callingCode); + +const ensureValidE164 = (value: string): string => { + if (!/^\+[1-9]\d{7,14}$/.test(value)) { + throw new Error('Invalid phone number'); + } + return value; +}; + +export const normalizePhone = (phone: unknown): string => { + if (typeof phone !== 'string' || phone.trim().length === 0) { + throw new Error('Missing required field: phone'); + } + if (hasInvalidPhoneCharacters(phone)) { + throw new Error('Invalid phone number'); + } + + const parsed = parsePhoneNumberFromString(phone); + if (parsed?.isValid()) { + return parsed.number; + } + + const compact = phone.replace(/[().\s-]/g, ''); + return ensureValidE164(compact); +}; + +export const normalizePhoneParts = (phoneCc: unknown, phoneNumber: unknown): string => { + const callingCode = normalizeCountryCallingCode(phoneCc); + if (typeof phoneNumber !== 'string' || phoneNumber.trim().length === 0) { + throw new Error('Missing required field: phone_number'); + } + if (hasInvalidPhoneCharacters(phoneNumber) || phoneNumber.includes('+')) { + throw new Error('Invalid phone number'); + } + + const country = countryForCallingCode(callingCode); + if (country) { + const parsedNational = parsePhoneNumberFromString(phoneNumber, country); + if (parsedNational?.isValid() && parsedNational.countryCallingCode === callingCode) { + return parsedNational.number; + } + } + + const nationalDigits = phoneNumber.replace(/\D/g, ''); + const candidates = [nationalDigits, nationalDigits.replace(/^0+/, '')] + .filter((candidate, index, all) => candidate && all.indexOf(candidate) === index); + + for (const candidate of candidates) { + const parsed = parsePhoneNumberFromString(`+${callingCode}${candidate}`); + if (parsed?.isValid()) { + return parsed.number; + } + } + + return ensureValidE164(`+${callingCode}${candidates[0] ?? ''}`); +}; + +export const maskPhone = (phone: string): string => { + if (phone.length <= 8) { + return `${phone.slice(0, 2)}***${phone.slice(-2)}`; + } + return `${phone.slice(0, 5)}***${phone.slice(-4)}`; +}; diff --git a/functions/send-sms/providers/devsms.ts b/functions/send-sms/providers/devsms.ts new file mode 100644 index 000000000..b98e38bb6 --- /dev/null +++ b/functions/send-sms/providers/devsms.ts @@ -0,0 +1,103 @@ +import type { SmsProvider, SmsSendRequest, SmsSendResult } from './types'; + +export type DevSmsProviderOptions = { + baseUrl: string; + requestTimeoutMs: number; + fetchImpl?: typeof fetch; +}; + +type DevSmsResponse = { + id?: string; + provider?: string; + provider_message_id?: string; + providerMessageId?: string; + sid?: string; + status?: string; +}; + +const DEVSMS_TWILIO_SEND_PATH = '/api/sms/send/twilio'; + +const normalizeBaseUrl = (baseUrl: string): string => { + const trimmed = baseUrl.trim(); + if (!trimmed) { + throw new Error('DevSmsProvider requires sms.devsms.baseUrl'); + } + return trimmed.endsWith('/') ? trimmed.slice(0, -1) : trimmed; +}; + +const normalizeStatus = (status?: string): SmsSendResult['status'] => { + if (status === 'queued' || status === 'sent' || status === 'delivered') { + return status; + } + return 'unknown'; +}; + +const getMessageId = (body: DevSmsResponse): string | undefined => + body.provider_message_id ?? body.providerMessageId ?? body.sid ?? body.id; + +export class DevSmsProvider implements SmsProvider { + private readonly baseUrl: string; + private readonly requestTimeoutMs: number; + private readonly fetchImpl: typeof fetch; + + constructor(options: DevSmsProviderOptions) { + this.baseUrl = normalizeBaseUrl(options.baseUrl); + this.requestTimeoutMs = options.requestTimeoutMs; + this.fetchImpl = options.fetchImpl ?? fetch; + } + + async send(request: SmsSendRequest): Promise { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), this.requestTimeoutMs); + + let response: Response; + try { + response = await this.fetchImpl(`${this.baseUrl}${DEVSMS_TWILIO_SEND_PATH}`, { + method: 'POST', + headers: { + 'content-type': 'application/json' + }, + body: JSON.stringify({ + ...(request.senderId && { From: request.senderId }), + To: request.to, + Body: request.body + }), + signal: controller.signal + }); + } catch (err) { + const error = err as Error; + if (error.name === 'AbortError') { + throw new Error(`DevSmsProvider timed out after ${this.requestTimeoutMs}ms`); + } + throw new Error(`DevSmsProvider request failed: ${error.message}`); + } finally { + clearTimeout(timeout); + } + + if (!response.ok) { + // Provider responses may echo the request body, OTP, or recipient. + // Keep failures safe for job and CI logs by reporting status only. + throw new Error(`DevSmsProvider request failed with ${response.status}`); + } + + let body: DevSmsResponse; + try { + body = await response.json() as DevSmsResponse; + } catch { + throw new Error('DevSmsProvider returned invalid JSON'); + } + + const messageId = getMessageId(body); + if (!messageId) { + throw new Error('DevSmsProvider response missing message ID'); + } + + return { + provider: 'devsms', + messageId, + status: normalizeStatus(body.status) + }; + } +} + +export { DEVSMS_TWILIO_SEND_PATH }; diff --git a/functions/send-sms/providers/factory.ts b/functions/send-sms/providers/factory.ts new file mode 100644 index 000000000..23bfb9531 --- /dev/null +++ b/functions/send-sms/providers/factory.ts @@ -0,0 +1,50 @@ +import type { SmsOptions } from '../config'; +import { DevSmsProvider } from './devsms'; +import type { SmsProvider } from './types'; + +export type SmsProviderFactoryOptions = { + nodeEnv?: string; + fetchImpl?: typeof fetch; +}; + +const getRequestTimeoutMs = (options: SmsOptions): number => + options.requestTimeoutMs ?? 5000; + +export const assertProviderAllowed = ( + options: SmsOptions, + factoryOptions: SmsProviderFactoryOptions = {} +): void => { + if (factoryOptions.nodeEnv === 'production' && options.provider === 'devsms') { + throw new Error('SMS_PROVIDER=devsms is not allowed when NODE_ENV=production'); + } +}; + +export const createSmsProvider = ( + options: SmsOptions, + factoryOptions: SmsProviderFactoryOptions = {} +): SmsProvider => { + assertProviderAllowed(options, factoryOptions); + + if (!options.provider) { + throw new Error('Missing SMS provider configuration: options.sms.provider'); + } + + switch (options.provider) { + case 'devsms': { + const baseUrl = options.devsms?.baseUrl; + if (!baseUrl) { + throw new Error('Missing DevSms base URL configuration: options.sms.devsms.baseUrl'); + } + return new DevSmsProvider({ + baseUrl, + requestTimeoutMs: getRequestTimeoutMs(options), + fetchImpl: factoryOptions.fetchImpl + }); + } + case 'twilio': + case 'sns': + throw new Error(`Unsupported SMS provider: ${options.provider}`); + default: + throw new Error(`Unsupported SMS provider: ${String(options.provider)}`); + } +}; diff --git a/functions/send-sms/providers/types.ts b/functions/send-sms/providers/types.ts new file mode 100644 index 000000000..81c3047e4 --- /dev/null +++ b/functions/send-sms/providers/types.ts @@ -0,0 +1,22 @@ +export type SmsPurpose = 'sign_in_otp' | 'mfa_verification'; + +export type SmsSendRequest = { + to: string; + body: string; + senderId?: string; + metadata?: { + jobId?: string; + databaseId?: string; + purpose: SmsPurpose; + }; +}; + +export type SmsSendResult = { + provider: string; + messageId: string; + status: 'queued' | 'sent' | 'delivered' | 'unknown'; +}; + +export interface SmsProvider { + send(request: SmsSendRequest): Promise; +} diff --git a/functions/send-sms/templates.ts b/functions/send-sms/templates.ts new file mode 100644 index 000000000..7f485ee12 --- /dev/null +++ b/functions/send-sms/templates.ts @@ -0,0 +1,9 @@ +export type SmsType = 'sms_otp_code' | 'mfa_verification_code'; + +const SMS_TEMPLATES: Record = { + sms_otp_code: 'Your sign-in code is {code}. Do not share this code.', + mfa_verification_code: 'Your verification code is {code}. Do not share this code.' +}; + +export const renderSmsBody = (smsType: SmsType, code: string): string => + SMS_TEMPLATES[smsType].replace('{code}', code); diff --git a/job/service/package.json b/job/service/package.json index 888935b44..ee93f1368 100644 --- a/job/service/package.json +++ b/job/service/package.json @@ -23,6 +23,7 @@ "@constructive-io/knative-job-server": "workspace:^", "@constructive-io/knative-job-worker": "workspace:^", "@constructive-io/send-email-fn": "workspace:^", + "@constructive-io/send-sms-fn": "workspace:^", "@constructive-io/send-verification-link-fn": "workspace:^", "@pgpmjs/env": "^2.15.3", "@pgpmjs/logger": "^2.4.3", diff --git a/k8s/base/constructive/knative-job-service.yaml b/k8s/base/constructive/knative-job-service.yaml index cf0fcbeba..031ef1f6d 100644 --- a/k8s/base/constructive/knative-job-service.yaml +++ b/k8s/base/constructive/knative-job-service.yaml @@ -49,7 +49,7 @@ spec: - name: JOBS_SUPPORT_ANY value: "false" - name: JOBS_SUPPORTED - value: "email:send_email,email:send_verification_link" + value: "email:send_email,email:send_verification_link,sms:send_verification_code" - name: JOBS_CALLBACK_HOST value: "knative-job-service.interweb.svc.cluster.local" - name: JOBS_CALLBACK_BASE_URL @@ -65,7 +65,7 @@ spec: value: "development" - name: INTERNAL_GATEWAY_DEVELOPMENT_MAP - value: '{"email:send_email":"http://send-email.interweb.svc.cluster.local","email:send_verification_link":"http://send-verification-link.interweb.svc.cluster.local"}' + value: '{"email:send_email":"http://send-email.interweb.svc.cluster.local","email:send_verification_link":"http://send-verification-link.interweb.svc.cluster.local","sms:send_verification_code":"http://send-sms.interweb.svc.cluster.local"}' # Optional: stable hostname for logging/coordination - name: HOSTNAME diff --git a/k8s/base/functions/send-sms.yaml b/k8s/base/functions/send-sms.yaml new file mode 100644 index 000000000..a413ebbf1 --- /dev/null +++ b/k8s/base/functions/send-sms.yaml @@ -0,0 +1,67 @@ +apiVersion: serving.knative.dev/v1 +kind: Service +metadata: + name: send-sms + labels: + app.kubernetes.io/name: send-sms + app.kubernetes.io/component: function + app.kubernetes.io/part-of: constructive-jobs + networking.knative.dev/visibility: cluster-local +spec: + template: + metadata: + labels: + app.kubernetes.io/name: send-sms + app.kubernetes.io/component: function + app.kubernetes.io/part-of: constructive-jobs + annotations: + autoscaling.knative.dev/minScale: "1" + autoscaling.knative.dev/maxScale: "10" + autoscaling.knative.dev/target: "50" + serving.knative.dev/timeout: "300s" + run.googleapis.com/cpu-throttling: "false" + spec: + containerConcurrency: 10 + timeoutSeconds: 300 + + containers: + - name: function + image: ghcr.io/constructive-io/constructive:e0b55cc + imagePullPolicy: Always + + command: ["node"] + args: ["functions/send-sms/dist/index.js"] + + ports: + - containerPort: 8080 + protocol: TCP + + env: + - name: NODE_ENV + value: "production" + - name: LOG_LEVEL + value: "debug" + - name: LOG_TIMESTAMP + value: "true" + - name: SMS_REQUEST_TIMEOUT_MS + value: "5000" + + resources: + requests: + memory: "128Mi" + cpu: "100m" + limits: + memory: "512Mi" + cpu: "500m" + + volumeMounts: + - name: tmp + mountPath: /tmp + + volumes: + - name: tmp + emptyDir: {} + + traffic: + - percent: 100 + latestRevision: true diff --git a/k8s/base/kustomization.yaml b/k8s/base/kustomization.yaml index d7c2eec61..fcb0f633a 100644 --- a/k8s/base/kustomization.yaml +++ b/k8s/base/kustomization.yaml @@ -23,6 +23,7 @@ resources: # Function workloads - ./functions/send-email.yaml - ./functions/send-verification-link.yaml + - ./functions/send-sms.yaml # Required Secrets are intentionally not committed in the shared base. Create # pg-credentials, postgres-superuser, pgadmin-credentials, mailgun-credentials, diff --git a/k8s/overlays/local-simple/config.yaml b/k8s/overlays/local-simple/config.yaml index c54f2c04c..77ac5fe9b 100644 --- a/k8s/overlays/local-simple/config.yaml +++ b/k8s/overlays/local-simple/config.yaml @@ -22,5 +22,9 @@ data: LOG_TIMESTAMP: "true" + SMS_PROVIDER: "devsms" + SMS_SENDER_ID: "Constructive" + DEVSMS_BASE_URL: "http://devsms.constructive-functions.svc.cluster.local:4000" + SEND_EMAIL_DRY_RUN: "true" SEND_VERIFICATION_LINK_DRY_RUN: "true" diff --git a/k8s/overlays/local-simple/devsms-local.yaml b/k8s/overlays/local-simple/devsms-local.yaml new file mode 100644 index 000000000..c9d9a0462 --- /dev/null +++ b/k8s/overlays/local-simple/devsms-local.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: devsms + labels: + app: devsms +spec: + replicas: 1 + selector: + matchLabels: + app: devsms + template: + metadata: + labels: + app: devsms + spec: + containers: + - name: devsms + image: ghcr.io/mrmeaow/devsms:latest + ports: + - containerPort: 4000 + name: api + - containerPort: 5153 + name: web +--- +apiVersion: v1 +kind: Service +metadata: + name: devsms + labels: + app: devsms +spec: + type: ClusterIP + selector: + app: devsms + ports: + - name: api + port: 4000 + targetPort: api + - name: web + port: 5153 + targetPort: web diff --git a/k8s/overlays/local-simple/kustomization.yaml b/k8s/overlays/local-simple/kustomization.yaml index 12a50871f..4cf188c0f 100644 --- a/k8s/overlays/local-simple/kustomization.yaml +++ b/k8s/overlays/local-simple/kustomization.yaml @@ -7,6 +7,7 @@ resources: # Infrastructure - ./postgres-local.yaml - ./minio-local.yaml + - ./devsms-local.yaml # Secrets & config - ./config.yaml - ./pg-secret.yaml diff --git a/k8s/overlays/local/constructive/config.yaml b/k8s/overlays/local/constructive/config.yaml index c3fc0f43e..6c20560f4 100644 --- a/k8s/overlays/local/constructive/config.yaml +++ b/k8s/overlays/local/constructive/config.yaml @@ -4,3 +4,6 @@ metadata: name: constructive data: PGHOST: "postgres.constructive-functions.svc.cluster.local" + SMS_PROVIDER: "devsms" + SMS_SENDER_ID: "Constructive" + DEVSMS_BASE_URL: "http://devsms.constructive-functions.svc.cluster.local:4000" diff --git a/k8s/overlays/local/devsms-local.yaml b/k8s/overlays/local/devsms-local.yaml new file mode 100644 index 000000000..c9d9a0462 --- /dev/null +++ b/k8s/overlays/local/devsms-local.yaml @@ -0,0 +1,42 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: devsms + labels: + app: devsms +spec: + replicas: 1 + selector: + matchLabels: + app: devsms + template: + metadata: + labels: + app: devsms + spec: + containers: + - name: devsms + image: ghcr.io/mrmeaow/devsms:latest + ports: + - containerPort: 4000 + name: api + - containerPort: 5153 + name: web +--- +apiVersion: v1 +kind: Service +metadata: + name: devsms + labels: + app: devsms +spec: + type: ClusterIP + selector: + app: devsms + ports: + - name: api + port: 4000 + targetPort: api + - name: web + port: 5153 + targetPort: web diff --git a/k8s/overlays/local/kustomization.yaml b/k8s/overlays/local/kustomization.yaml index 35f58afee..b9f202158 100644 --- a/k8s/overlays/local/kustomization.yaml +++ b/k8s/overlays/local/kustomization.yaml @@ -7,6 +7,7 @@ resources: - ./namespace.yaml - ./postgres-local.yaml - ./minio-local.yaml + - ./devsms-local.yaml # Core Constructive config - ../../base/constructive/config.yaml - ../../base/constructive/dashboard-config.yaml @@ -26,6 +27,7 @@ resources: # Functions - ../../base/functions/send-email.yaml - ../../base/functions/send-verification-link.yaml + - ../../base/functions/send-sms.yaml patches: # Skaffold image replacement: only rewrite function images, not server/dashboard/db-job @@ -47,6 +49,15 @@ patches: version: v1 kind: Service name: send-verification-link + - patch: |- + - op: replace + path: /spec/template/spec/containers/0/image + value: constructive-functions:local + target: + group: serving.knative.dev + version: v1 + kind: Service + name: send-sms - patch: |- - op: replace path: /spec/template/spec/containers/0/image @@ -86,3 +97,27 @@ patches: version: v1 kind: Service name: send-verification-link + - patch: |- + - op: replace + path: /spec/template/spec/containers/0/env/0/value + value: "development" + - op: add + path: /spec/template/spec/containers/0/env/- + value: + name: SMS_PROVIDER + value: "devsms" + - op: add + path: /spec/template/spec/containers/0/env/- + value: + name: SMS_SENDER_ID + value: "Constructive" + - op: add + path: /spec/template/spec/containers/0/env/- + value: + name: DEVSMS_BASE_URL + value: "http://devsms.constructive-functions.svc.cluster.local:4000" + target: + group: serving.knative.dev + version: v1 + kind: Service + name: send-sms diff --git a/package.json b/package.json index 9d3b522a3..d8310be27 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "eslint-plugin-unused-imports": "^4.0.0", "globals": "^16.5.0", "jest": "^30.2.0", + "libphonenumber-js": "1.13.8", "prettier": "^3.7.4", "ts-jest": "^29.4.0", "tsx": "^4.19.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8dcd03e50..ed2ba31d3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -38,6 +38,9 @@ importers: jest: specifier: ^30.2.0 version: 30.2.0(@types/node@22.19.3) + libphonenumber-js: + specifier: 1.13.8 + version: 1.13.8 pg: specifier: ^8.20.0 version: 8.20.0 @@ -101,6 +104,28 @@ importers: specifier: ^5.1.6 version: 5.9.3 + generated/send-sms: + dependencies: + '@constructive-io/fn-runtime': + specifier: workspace:^ + version: link:../../packages/fn-runtime + '@constructive-io/graphql-env': + specifier: ^3.13.0 + version: 3.15.0 + libphonenumber-js: + specifier: ^1.13.8 + version: 1.13.8 + devDependencies: + '@types/node': + specifier: ^22.10.4 + version: 22.19.3 + makage: + specifier: ^0.1.10 + version: 0.1.12 + typescript: + specifier: ^5.1.6 + version: 5.9.3 + generated/send-verification-link: dependencies: '@constructive-io/fn-runtime': @@ -230,6 +255,9 @@ importers: '@constructive-io/send-email-fn': specifier: workspace:^ version: link:../../generated/send-email + '@constructive-io/send-sms-fn': + specifier: workspace:^ + version: link:../../generated/send-sms '@constructive-io/send-verification-link-fn': specifier: workspace:^ version: link:../../generated/send-verification-link @@ -619,6 +647,12 @@ packages: '@bcoe/v8-coverage@0.2.3': resolution: {integrity: sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==} + '@constructive-io/graphql-env@3.15.0': + resolution: {integrity: sha512-twK195FV69XUwLCQ8ifYxALiGF157N424YUzZhKP52DNmHjBY2GY1hLj6SM5K2t7+LrOFJPNeo7U/d5I9yK+LQ==} + + '@constructive-io/graphql-types@3.14.0': + resolution: {integrity: sha512-ATT9HIXD4Qa+WuayaAWzwvbgNBlU1ssCjUroo7zbZsSxXX7pgSDK1KSk/jmCldYIYLez19xgXx3QI7rlP6A/iQ==} + '@constructive-io/job-pg@2.5.4': resolution: {integrity: sha512-cjJxL/P1g4s07PiXiw31jb7c56aQmdPur+WyQXC0lAig1DIFSeJqbQjY6PXaFQepxgivwIBf9P9v+EalQaOtPQ==} @@ -1076,6 +1110,9 @@ packages: '@pgpmjs/env@2.17.0': resolution: {integrity: sha512-3WPwJ4prFWGGIRzyR52/JG84hM+Qe6lVtQ+bcCpGnGuhukFowALpaegRZxi3LT/pO6D8wW1Y3nW9LugfJLO6KQ==} + '@pgpmjs/env@2.27.0': + resolution: {integrity: sha512-/JTHMBx7X7bnoy5kPCNDgu2eWNkjrBaeIHqlXErcErldt1RMEMjLYQykCYW5QdOAUP8RKS6pIzszvatoROlkAA==} + '@pgpmjs/logger@1.5.0': resolution: {integrity: sha512-R27o5MiOsezI5rAWdJyuOkWUK6zxr8Mg61hPs7uCu//sECoprR4/7CVeFIHwn7+gyrjUk0wBz0dQcJhjYzVDpw==} @@ -1091,6 +1128,9 @@ packages: '@pgpmjs/types@2.28.0': resolution: {integrity: sha512-XYCcWnxkIrZEHF2oxxtU1yMeMa4bfw5za5CsDnMx0uasdtG0Y5YwDqruuv0uzYdz0id927LMb6svE38vrmPTIg==} + '@pgpmjs/types@2.34.0': + resolution: {integrity: sha512-2Y6o1IyqfuaogcoIGOECuR33sl7XlLYGQzi5qD6CLLPwAgEXzQj3u6dxchuzWxnWaE3Sf6W+SwLStxx688k2nQ==} + '@pkgjs/parseargs@0.11.0': resolution: {integrity: sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg==} engines: {node: '>=14'} @@ -2181,6 +2221,10 @@ packages: graceful-fs@4.2.11: resolution: {integrity: sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==} + graphile-config@1.0.1: + resolution: {integrity: sha512-sVdSWNmetW/WZKVQ0Dii2kCu0Le6X6qwuBRecWg575iOMjbZgxo+b4oVeSOTtR6NTKuLsMYQBkzSaeoAKOPB+A==} + engines: {node: '>=22'} + graphql-request@7.4.0: resolution: {integrity: sha512-xfr+zFb/QYbs4l4ty0dltqiXIp07U6sl+tOKAb0t50/EnQek6CVVBLjETXi+FghElytvgaAWtIOt3EV7zLzIAQ==} peerDependencies: @@ -2295,6 +2339,10 @@ packages: inquirerer@4.8.1: resolution: {integrity: sha512-X8cPy91JMH6EmUPUqgnxc+oYssHdQlitWR23youH2208F2enxElCKc6Mt/5H8KAupYDgOuRuyBO+SRaRXStj8A==} + interpret@3.1.1: + resolution: {integrity: sha512-6xwYfHbajpoF0xLW+iwLkhwgvLoZDfjYfoFNu8ftMoXINzwuymNLd9u/KmwtdT2GbR+/Cz66otEGEVVUHX9QLQ==} + engines: {node: '>=10.13.0'} + ipaddr.js@1.9.1: resolution: {integrity: sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==} engines: {node: '>= 0.10'} @@ -2685,6 +2733,9 @@ packages: resolution: {integrity: sha512-+bT2uH4E5LGE7h/n3evcS/sQlJXCpIp6ym8OWJ5eV6+67Dsql/LaaT7qJBAt2rzfoa/5QBGBhxDix1dMt2kQKQ==} engines: {node: '>= 0.8.0'} + libphonenumber-js@1.13.8: + resolution: {integrity: sha512-80xal1m93rADejw2pMp2MSzFhHCPLEspjHxnH2UtqI+DgAmElsbmLMiqk9niwH9NWAfjsRtaJI+qBrOEmRx9nQ==} + lines-and-columns@1.2.4: resolution: {integrity: sha512-7ylylesZQ/PV29jhEDl3Ufjo6ZX7gCqJr5F7PKrqc93v7fzSymt1BpwEU8nAUXs8qzzvqhbjhK5QZg6Mt/HkBg==} @@ -3093,6 +3144,9 @@ packages: pg-env@1.15.0: resolution: {integrity: sha512-1bs3pcNOOrA0om3TNJGwbZu7JXKc/tenyVW4KX6ljARxvKtRSUcGl6sbpKVCXJo+Y98W0nRvPgfa/SlqlumRsg==} + pg-env@1.18.0: + resolution: {integrity: sha512-RzZISyA7ZJlnKOurlkqX2k/1iJl5kqn39OlvUbn0wtX1ENYtfClFlbkzye+WFdw/s9VnPY0uSzPJPKcA+8gBxQ==} + pg-env@1.8.2: resolution: {integrity: sha512-YzxNQKZmFRRJKX5t149Ys2JoAsc6OCHcaoYH/82si7gwVC9ODaFTFtQn7gv3VpoGsNkH90t6iEPWvmLIgv2rDg==} @@ -3922,6 +3976,23 @@ snapshots: '@bcoe/v8-coverage@0.2.3': {} + '@constructive-io/graphql-env@3.15.0': + dependencies: + '@constructive-io/graphql-types': 3.14.0 + '@pgpmjs/env': 2.27.0 + deepmerge: 4.3.1 + transitivePeerDependencies: + - supports-color + + '@constructive-io/graphql-types@3.14.0': + dependencies: + '@pgpmjs/types': 2.34.0 + deepmerge: 4.3.1 + graphile-config: 1.0.1 + pg-env: 1.18.0 + transitivePeerDependencies: + - supports-color + '@constructive-io/job-pg@2.5.4': dependencies: '@constructive-io/job-utils': 2.5.4 @@ -4550,6 +4621,11 @@ snapshots: '@pgpmjs/types': 2.21.0 deepmerge: 4.3.1 + '@pgpmjs/env@2.27.0': + dependencies: + '@pgpmjs/types': 2.34.0 + deepmerge: 4.3.1 + '@pgpmjs/logger@1.5.0': dependencies: yanse: 0.2.1 @@ -4570,6 +4646,10 @@ snapshots: dependencies: pg-env: 1.15.0 + '@pgpmjs/types@2.34.0': + dependencies: + pg-env: 1.18.0 + '@pkgjs/parseargs@0.11.0': optional: true @@ -5825,6 +5905,17 @@ snapshots: graceful-fs@4.2.11: {} + graphile-config@1.0.1: + dependencies: + chalk: 4.1.2 + debug: 4.4.3(supports-color@5.5.0) + interpret: 3.1.1 + semver: 7.7.3 + tslib: 2.8.1 + yargs: 17.7.2 + transitivePeerDependencies: + - supports-color + graphql-request@7.4.0(graphql@16.13.0): dependencies: '@graphql-typed-document-node/core': 3.2.0(graphql@16.13.0) @@ -5951,6 +6042,8 @@ snapshots: minimist: 1.2.8 yanse: 0.2.1 + interpret@3.1.1: {} + ipaddr.js@1.9.1: {} is-arrayish@0.2.1: {} @@ -6715,6 +6808,8 @@ snapshots: prelude-ls: 1.2.1 type-check: 0.4.0 + libphonenumber-js@1.13.8: {} + lines-and-columns@1.2.4: {} locate-path@5.0.0: @@ -7280,6 +7375,8 @@ snapshots: pg-env@1.15.0: {} + pg-env@1.18.0: {} + pg-env@1.8.2: {} pg-int8@1.0.1: {} diff --git a/scripts/dev.ts b/scripts/dev.ts index f7f6177e5..b74e7b057 100644 --- a/scripts/dev.ts +++ b/scripts/dev.ts @@ -61,6 +61,9 @@ const sharedEnv: Record = { SMTP_HOST: 'localhost', SMTP_PORT: '1025', LOCAL_APP_PORT: '3000', + SMS_PROVIDER: 'devsms', + DEVSMS_BASE_URL: 'http://localhost:4000', + SMS_SENDER_ID: 'Constructive', SEND_VERIFICATION_LINK_DRY_RUN: 'true', SEND_EMAIL_DRY_RUN: 'true', }; diff --git a/scripts/generate.ts b/scripts/generate.ts index c4f038520..1aaa134fa 100644 --- a/scripts/generate.ts +++ b/scripts/generate.ts @@ -282,6 +282,37 @@ function generateSkaffoldYaml(fns: FunctionInfo[]): void { } } +function symlinkFunctionSupportFiles(fnDir: string, genDir: string, base: string = ''): void { + const entries = fs.readdirSync(path.join(fnDir, base)) as string[]; + for (const entry of entries) { + if (entry === '__tests__') continue; + + const relPath = path.join(base, entry); + const sourcePath = path.join(fnDir, relPath); + const targetPath = path.join(genDir, relPath); + const stat = fs.statSync(sourcePath); + + if (stat.isDirectory()) { + symlinkFunctionSupportFiles(fnDir, genDir, relPath); + continue; + } + + if ( + relPath === 'handler.ts' || + relPath === 'handler.py' || + !(/\.(ts|d\.ts|py)$/.test(entry)) || + entry.endsWith('.test.ts') + ) { + continue; + } + + const targetDir = path.dirname(targetPath); + if (!fs.existsSync(targetDir)) fs.mkdirSync(targetDir, { recursive: true }); + const linked = ensureSymlink(sourcePath, targetPath); + if (linked) console.log(` - ${relPath} -> functions/${path.basename(fnDir)}/${relPath}`); + } +} + // --- Main --- function main(): void { @@ -360,15 +391,8 @@ function main(): void { if (linked) console.log(` - handler.py -> functions/${fnName}/handler.py`); } - // Symlink any .d.ts or .py files (excluding handler.py which is handled above) - const files = fs.readdirSync(fnDir) as string[]; - for (const file of files) { - if (file.endsWith('.d.ts') || (file.endsWith('.py') && file !== 'handler.py')) { - const target = path.join(fnDir, file); - const linked = ensureSymlink(target, path.join(genDir, file)); - if (linked) console.log(` - ${file} -> functions/${fnName}/${file}`); - } - } + // Symlink additional support source files/directories used by richer functions. + symlinkFunctionSupportFiles(fnDir, genDir); } // --packages-only: stop here, only workspace packages were needed @@ -385,9 +409,11 @@ function main(): void { allManifests.push(readManifest(fnDir)); } - // Auto-assign ports for functions that don't have one + // Auto-assign ports for functions that don't have one, filling the lowest + // available local function port first so adding a high explicit port does not + // shift existing unpinned examples. const usedPorts = new Set(allManifests.filter((m) => m.port).map((m) => m.port!)); - let nextPort = usedPorts.size > 0 ? Math.max(...usedPorts) + 1 : 8081; + let nextPort = 8081; for (const m of allManifests) { if (!m.port) { while (usedPorts.has(nextPort)) nextPort++; diff --git a/skaffold.yaml b/skaffold.yaml index bf010088d..350a55bdd 100644 --- a/skaffold.yaml +++ b/skaffold.yaml @@ -149,6 +149,50 @@ profiles: namespace: constructive-functions port: 3000 localPort: 3002 + - name: send-sms + build: + artifacts: + - image: constructive-functions + context: . + docker: + dockerfile: Dockerfile.dev + sync: + manual: + - src: 'functions/**/*.ts' + dest: /usr/src/app + local: + push: false + manifests: + kustomize: + paths: + - k8s/overlays/local-simple + rawYaml: + - generated/send-sms/k8s/local-deployment.yaml + - generated/send-sms/k8s/functions-configmap.yaml + deploy: + kubectl: + defaultNamespace: constructive-functions + portForward: + - resourceType: service + resourceName: send-sms + namespace: constructive-functions + port: 80 + localPort: 8086 + - resourceType: service + resourceName: knative-job-service + namespace: constructive-functions + port: 8080 + localPort: 8080 + - resourceType: service + resourceName: postgres + namespace: constructive-functions + port: 5432 + localPort: 5432 + - resourceType: service + resourceName: constructive-server + namespace: constructive-functions + port: 3000 + localPort: 3002 - name: send-verification-link build: artifacts: @@ -270,6 +314,7 @@ profiles: - generated/example/k8s/local-deployment.yaml - generated/python-example/k8s/local-deployment.yaml - generated/send-email/k8s/local-deployment.yaml + - generated/send-sms/k8s/local-deployment.yaml - generated/send-verification-link/k8s/local-deployment.yaml - generated/sql-example/k8s/local-deployment.yaml - generated/functions-configmap.yaml @@ -292,6 +337,11 @@ profiles: namespace: constructive-functions port: 80 localPort: 8081 + - resourceType: service + resourceName: send-sms + namespace: constructive-functions + port: 80 + localPort: 8086 - resourceType: service resourceName: send-verification-link namespace: constructive-functions @@ -352,6 +402,11 @@ profiles: namespace: constructive-functions port: 80 localPort: 8081 + - resourceType: service + resourceName: send-sms + namespace: constructive-functions + port: 80 + localPort: 8086 - resourceType: service resourceName: send-verification-link namespace: constructive-functions From 2860ee559bffe5aebc13c8d0fde7e582b9380bbe Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 17 Jul 2026 21:05:02 +0800 Subject: [PATCH 02/12] fix: keep fn generator aligned with SMS function output --- packages/fn-generator/src/builders/package.ts | 39 +++++++++++++++---- packages/fn-generator/src/discovery.ts | 4 +- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/packages/fn-generator/src/builders/package.ts b/packages/fn-generator/src/builders/package.ts index 327f5ddbe..ad9b9009a 100644 --- a/packages/fn-generator/src/builders/package.ts +++ b/packages/fn-generator/src/builders/package.ts @@ -8,7 +8,7 @@ import type { FunctionInfo, Manifest } from '../types'; * Build all manifests for a single function: * - per-template files (placeholders applied, package.json/tsconfig.json processed) * - shared template files (same processing) - * - symlinks for handler.{ts,py}, *.d.ts, and any other *.py + * - symlinks for handler.{ts,py} and supporting TypeScript/Python sources * * `templateDir` is the resolved type-specific dir (`templates/node-graphql/` etc.). */ @@ -56,17 +56,40 @@ export const buildPackageManifests = ( out.push({ kind: 'symlink', path: path.join(args.genDir, 'handler.py'), target: handlerPy }); } - // 4. Auxiliary symlinks: all *.d.ts and *.py (except handler.py already linked above) - const files = fs.readdirSync(args.fnDir); - for (const file of files) { - if (file.endsWith('.d.ts') || (file.endsWith('.py') && file !== 'handler.py')) { + // 4. Supporting source symlinks. Richer functions may split implementation + // across nested TypeScript/Python files; mirror scripts/generate.ts exactly. + const addSupportSymlinks = (base = ''): void => { + const entries = fs.readdirSync(path.join(args.fnDir, base)); + for (const entry of entries) { + if (entry === '__tests__') continue; + + const relPath = path.join(base, entry); + const sourcePath = path.join(args.fnDir, relPath); + const stat = fs.statSync(sourcePath); + + if (stat.isDirectory()) { + addSupportSymlinks(relPath); + continue; + } + + if ( + relPath === 'handler.ts' || + relPath === 'handler.py' || + !(/\.(ts|d\.ts|py)$/.test(entry)) || + entry.endsWith('.test.ts') + ) { + continue; + } + out.push({ kind: 'symlink', - path: path.join(args.genDir, file), - target: path.join(args.fnDir, file), + path: path.join(args.genDir, relPath), + target: sourcePath, }); } - } + }; + + addSupportSymlinks(); return out; }; diff --git a/packages/fn-generator/src/discovery.ts b/packages/fn-generator/src/discovery.ts index 6553c88af..421bc7168 100644 --- a/packages/fn-generator/src/discovery.ts +++ b/packages/fn-generator/src/discovery.ts @@ -52,7 +52,9 @@ export const assignAndValidatePorts = ( const usedPorts = new Set( manifests.filter((m) => m.port).map((m) => m.port as number) ); - let nextPort = usedPorts.size > 0 ? Math.max(...usedPorts) + 1 : 8081; + // Fill the lowest available local function port first. Explicit high ports + // must not shift existing functions that rely on automatic assignment. + let nextPort = 8081; for (const m of manifests) { if (!m.port) { while (usedPorts.has(nextPort)) nextPort++; From 57312cecedea3f4dd0761465086a3549f3364fb8 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 24 Jul 2026 11:08:12 +0800 Subject: [PATCH 03/12] refactor: resolve SMS defaults in consumer --- functions/send-sms/__tests__/handler.test.ts | 31 ++++++++++++++++++-- functions/send-sms/config.ts | 4 +-- functions/send-sms/handler.ts | 3 +- functions/send-sms/providers/factory.ts | 4 ++- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/functions/send-sms/__tests__/handler.test.ts b/functions/send-sms/__tests__/handler.test.ts index b79b9a527..51e2cc663 100644 --- a/functions/send-sms/__tests__/handler.test.ts +++ b/functions/send-sms/__tests__/handler.test.ts @@ -143,14 +143,41 @@ describe('send-sms handler', () => { }, createMockContext() as any)).rejects.toThrow('Invalid phone number'); }); - it('throws when provider is missing outside dry-run', async () => { - mockGetEnvOptions.mockReturnValue({ sms: { requestTimeoutMs: 5000, dryRun: false } }); + it.each([ + ['no sms configuration', {}], + ['an empty sms configuration', { sms: {} }] + ])('defaults dry-run to false and requires a provider with %s', async (_case, config) => { + mockGetEnvOptions.mockReturnValue(config); await expect(handler({ sms_type: 'sms_otp_code', phone: '+14155550123', code: '123456' }, createMockContext() as any)).rejects.toThrow('Missing SMS provider configuration'); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('applies consumer defaults when timeout and dry-run are not configured', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'devsms', + senderId: 'TestSender', + devsms: { + baseUrl: 'http://devsms:4000' + } + } + }); + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); + + await handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, createMockContext() as any); + + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 5000); + setTimeoutSpy.mockRestore(); }); it('rejects devsms in production', async () => { diff --git a/functions/send-sms/config.ts b/functions/send-sms/config.ts index 3201f283a..9f57252d7 100644 --- a/functions/send-sms/config.ts +++ b/functions/send-sms/config.ts @@ -1,9 +1,7 @@ import { getEnvOptions } from '@constructive-io/graphql-env'; -export type SmsProviderName = 'devsms' | 'twilio' | 'sns'; - export type SmsOptions = { - provider?: SmsProviderName; + provider?: string; senderId?: string; requestTimeoutMs?: number; dryRun?: boolean; diff --git a/functions/send-sms/handler.ts b/functions/send-sms/handler.ts index b16b5da0f..e6d449e14 100644 --- a/functions/send-sms/handler.ts +++ b/functions/send-sms/handler.ts @@ -85,6 +85,7 @@ const handler: FunctionHandler = async (params, context) => { const databaseId = context.job?.databaseId; const maskedPhone = maskPhone(normalized.to); const providerName = options.provider; + const dryRun = options.dryRun ?? false; assertProviderAllowed(options, { nodeEnv: context.env.NODE_ENV }); @@ -98,7 +99,7 @@ const handler: FunctionHandler = async (params, context) => { const request = buildSmsRequest(normalized, options.senderId, jobId, databaseId); - if (options.dryRun) { + if (dryRun) { context.log.info('[send-sms] Dry run complete; provider send skipped', { jobId, databaseId, diff --git a/functions/send-sms/providers/factory.ts b/functions/send-sms/providers/factory.ts index 23bfb9531..ff1333d21 100644 --- a/functions/send-sms/providers/factory.ts +++ b/functions/send-sms/providers/factory.ts @@ -7,8 +7,10 @@ export type SmsProviderFactoryOptions = { fetchImpl?: typeof fetch; }; +const DEFAULT_REQUEST_TIMEOUT_MS = 5000; + const getRequestTimeoutMs = (options: SmsOptions): number => - options.requestTimeoutMs ?? 5000; + options.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS; export const assertProviderAllowed = ( options: SmsOptions, From 815beb9197fce326e91c0fe8acee0260cfa85112 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 24 Jul 2026 11:26:35 +0800 Subject: [PATCH 04/12] test: extend send verification e2e timeout --- tests/e2e/__tests__/send-verification-link.e2e.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/e2e/__tests__/send-verification-link.e2e.test.ts b/tests/e2e/__tests__/send-verification-link.e2e.test.ts index c62d46f41..553f1edd2 100644 --- a/tests/e2e/__tests__/send-verification-link.e2e.test.ts +++ b/tests/e2e/__tests__/send-verification-link.e2e.test.ts @@ -19,6 +19,8 @@ import { addJob, waitForJobComplete, deleteTestJobs } from '../utils/jobs'; const TEST_PREFIX = 'k8s-e2e-send-verification-link'; describe('E2E: send-verification-link', () => { + jest.setTimeout(90000); + let pg: TestClient; let databaseId: string; @@ -44,7 +46,7 @@ describe('E2E: send-verification-link', () => { expect(job.id).toBeDefined(); console.log(`Added email:send_verification_link job: ${job.id}`); - const result = await waitForJobComplete(pg, job.id, { timeout: 30000 }); + const result = await waitForJobComplete(pg, job.id, { timeout: 60000 }); console.log(`Job result: ${result.status}`, result.error || ''); From f857bfb46a1e0b26cbb4df8035342d7a3f5c707e Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 24 Jul 2026 12:13:04 +0800 Subject: [PATCH 05/12] refactor: apply SMS defaults at consumer boundary --- functions/send-sms/README.md | 7 + functions/send-sms/__tests__/config.test.ts | 136 +++++++++++++++++++ functions/send-sms/__tests__/handler.test.ts | 67 +++++++++ functions/send-sms/config.ts | 103 +++++++++++++- functions/send-sms/handler.json | 1 + functions/send-sms/handler.ts | 2 +- functions/send-sms/providers/factory.ts | 11 +- pnpm-lock.yaml | 11 +- 8 files changed, 321 insertions(+), 17 deletions(-) create mode 100644 functions/send-sms/__tests__/config.test.ts diff --git a/functions/send-sms/README.md b/functions/send-sms/README.md index 71392e47d..6a181e14f 100644 --- a/functions/send-sms/README.md +++ b/functions/send-sms/README.md @@ -10,6 +10,13 @@ Current provider support is intentionally local-only: All SMS configuration is loaded through `@constructive-io/graphql-env` via `getEnvOptions({}, process.cwd(), context.env)`. The handler must not read `SMS_*` or `DEVSMS_*` values directly. +The send-sms consumer resolves defaults only after `graphql-env` has merged config files and environment overrides: + +- `SMS_REQUEST_TIMEOUT_MS` has an all-environment default of `5000`. +- `SEND_SMS_DRY_RUN` defaults to `false` in development and tests. +- Production must explicitly configure `SEND_SMS_DRY_RUN=true` or `false`. +- The provider and provider-specific settings have no global defaults. + ## Retry and idempotency The job worker may retry a job after a timeout or provider error. The `SmsSendRequest.metadata` includes `jobId` and `databaseId` for future idempotency support, but DevSms does not currently expose an idempotency key. A retried job can therefore create duplicate local SMS messages. The handler intentionally does not implement its own retry loop; timeout and transient failures are left to the existing job retry mechanism. diff --git a/functions/send-sms/__tests__/config.test.ts b/functions/send-sms/__tests__/config.test.ts new file mode 100644 index 000000000..989cf1243 --- /dev/null +++ b/functions/send-sms/__tests__/config.test.ts @@ -0,0 +1,136 @@ +const mockGetEnvOptions = jest.fn(); + +jest.mock('@constructive-io/graphql-env', () => ({ + getEnvOptions: mockGetEnvOptions +}), { virtual: true }); + +import { loadSmsOptions } from '../config'; + +describe('send-sms configuration', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each(['development', 'test'])( + 'applies consumer defaults in %s', + (nodeEnv) => { + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'devsms', + devsms: { + baseUrl: 'http://devsms:4000' + } + } + }); + + expect(loadSmsOptions({ NODE_ENV: nodeEnv }, '/tmp/send-sms')).toEqual({ + provider: 'devsms', + requestTimeoutMs: 5000, + dryRun: false, + devsms: { + baseUrl: 'http://devsms:4000' + } + }); + } + ); + + it('requires an explicit dry-run choice in production', () => { + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'twilio' + } + }); + + expect(() => + loadSmsOptions({ NODE_ENV: 'production' }, '/tmp/send-sms') + ).toThrow('SEND_SMS_DRY_RUN'); + }); + + it('rejects a malformed raw dry-run override before it can become false', () => { + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'twilio', + dryRun: false + } + }); + + expect(() => + loadSmsOptions( + { + NODE_ENV: 'production', + SEND_SMS_DRY_RUN: 'treu' + }, + '/tmp/send-sms' + ) + ).toThrow('SEND_SMS_DRY_RUN'); + expect(mockGetEnvOptions).not.toHaveBeenCalled(); + }); + + it.each(['not-a-number', '5s'])( + 'rejects malformed raw timeout %s instead of applying the fallback', + (timeout) => { + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'devsms', + requestTimeoutMs: 5000, + dryRun: false + } + }); + + expect(() => + loadSmsOptions( + { + NODE_ENV: 'development', + SMS_REQUEST_TIMEOUT_MS: timeout + }, + '/tmp/send-sms' + ) + ).toThrow('SMS_REQUEST_TIMEOUT_MS'); + expect(mockGetEnvOptions).not.toHaveBeenCalled(); + } + ); + + it('preserves an explicit false dry-run value in production', () => { + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'twilio', + dryRun: false + } + }); + + expect( + loadSmsOptions({ NODE_ENV: 'production' }, '/tmp/send-sms') + ).toEqual({ + provider: 'twilio', + requestTimeoutMs: 5000, + dryRun: false + }); + }); + + it('preserves the final values merged by graphql-env', () => { + const inputEnv = { NODE_ENV: 'test' }; + const cwd = '/tmp/send-sms-config'; + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'custom', + senderId: 'ConfiguredSender', + requestTimeoutMs: 3200, + dryRun: true, + devsms: { + baseUrl: 'http://configured-devsms:4000' + } + } + }); + + expect(loadSmsOptions(inputEnv, cwd)).toEqual({ + provider: 'custom', + senderId: 'ConfiguredSender', + requestTimeoutMs: 3200, + dryRun: true, + devsms: { + baseUrl: 'http://configured-devsms:4000' + } + }); + expect(mockGetEnvOptions).toHaveBeenCalledWith({}, cwd, inputEnv); + }); +}); diff --git a/functions/send-sms/__tests__/handler.test.ts b/functions/send-sms/__tests__/handler.test.ts index 51e2cc663..5cfd1a406 100644 --- a/functions/send-sms/__tests__/handler.test.ts +++ b/functions/send-sms/__tests__/handler.test.ts @@ -180,6 +180,73 @@ describe('send-sms handler', () => { setTimeoutSpy.mockRestore(); }); + it('preserves a configured request timeout after graphql-env merging', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: smsConfig({ requestTimeoutMs: 1234 }) + }); + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); + + await handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, createMockContext() as any); + + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 1234); + setTimeoutSpy.mockRestore(); + }); + + it('rejects non-positive request timeouts', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: smsConfig({ requestTimeoutMs: 0 }) + }); + + await expect(handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, createMockContext() as any)).rejects.toThrow( + 'options.sms.requestTimeoutMs must be a positive integer' + ); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('requires an explicit dry-run choice in production', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: { + provider: 'twilio', + requestTimeoutMs: 5000 + } + }); + const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); + + await expect(handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, ctx as any)).rejects.toThrow('SEND_SMS_DRY_RUN'); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('accepts an explicit production dry-run without provider configuration', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: { + requestTimeoutMs: 5000, + dryRun: true + } + }); + const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); + + const result = await handler({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456' + }, ctx as any); + + expect(result).toEqual({ complete: true, dryRun: true }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + it('rejects devsms in production', async () => { const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); diff --git a/functions/send-sms/config.ts b/functions/send-sms/config.ts index 9f57252d7..c962a035c 100644 --- a/functions/send-sms/config.ts +++ b/functions/send-sms/config.ts @@ -1,4 +1,11 @@ import { getEnvOptions } from '@constructive-io/graphql-env'; +import { + bool, + devDefault, + env as validateEnv, + makeValidator, + withDefault +} from '12factor-env'; export type SmsOptions = { provider?: string; @@ -10,14 +17,102 @@ export type SmsOptions = { }; }; +export type ResolvedSmsOptions = Omit & { + requestTimeoutMs: number; + dryRun: boolean; +}; + type ConstructiveOptionsWithSms = { sms?: SmsOptions; }; +const DEFAULT_SMS_REQUEST_TIMEOUT_MS = 5000; + +const positiveInteger = makeValidator((value: string) => { + const raw = value as unknown; + if (typeof raw === 'number' && Number.isSafeInteger(raw) && raw > 0) { + return raw; + } + + const normalized = String(raw).trim(); + if (!/^[1-9]\d*$/.test(normalized)) { + throw new Error( + 'options.sms.requestTimeoutMs must be a positive integer' + ); + } + + const parsed = Number(normalized); + if (!Number.isSafeInteger(parsed)) { + throw new Error( + 'options.sms.requestTimeoutMs must be a positive integer' + ); + } + return parsed; +}); + +const toEnvString = (value: number | boolean | undefined): string | undefined => + value === undefined ? undefined : String(value); + +/** + * Validate explicitly supplied overrides before graphql-env's lenient parsers + * can collapse a typo such as SEND_SMS_DRY_RUN=treu into false. These values are + * not used to resolve config here; getEnvOptions remains the source of truth for + * config-file and environment precedence. + */ +const validateRawSmsOverrides = ( + inputEnv: Record +): void => { + validateEnv( + inputEnv, + {}, + { + SMS_REQUEST_TIMEOUT_MS: positiveInteger({ default: undefined }), + SEND_SMS_DRY_RUN: bool({ default: undefined }) + } + ); +}; + +/** + * Apply defaults and production requirements only after graphql-env has merged + * pgpm.json, environment variables, and runtime overrides. 12factor-env accepts + * flat string inputs, so the final typed SMS values are adapted back to that + * boundary rather than reparsing the raw SMS_* variables. + */ +export const resolveSmsOptions = ( + options: SmsOptions, + inputEnv: Record +): ResolvedSmsOptions => { + const resolved = validateEnv( + { + NODE_ENV: inputEnv.NODE_ENV, + GITHUB_ACTIONS: inputEnv.GITHUB_ACTIONS, + SMS_REQUEST_TIMEOUT_MS: toEnvString(options.requestTimeoutMs), + SEND_SMS_DRY_RUN: toEnvString(options.dryRun) + }, + {}, + { + SMS_REQUEST_TIMEOUT_MS: withDefault( + positiveInteger, + DEFAULT_SMS_REQUEST_TIMEOUT_MS + ), + // Local and test environments send to explicitly configured development + // providers by default. Production must explicitly choose true or false. + SEND_SMS_DRY_RUN: devDefault(bool, false) + } + ); + + return { + ...options, + requestTimeoutMs: resolved.SMS_REQUEST_TIMEOUT_MS, + dryRun: resolved.SEND_SMS_DRY_RUN + }; +}; + export const loadSmsOptions = ( - env: Record, + inputEnv: Record, cwd: string = process.cwd() -): SmsOptions => { - const options = getEnvOptions({}, cwd, env) as ConstructiveOptionsWithSms; - return options.sms ?? {}; +): ResolvedSmsOptions => { + validateRawSmsOverrides(inputEnv); + const options = getEnvOptions({}, cwd, inputEnv) as ConstructiveOptionsWithSms; + return resolveSmsOptions(options.sms ?? {}, inputEnv); }; diff --git a/functions/send-sms/handler.json b/functions/send-sms/handler.json index 20c1f2ce4..4261f481e 100644 --- a/functions/send-sms/handler.json +++ b/functions/send-sms/handler.json @@ -7,6 +7,7 @@ "description": "Sends SMS verification codes from job payloads", "dependencies": { "@constructive-io/graphql-env": "^3.13.0", + "12factor-env": "^1.17.1", "libphonenumber-js": "^1.13.8" } } diff --git a/functions/send-sms/handler.ts b/functions/send-sms/handler.ts index e6d449e14..ae0c3821e 100644 --- a/functions/send-sms/handler.ts +++ b/functions/send-sms/handler.ts @@ -85,7 +85,7 @@ const handler: FunctionHandler = async (params, context) => { const databaseId = context.job?.databaseId; const maskedPhone = maskPhone(normalized.to); const providerName = options.provider; - const dryRun = options.dryRun ?? false; + const dryRun = options.dryRun; assertProviderAllowed(options, { nodeEnv: context.env.NODE_ENV }); diff --git a/functions/send-sms/providers/factory.ts b/functions/send-sms/providers/factory.ts index ff1333d21..fffac09d3 100644 --- a/functions/send-sms/providers/factory.ts +++ b/functions/send-sms/providers/factory.ts @@ -1,4 +1,4 @@ -import type { SmsOptions } from '../config'; +import type { ResolvedSmsOptions, SmsOptions } from '../config'; import { DevSmsProvider } from './devsms'; import type { SmsProvider } from './types'; @@ -7,11 +7,6 @@ export type SmsProviderFactoryOptions = { fetchImpl?: typeof fetch; }; -const DEFAULT_REQUEST_TIMEOUT_MS = 5000; - -const getRequestTimeoutMs = (options: SmsOptions): number => - options.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS; - export const assertProviderAllowed = ( options: SmsOptions, factoryOptions: SmsProviderFactoryOptions = {} @@ -22,7 +17,7 @@ export const assertProviderAllowed = ( }; export const createSmsProvider = ( - options: SmsOptions, + options: ResolvedSmsOptions, factoryOptions: SmsProviderFactoryOptions = {} ): SmsProvider => { assertProviderAllowed(options, factoryOptions); @@ -39,7 +34,7 @@ export const createSmsProvider = ( } return new DevSmsProvider({ baseUrl, - requestTimeoutMs: getRequestTimeoutMs(options), + requestTimeoutMs: options.requestTimeoutMs, fetchImpl: factoryOptions.fetchImpl }); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ed2ba31d3..746ff8570 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -106,6 +106,9 @@ importers: generated/send-sms: dependencies: + 12factor-env: + specifier: ^1.17.1 + version: 1.17.1 '@constructive-io/fn-runtime': specifier: workspace:^ version: link:../../packages/fn-runtime @@ -467,8 +470,8 @@ importers: packages: - 12factor-env@1.6.2: - resolution: {integrity: sha512-U4EO6sy9Cc6h1ST3hhLD2rc2s4LERxProove3XZ52rMq2rTo5uTKWNKwD2OYDUwqNij+p5SgjmpPO6L/Gqtizw==} + 12factor-env@1.17.1: + resolution: {integrity: sha512-IBDbA96dDGAkfzsSvZuveDhKIKQfFG+4lZ6KH77zd3GmqIJo+5lyXeoXL3MsRUSNKZ0ZqBr0M4oSoPrgfjIHJA==} '@babel/code-frame@7.27.1': resolution: {integrity: sha512-cjQ7ZlQ0Mv3b47hABuTevyTuYN4i+loJKGeV9flcCgIK37cCXRh+L1bd3iBHlynerhQ7BhCkn2BPbQUL+rGqFg==} @@ -3775,7 +3778,7 @@ packages: snapshots: - 12factor-env@1.6.2: + 12factor-env@1.17.1: dependencies: envalid: 8.1.1 @@ -4022,7 +4025,7 @@ snapshots: '@constructive-io/postmaster@1.6.2': dependencies: - 12factor-env: 1.6.2 + 12factor-env: 1.17.1 form-data: 4.0.5 mailgun.js: 10.4.0 transitivePeerDependencies: From 9ac90e557d10b0c2cea0ff6e9a9218a8d9e11b53 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 24 Jul 2026 16:11:07 +0800 Subject: [PATCH 06/12] docs: mark temporary SMS env compatibility --- functions/send-sms/README.md | 9 +++++++++ functions/send-sms/__tests__/config.test.ts | 2 ++ functions/send-sms/__tests__/handler.test.ts | 2 ++ functions/send-sms/config.ts | 8 ++++++++ 4 files changed, 21 insertions(+) diff --git a/functions/send-sms/README.md b/functions/send-sms/README.md index 6a181e14f..3dabd4bd7 100644 --- a/functions/send-sms/README.md +++ b/functions/send-sms/README.md @@ -17,6 +17,15 @@ The send-sms consumer resolves defaults only after `graphql-env` has merged conf - Production must explicitly configure `SEND_SMS_DRY_RUN=true` or `false`. - The provider and provider-specific settings have no global defaults. + +## Temporary cross-PR cleanup + +`constructive#1383` is merged, but its SMS-aware `graphql-env` and `graphql-types` builds do not yet have new npm versions. After those versions are published: + +1. Upgrade `@constructive-io/graphql-env` in `handler.json` and regenerate the lockfile. +2. Replace the local `SmsOptions` compatibility types and cast in `config.ts` with the published types. +3. Replace the virtual config mock with a real package integration test and remove the handler test's `virtual: true`. + ## Retry and idempotency The job worker may retry a job after a timeout or provider error. The `SmsSendRequest.metadata` includes `jobId` and `databaseId` for future idempotency support, but DevSms does not currently expose an idempotency key. A retried job can therefore create duplicate local SMS messages. The handler intentionally does not implement its own retry loop; timeout and transient failures are left to the existing job retry mechanism. diff --git a/functions/send-sms/__tests__/config.test.ts b/functions/send-sms/__tests__/config.test.ts index 989cf1243..be2a5b652 100644 --- a/functions/send-sms/__tests__/config.test.ts +++ b/functions/send-sms/__tests__/config.test.ts @@ -1,5 +1,7 @@ const mockGetEnvOptions = jest.fn(); +// TEMP-DEVSMS-CROSS-PR: Replace this virtual mock with coverage against the real +// SMS-aware graphql-env package after the first post-constructive#1383 release. jest.mock('@constructive-io/graphql-env', () => ({ getEnvOptions: mockGetEnvOptions }), { virtual: true }); diff --git a/functions/send-sms/__tests__/handler.test.ts b/functions/send-sms/__tests__/handler.test.ts index 5cfd1a406..e6ffc300f 100644 --- a/functions/send-sms/__tests__/handler.test.ts +++ b/functions/send-sms/__tests__/handler.test.ts @@ -3,6 +3,8 @@ import { createMockContext } from '../../../tests/helpers/mock-context'; const mockGetEnvOptions = jest.fn(); +// TEMP-DEVSMS-CROSS-PR: Drop `virtual: true` after handler.json is upgraded to +// the first published graphql-env version containing constructive#1383. jest.mock('@constructive-io/graphql-env', () => ({ getEnvOptions: mockGetEnvOptions }), { virtual: true }); diff --git a/functions/send-sms/config.ts b/functions/send-sms/config.ts index c962a035c..a6f5bc18a 100644 --- a/functions/send-sms/config.ts +++ b/functions/send-sms/config.ts @@ -7,6 +7,9 @@ import { withDefault } from '12factor-env'; +// TEMP-DEVSMS-CROSS-PR: Replace this local copy with SmsOptions imported from +// @constructive-io/graphql-env after a post-constructive#1383 package version +// containing the SMS contract has been published. export type SmsOptions = { provider?: string; senderId?: string; @@ -22,6 +25,8 @@ export type ResolvedSmsOptions = Omit dryRun: boolean; }; +// TEMP-DEVSMS-CROSS-PR: Delete this compatibility type and the matching cast in +// loadSmsOptions once the published getEnvOptions return type includes `sms`. type ConstructiveOptionsWithSms = { sms?: SmsOptions; }; @@ -113,6 +118,9 @@ export const loadSmsOptions = ( cwd: string = process.cwd() ): ResolvedSmsOptions => { validateRawSmsOverrides(inputEnv); + // TEMP-DEVSMS-CROSS-PR: Remove this cast together with + // ConstructiveOptionsWithSms after upgrading graphql-env to the first + // post-constructive#1383 release. const options = getEnvOptions({}, cwd, inputEnv) as ConstructiveOptionsWithSms; return resolveSmsOptions(options.sms ?? {}, inputEnv); }; From e7c4df231d2bd667fe8159fa4d50f0fafa95874b Mon Sep 17 00:00:00 2001 From: zetazzz Date: Mon, 27 Jul 2026 12:58:01 +0800 Subject: [PATCH 07/12] refactor: use published SMS configuration --- functions/send-sms/README.md | 9 -- functions/send-sms/__tests__/config.test.ts | 141 +++++++++---------- functions/send-sms/__tests__/handler.test.ts | 4 +- functions/send-sms/config.ts | 64 ++------- functions/send-sms/handler.json | 4 +- jest.config.ts | 1 - pnpm-lock.yaml | 71 ++++++---- tests/__mocks__/@pgpmjs/env.ts | 17 --- 8 files changed, 128 insertions(+), 183 deletions(-) delete mode 100644 tests/__mocks__/@pgpmjs/env.ts diff --git a/functions/send-sms/README.md b/functions/send-sms/README.md index 3dabd4bd7..6a181e14f 100644 --- a/functions/send-sms/README.md +++ b/functions/send-sms/README.md @@ -17,15 +17,6 @@ The send-sms consumer resolves defaults only after `graphql-env` has merged conf - Production must explicitly configure `SEND_SMS_DRY_RUN=true` or `false`. - The provider and provider-specific settings have no global defaults. - -## Temporary cross-PR cleanup - -`constructive#1383` is merged, but its SMS-aware `graphql-env` and `graphql-types` builds do not yet have new npm versions. After those versions are published: - -1. Upgrade `@constructive-io/graphql-env` in `handler.json` and regenerate the lockfile. -2. Replace the local `SmsOptions` compatibility types and cast in `config.ts` with the published types. -3. Replace the virtual config mock with a real package integration test and remove the handler test's `virtual: true`. - ## Retry and idempotency The job worker may retry a job after a timeout or provider error. The `SmsSendRequest.metadata` includes `jobId` and `databaseId` for future idempotency support, but DevSms does not currently expose an idempotency key. A retried job can therefore create duplicate local SMS messages. The handler intentionally does not implement its own retry loop; timeout and transient failures are left to the existing job retry mechanism. diff --git a/functions/send-sms/__tests__/config.test.ts b/functions/send-sms/__tests__/config.test.ts index be2a5b652..fb814116a 100644 --- a/functions/send-sms/__tests__/config.test.ts +++ b/functions/send-sms/__tests__/config.test.ts @@ -1,31 +1,33 @@ -const mockGetEnvOptions = jest.fn(); - -// TEMP-DEVSMS-CROSS-PR: Replace this virtual mock with coverage against the real -// SMS-aware graphql-env package after the first post-constructive#1383 release. -jest.mock('@constructive-io/graphql-env', () => ({ - getEnvOptions: mockGetEnvOptions -}), { virtual: true }); +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { loadSmsOptions } from '../config'; describe('send-sms configuration', () => { + let tempDir: string; + beforeEach(() => { - jest.clearAllMocks(); + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'send-sms-config-')); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); }); it.each(['development', 'test'])( - 'applies consumer defaults in %s', + 'loads published graphql-env values and applies consumer defaults in %s', (nodeEnv) => { - mockGetEnvOptions.mockReturnValue({ - sms: { - provider: 'devsms', - devsms: { - baseUrl: 'http://devsms:4000' - } - } - }); - - expect(loadSmsOptions({ NODE_ENV: nodeEnv }, '/tmp/send-sms')).toEqual({ + expect( + loadSmsOptions( + { + NODE_ENV: nodeEnv, + SMS_PROVIDER: 'devsms', + DEVSMS_BASE_URL: 'http://devsms:4000' + }, + tempDir + ) + ).toEqual({ provider: 'devsms', requestTimeoutMs: 5000, dryRun: false, @@ -37,71 +39,64 @@ describe('send-sms configuration', () => { ); it('requires an explicit dry-run choice in production', () => { - mockGetEnvOptions.mockReturnValue({ - sms: { - provider: 'twilio' - } - }); - expect(() => - loadSmsOptions({ NODE_ENV: 'production' }, '/tmp/send-sms') + loadSmsOptions( + { + NODE_ENV: 'production', + SMS_PROVIDER: 'twilio' + }, + tempDir + ) ).toThrow('SEND_SMS_DRY_RUN'); }); - it('rejects a malformed raw dry-run override before it can become false', () => { - mockGetEnvOptions.mockReturnValue({ - sms: { - provider: 'twilio', - dryRun: false - } - }); - - expect(() => + it('uses graphql-env boolean coercion as the configuration source of truth', () => { + expect( loadSmsOptions( { NODE_ENV: 'production', + SMS_PROVIDER: 'twilio', SEND_SMS_DRY_RUN: 'treu' }, - '/tmp/send-sms' + tempDir ) - ).toThrow('SEND_SMS_DRY_RUN'); - expect(mockGetEnvOptions).not.toHaveBeenCalled(); + ).toEqual({ + provider: 'twilio', + requestTimeoutMs: 5000, + dryRun: false + }); }); it.each(['not-a-number', '5s'])( - 'rejects malformed raw timeout %s instead of applying the fallback', + 'lets graphql-env omit malformed timeout %s before applying the fallback', (timeout) => { - mockGetEnvOptions.mockReturnValue({ - sms: { - provider: 'devsms', - requestTimeoutMs: 5000, - dryRun: false - } - }); - - expect(() => + expect( loadSmsOptions( { NODE_ENV: 'development', + SMS_PROVIDER: 'devsms', SMS_REQUEST_TIMEOUT_MS: timeout }, - '/tmp/send-sms' + tempDir ) - ).toThrow('SMS_REQUEST_TIMEOUT_MS'); - expect(mockGetEnvOptions).not.toHaveBeenCalled(); + ).toEqual({ + provider: 'devsms', + requestTimeoutMs: 5000, + dryRun: false + }); } ); it('preserves an explicit false dry-run value in production', () => { - mockGetEnvOptions.mockReturnValue({ - sms: { - provider: 'twilio', - dryRun: false - } - }); - expect( - loadSmsOptions({ NODE_ENV: 'production' }, '/tmp/send-sms') + loadSmsOptions( + { + NODE_ENV: 'production', + SMS_PROVIDER: 'twilio', + SEND_SMS_DRY_RUN: 'false' + }, + tempDir + ) ).toEqual({ provider: 'twilio', requestTimeoutMs: 5000, @@ -110,21 +105,22 @@ describe('send-sms configuration', () => { }); it('preserves the final values merged by graphql-env', () => { - const inputEnv = { NODE_ENV: 'test' }; - const cwd = '/tmp/send-sms-config'; - mockGetEnvOptions.mockReturnValue({ - sms: { - provider: 'custom', - senderId: 'ConfiguredSender', - requestTimeoutMs: 3200, - dryRun: true, - devsms: { - baseUrl: 'http://configured-devsms:4000' + fs.writeFileSync( + path.join(tempDir, 'pgpm.json'), + JSON.stringify({ + sms: { + provider: 'custom', + senderId: 'ConfiguredSender', + requestTimeoutMs: 3200, + dryRun: true, + devsms: { + baseUrl: 'http://configured-devsms:4000' + } } - } - }); + }) + ); - expect(loadSmsOptions(inputEnv, cwd)).toEqual({ + expect(loadSmsOptions({ NODE_ENV: 'test' }, tempDir)).toEqual({ provider: 'custom', senderId: 'ConfiguredSender', requestTimeoutMs: 3200, @@ -133,6 +129,5 @@ describe('send-sms configuration', () => { baseUrl: 'http://configured-devsms:4000' } }); - expect(mockGetEnvOptions).toHaveBeenCalledWith({}, cwd, inputEnv); }); }); diff --git a/functions/send-sms/__tests__/handler.test.ts b/functions/send-sms/__tests__/handler.test.ts index e6ffc300f..5617211ef 100644 --- a/functions/send-sms/__tests__/handler.test.ts +++ b/functions/send-sms/__tests__/handler.test.ts @@ -3,11 +3,9 @@ import { createMockContext } from '../../../tests/helpers/mock-context'; const mockGetEnvOptions = jest.fn(); -// TEMP-DEVSMS-CROSS-PR: Drop `virtual: true` after handler.json is upgraded to -// the first published graphql-env version containing constructive#1383. jest.mock('@constructive-io/graphql-env', () => ({ getEnvOptions: mockGetEnvOptions -}), { virtual: true }); +})); const smsConfig = (overrides: Record = {}) => ({ provider: 'devsms', diff --git a/functions/send-sms/config.ts b/functions/send-sms/config.ts index a6f5bc18a..5299b3486 100644 --- a/functions/send-sms/config.ts +++ b/functions/send-sms/config.ts @@ -1,4 +1,4 @@ -import { getEnvOptions } from '@constructive-io/graphql-env'; +import { getEnvOptions, type SmsOptions } from '@constructive-io/graphql-env'; import { bool, devDefault, @@ -7,30 +7,16 @@ import { withDefault } from '12factor-env'; -// TEMP-DEVSMS-CROSS-PR: Replace this local copy with SmsOptions imported from -// @constructive-io/graphql-env after a post-constructive#1383 package version -// containing the SMS contract has been published. -export type SmsOptions = { - provider?: string; - senderId?: string; - requestTimeoutMs?: number; - dryRun?: boolean; - devsms?: { - baseUrl?: string; - }; -}; +export type { SmsOptions } from '@constructive-io/graphql-env'; -export type ResolvedSmsOptions = Omit & { +export type ResolvedSmsOptions = Omit< + SmsOptions, + 'requestTimeoutMs' | 'dryRun' +> & { requestTimeoutMs: number; dryRun: boolean; }; -// TEMP-DEVSMS-CROSS-PR: Delete this compatibility type and the matching cast in -// loadSmsOptions once the published getEnvOptions return type includes `sms`. -type ConstructiveOptionsWithSms = { - sms?: SmsOptions; -}; - const DEFAULT_SMS_REQUEST_TIMEOUT_MS = 5000; const positiveInteger = makeValidator((value: string) => { @@ -41,41 +27,19 @@ const positiveInteger = makeValidator((value: string) => { const normalized = String(raw).trim(); if (!/^[1-9]\d*$/.test(normalized)) { - throw new Error( - 'options.sms.requestTimeoutMs must be a positive integer' - ); + throw new Error('options.sms.requestTimeoutMs must be a positive integer'); } const parsed = Number(normalized); if (!Number.isSafeInteger(parsed)) { - throw new Error( - 'options.sms.requestTimeoutMs must be a positive integer' - ); + throw new Error('options.sms.requestTimeoutMs must be a positive integer'); } return parsed; }); -const toEnvString = (value: number | boolean | undefined): string | undefined => - value === undefined ? undefined : String(value); - -/** - * Validate explicitly supplied overrides before graphql-env's lenient parsers - * can collapse a typo such as SEND_SMS_DRY_RUN=treu into false. These values are - * not used to resolve config here; getEnvOptions remains the source of truth for - * config-file and environment precedence. - */ -const validateRawSmsOverrides = ( - inputEnv: Record -): void => { - validateEnv( - inputEnv, - {}, - { - SMS_REQUEST_TIMEOUT_MS: positiveInteger({ default: undefined }), - SEND_SMS_DRY_RUN: bool({ default: undefined }) - } - ); -}; +const toEnvString = ( + value: number | boolean | undefined +): string | undefined => (value === undefined ? undefined : String(value)); /** * Apply defaults and production requirements only after graphql-env has merged @@ -117,10 +81,6 @@ export const loadSmsOptions = ( inputEnv: Record, cwd: string = process.cwd() ): ResolvedSmsOptions => { - validateRawSmsOverrides(inputEnv); - // TEMP-DEVSMS-CROSS-PR: Remove this cast together with - // ConstructiveOptionsWithSms after upgrading graphql-env to the first - // post-constructive#1383 release. - const options = getEnvOptions({}, cwd, inputEnv) as ConstructiveOptionsWithSms; + const options = getEnvOptions({}, cwd, inputEnv); return resolveSmsOptions(options.sms ?? {}, inputEnv); }; diff --git a/functions/send-sms/handler.json b/functions/send-sms/handler.json index 4261f481e..e22517473 100644 --- a/functions/send-sms/handler.json +++ b/functions/send-sms/handler.json @@ -6,8 +6,8 @@ "taskIdentifier": "sms:send_verification_code", "description": "Sends SMS verification codes from job payloads", "dependencies": { - "@constructive-io/graphql-env": "^3.13.0", - "12factor-env": "^1.17.1", + "@constructive-io/graphql-env": "^3.20.0", + "12factor-env": "^1.18.2", "libphonenumber-js": "^1.13.8" } } diff --git a/jest.config.ts b/jest.config.ts index da557974d..c3a8ea324 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -17,7 +17,6 @@ const config: Config = { moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], modulePathIgnorePatterns: ['dist/', 'generated/'], moduleNameMapper: { - '^@pgpmjs/env$': '/tests/__mocks__/@pgpmjs/env', '^@pgpmjs/logger$': '/tests/__mocks__/@pgpmjs/logger', '^@constructive-io/postmaster$': '/tests/__mocks__/@constructive-io/postmaster', diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 746ff8570..f5f473ea6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -107,14 +107,14 @@ importers: generated/send-sms: dependencies: 12factor-env: - specifier: ^1.17.1 - version: 1.17.1 + specifier: ^1.18.2 + version: 1.18.2 '@constructive-io/fn-runtime': specifier: workspace:^ version: link:../../packages/fn-runtime '@constructive-io/graphql-env': - specifier: ^3.13.0 - version: 3.15.0 + specifier: ^3.20.0 + version: 3.20.0 libphonenumber-js: specifier: ^1.13.8 version: 1.13.8 @@ -473,6 +473,9 @@ packages: 12factor-env@1.17.1: resolution: {integrity: sha512-IBDbA96dDGAkfzsSvZuveDhKIKQfFG+4lZ6KH77zd3GmqIJo+5lyXeoXL3MsRUSNKZ0ZqBr0M4oSoPrgfjIHJA==} + 12factor-env@1.18.2: + resolution: {integrity: sha512-lW+ytI7TxOW2c7n6U2xr2yEENtj/npHffM8nPUpdxmORhs69asLTb9X469AY9h41HOdulZbKL1cDTxeHUkEX6w==} + '@babel/code-frame@7.27.1': resolution: {integrity: sha512-cjQ7ZlQ0Mv3b47hABuTevyTuYN4i+loJKGeV9flcCgIK37cCXRh+L1bd3iBHlynerhQ7BhCkn2BPbQUL+rGqFg==} engines: {node: '>=6.9.0'} @@ -650,11 +653,11 @@ packages: '@bcoe/v8-coverage@0.2.3': resolution: {integrity: sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==} - '@constructive-io/graphql-env@3.15.0': - resolution: {integrity: sha512-twK195FV69XUwLCQ8ifYxALiGF157N424YUzZhKP52DNmHjBY2GY1hLj6SM5K2t7+LrOFJPNeo7U/d5I9yK+LQ==} + '@constructive-io/graphql-env@3.20.0': + resolution: {integrity: sha512-gUQbLligxQEFxusUk/yiSkm6M06KYsiHKIdQ4cYgPwGkWoug2U+X7UL+JuqZSUordl7xFINY4pyWPwlOVEM47Q==} - '@constructive-io/graphql-types@3.14.0': - resolution: {integrity: sha512-ATT9HIXD4Qa+WuayaAWzwvbgNBlU1ssCjUroo7zbZsSxXX7pgSDK1KSk/jmCldYIYLez19xgXx3QI7rlP6A/iQ==} + '@constructive-io/graphql-types@3.19.0': + resolution: {integrity: sha512-Gt47e8AHlBdx2r3xbBUUtBtpXqPl9KAWHX+9npQZR+4m/5fIR29k+w7zz4ZqAF6tSpmZjM1umyrN1La2foJMyA==} '@constructive-io/job-pg@2.5.4': resolution: {integrity: sha512-cjJxL/P1g4s07PiXiw31jb7c56aQmdPur+WyQXC0lAig1DIFSeJqbQjY6PXaFQepxgivwIBf9P9v+EalQaOtPQ==} @@ -1113,8 +1116,8 @@ packages: '@pgpmjs/env@2.17.0': resolution: {integrity: sha512-3WPwJ4prFWGGIRzyR52/JG84hM+Qe6lVtQ+bcCpGnGuhukFowALpaegRZxi3LT/pO6D8wW1Y3nW9LugfJLO6KQ==} - '@pgpmjs/env@2.27.0': - resolution: {integrity: sha512-/JTHMBx7X7bnoy5kPCNDgu2eWNkjrBaeIHqlXErcErldt1RMEMjLYQykCYW5QdOAUP8RKS6pIzszvatoROlkAA==} + '@pgpmjs/env@2.30.2': + resolution: {integrity: sha512-ZxQH/Ck4zYt5sIK+qRbmlv++6J38wRcFgw927kFuf01UZwzYa7Cb7KMM0nDNNRaLQ/l/tVNLsAZXD3FNCFfN5g==} '@pgpmjs/logger@1.5.0': resolution: {integrity: sha512-R27o5MiOsezI5rAWdJyuOkWUK6zxr8Mg61hPs7uCu//sECoprR4/7CVeFIHwn7+gyrjUk0wBz0dQcJhjYzVDpw==} @@ -1131,8 +1134,8 @@ packages: '@pgpmjs/types@2.28.0': resolution: {integrity: sha512-XYCcWnxkIrZEHF2oxxtU1yMeMa4bfw5za5CsDnMx0uasdtG0Y5YwDqruuv0uzYdz0id927LMb6svE38vrmPTIg==} - '@pgpmjs/types@2.34.0': - resolution: {integrity: sha512-2Y6o1IyqfuaogcoIGOECuR33sl7XlLYGQzi5qD6CLLPwAgEXzQj3u6dxchuzWxnWaE3Sf6W+SwLStxx688k2nQ==} + '@pgpmjs/types@2.37.2': + resolution: {integrity: sha512-QscdUwDcqG4HfeTTDBWUPIyvTQSIdhDL36IPQDDyJJ+5N95ytga1nfBmAPfyhvTZOdXHOpno5WR6cirBjt7/0Q==} '@pkgjs/parseargs@0.11.0': resolution: {integrity: sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg==} @@ -1388,41 +1391,49 @@ packages: resolution: {integrity: sha512-34gw7PjDGB9JgePJEmhEqBhWvCiiWCuXsL9hYphDF7crW7UgI05gyBAi6MF58uGcMOiOqSJ2ybEeCvHcq0BCmQ==} cpu: [arm64] os: [linux] + libc: [glibc] '@unrs/resolver-binding-linux-arm64-musl@1.11.1': resolution: {integrity: sha512-RyMIx6Uf53hhOtJDIamSbTskA99sPHS96wxVE/bJtePJJtpdKGXO1wY90oRdXuYOGOTuqjT8ACccMc4K6QmT3w==} cpu: [arm64] os: [linux] + libc: [musl] '@unrs/resolver-binding-linux-ppc64-gnu@1.11.1': resolution: {integrity: sha512-D8Vae74A4/a+mZH0FbOkFJL9DSK2R6TFPC9M+jCWYia/q2einCubX10pecpDiTmkJVUH+y8K3BZClycD8nCShA==} cpu: [ppc64] os: [linux] + libc: [glibc] '@unrs/resolver-binding-linux-riscv64-gnu@1.11.1': resolution: {integrity: sha512-frxL4OrzOWVVsOc96+V3aqTIQl1O2TjgExV4EKgRY09AJ9leZpEg8Ak9phadbuX0BA4k8U5qtvMSQQGGmaJqcQ==} cpu: [riscv64] os: [linux] + libc: [glibc] '@unrs/resolver-binding-linux-riscv64-musl@1.11.1': resolution: {integrity: sha512-mJ5vuDaIZ+l/acv01sHoXfpnyrNKOk/3aDoEdLO/Xtn9HuZlDD6jKxHlkN8ZhWyLJsRBxfv9GYM2utQ1SChKew==} cpu: [riscv64] os: [linux] + libc: [musl] '@unrs/resolver-binding-linux-s390x-gnu@1.11.1': resolution: {integrity: sha512-kELo8ebBVtb9sA7rMe1Cph4QHreByhaZ2QEADd9NzIQsYNQpt9UkM9iqr2lhGr5afh885d/cB5QeTXSbZHTYPg==} cpu: [s390x] os: [linux] + libc: [glibc] '@unrs/resolver-binding-linux-x64-gnu@1.11.1': resolution: {integrity: sha512-C3ZAHugKgovV5YvAMsxhq0gtXuwESUKc5MhEtjBpLoHPLYM+iuwSj3lflFwK3DPm68660rZ7G8BMcwSro7hD5w==} cpu: [x64] os: [linux] + libc: [glibc] '@unrs/resolver-binding-linux-x64-musl@1.11.1': resolution: {integrity: sha512-rV0YSoyhK2nZ4vEswT/QwqzqQXw5I6CjoaYMOX0TqBlWhojUf8P94mvI7nuJTeaCkkds3QE4+zS8Ko+GdXuZtA==} cpu: [x64] os: [linux] + libc: [musl] '@unrs/resolver-binding-wasm32-wasi@1.11.1': resolution: {integrity: sha512-5u4RkfxJm+Ng7IWgkzi3qrFOvLvQYnPBmjmZQ8+szTK/b31fQCnleNl1GgEt7nIsZRIf5PLhPwT0WM+q45x/UQ==} @@ -2207,7 +2218,7 @@ packages: glob@7.2.3: resolution: {integrity: sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==} - deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me + deprecated: Glob versions prior to v9 are no longer supported globals@14.0.0: resolution: {integrity: sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ==} @@ -3147,8 +3158,8 @@ packages: pg-env@1.15.0: resolution: {integrity: sha512-1bs3pcNOOrA0om3TNJGwbZu7JXKc/tenyVW4KX6ljARxvKtRSUcGl6sbpKVCXJo+Y98W0nRvPgfa/SlqlumRsg==} - pg-env@1.18.0: - resolution: {integrity: sha512-RzZISyA7ZJlnKOurlkqX2k/1iJl5kqn39OlvUbn0wtX1ENYtfClFlbkzye+WFdw/s9VnPY0uSzPJPKcA+8gBxQ==} + pg-env@1.19.2: + resolution: {integrity: sha512-gNjwaKoNCS1zk0qF1Nb6EwhuW6R7kpQQo6IBqw/gz5rSPy7/vpYYz5HfdW4/vwEwd+/yRTkizBToI46wo5aweA==} pg-env@1.8.2: resolution: {integrity: sha512-YzxNQKZmFRRJKX5t149Ys2JoAsc6OCHcaoYH/82si7gwVC9ODaFTFtQn7gv3VpoGsNkH90t6iEPWvmLIgv2rDg==} @@ -3782,6 +3793,10 @@ snapshots: dependencies: envalid: 8.1.1 + 12factor-env@1.18.2: + dependencies: + envalid: 8.1.1 + '@babel/code-frame@7.27.1': dependencies: '@babel/helper-validator-identifier': 7.28.5 @@ -3979,20 +3994,21 @@ snapshots: '@bcoe/v8-coverage@0.2.3': {} - '@constructive-io/graphql-env@3.15.0': + '@constructive-io/graphql-env@3.20.0': dependencies: - '@constructive-io/graphql-types': 3.14.0 - '@pgpmjs/env': 2.27.0 + 12factor-env: 1.18.2 + '@constructive-io/graphql-types': 3.19.0 + '@pgpmjs/env': 2.30.2 deepmerge: 4.3.1 transitivePeerDependencies: - supports-color - '@constructive-io/graphql-types@3.14.0': + '@constructive-io/graphql-types@3.19.0': dependencies: - '@pgpmjs/types': 2.34.0 + '@pgpmjs/types': 2.37.2 deepmerge: 4.3.1 graphile-config: 1.0.1 - pg-env: 1.18.0 + pg-env: 1.19.2 transitivePeerDependencies: - supports-color @@ -4624,9 +4640,10 @@ snapshots: '@pgpmjs/types': 2.21.0 deepmerge: 4.3.1 - '@pgpmjs/env@2.27.0': + '@pgpmjs/env@2.30.2': dependencies: - '@pgpmjs/types': 2.34.0 + 12factor-env: 1.18.2 + '@pgpmjs/types': 2.37.2 deepmerge: 4.3.1 '@pgpmjs/logger@1.5.0': @@ -4649,9 +4666,9 @@ snapshots: dependencies: pg-env: 1.15.0 - '@pgpmjs/types@2.34.0': + '@pgpmjs/types@2.37.2': dependencies: - pg-env: 1.18.0 + pg-env: 1.19.2 '@pkgjs/parseargs@0.11.0': optional: true @@ -7378,7 +7395,9 @@ snapshots: pg-env@1.15.0: {} - pg-env@1.18.0: {} + pg-env@1.19.2: + dependencies: + 12factor-env: 1.18.2 pg-env@1.8.2: {} diff --git a/tests/__mocks__/@pgpmjs/env.ts b/tests/__mocks__/@pgpmjs/env.ts deleted file mode 100644 index 4d10ce8f6..000000000 --- a/tests/__mocks__/@pgpmjs/env.ts +++ /dev/null @@ -1,17 +0,0 @@ -export const parseEnvBoolean = ( - value: string | undefined -): boolean | undefined => { - if (value === undefined || value === '') return undefined; - const lower = value.toLowerCase(); - if (['true', '1', 'yes'].includes(lower)) return true; - if (['false', '0', 'no'].includes(lower)) return false; - return undefined; -}; - -export const parseEnvNumber = ( - value: string | undefined -): number | undefined => { - if (value === undefined || value === '') return undefined; - const num = Number(value); - return isNaN(num) ? undefined : num; -}; From 87aeeba69a9105e0f01e1ef32b024844372f9e86 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 28 Jul 2026 06:11:40 +0800 Subject: [PATCH 08/12] refactor: simplify send-sms handler --- functions/send-sms/README.md | 2 +- functions/send-sms/__tests__/config.test.ts | 190 ++++---- functions/send-sms/__tests__/devsms.test.ts | 134 ----- functions/send-sms/__tests__/handler.test.ts | 460 +++++++++++++----- functions/send-sms/__tests__/phone.test.ts | 25 - functions/send-sms/config.ts | 86 ---- functions/send-sms/handler.ts | 363 ++++++++++++-- functions/send-sms/phone.ts | 86 ---- functions/send-sms/providers/devsms.ts | 103 ---- functions/send-sms/providers/factory.ts | 47 -- functions/send-sms/providers/types.ts | 22 - functions/send-sms/templates.ts | 9 - packages/fn-generator/src/builders/package.ts | 39 +- scripts/generate.ts | 42 +- 14 files changed, 777 insertions(+), 831 deletions(-) delete mode 100644 functions/send-sms/__tests__/devsms.test.ts delete mode 100644 functions/send-sms/__tests__/phone.test.ts delete mode 100644 functions/send-sms/config.ts delete mode 100644 functions/send-sms/phone.ts delete mode 100644 functions/send-sms/providers/devsms.ts delete mode 100644 functions/send-sms/providers/factory.ts delete mode 100644 functions/send-sms/providers/types.ts delete mode 100644 functions/send-sms/templates.ts diff --git a/functions/send-sms/README.md b/functions/send-sms/README.md index 6a181e14f..c78692417 100644 --- a/functions/send-sms/README.md +++ b/functions/send-sms/README.md @@ -19,7 +19,7 @@ The send-sms consumer resolves defaults only after `graphql-env` has merged conf ## Retry and idempotency -The job worker may retry a job after a timeout or provider error. The `SmsSendRequest.metadata` includes `jobId` and `databaseId` for future idempotency support, but DevSms does not currently expose an idempotency key. A retried job can therefore create duplicate local SMS messages. The handler intentionally does not implement its own retry loop; timeout and transient failures are left to the existing job retry mechanism. +The job worker may retry a job after a timeout or provider error. DevSms does not currently expose an idempotency key, so a retried job can create duplicate local SMS messages. The handler intentionally does not implement its own retry loop; timeout and transient failures are left to the existing job retry mechanism. ## Logging diff --git a/functions/send-sms/__tests__/config.test.ts b/functions/send-sms/__tests__/config.test.ts index fb814116a..a9f04d197 100644 --- a/functions/send-sms/__tests__/config.test.ts +++ b/functions/send-sms/__tests__/config.test.ts @@ -2,132 +2,148 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { loadSmsOptions } from '../config'; +import { createMockContext } from '../../../tests/helpers/mock-context'; +import handler from '../handler'; -describe('send-sms configuration', () => { +const payload = { + sms_type: 'sms_otp_code' as const, + phone: '+14155550123', + code: '123456', +}; + +const successfulResponse = (): Response => + new Response( + JSON.stringify({ + provider_message_id: 'SM123', + status: 'queued', + }), + { + status: 201, + headers: { 'content-type': 'application/json' }, + } + ); + +const invoke = (env: Record) => + handler(payload, createMockContext({ env }) as any); + +describe('send-sms published configuration contract', () => { + const originalCwd = process.cwd(); let tempDir: string; beforeEach(() => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'send-sms-config-')); + process.chdir(tempDir); + global.fetch = jest + .fn() + .mockResolvedValue(successfulResponse()) as unknown as typeof fetch; + jest.spyOn(console, 'warn').mockImplementation(); }); afterEach(() => { + process.chdir(originalCwd); fs.rmSync(tempDir, { recursive: true, force: true }); + jest.restoreAllMocks(); }); it.each(['development', 'test'])( 'loads published graphql-env values and applies consumer defaults in %s', - (nodeEnv) => { - expect( - loadSmsOptions( - { - NODE_ENV: nodeEnv, - SMS_PROVIDER: 'devsms', - DEVSMS_BASE_URL: 'http://devsms:4000' - }, - tempDir - ) - ).toEqual({ + async (nodeEnv) => { + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); + + await expect( + invoke({ + NODE_ENV: nodeEnv, + SMS_PROVIDER: 'devsms', + DEVSMS_BASE_URL: 'http://devsms:4000', + }) + ).resolves.toEqual({ + complete: true, provider: 'devsms', - requestTimeoutMs: 5000, - dryRun: false, - devsms: { - baseUrl: 'http://devsms:4000' - } + messageId: 'SM123', + status: 'queued', }); + + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 5000); } ); - it('requires an explicit dry-run choice in production', () => { - expect(() => - loadSmsOptions( - { - NODE_ENV: 'production', - SMS_PROVIDER: 'twilio' - }, - tempDir - ) - ).toThrow('SEND_SMS_DRY_RUN'); + it('requires an explicit dry-run choice in production', async () => { + await expect( + invoke({ + NODE_ENV: 'production', + SMS_PROVIDER: 'twilio', + }) + ).rejects.toThrow('SEND_SMS_DRY_RUN'); + expect(global.fetch).not.toHaveBeenCalled(); }); - it('uses graphql-env boolean coercion as the configuration source of truth', () => { - expect( - loadSmsOptions( - { - NODE_ENV: 'production', - SMS_PROVIDER: 'twilio', - SEND_SMS_DRY_RUN: 'treu' - }, - tempDir - ) - ).toEqual({ - provider: 'twilio', - requestTimeoutMs: 5000, - dryRun: false - }); + it('uses graphql-env boolean coercion as the configuration source of truth', async () => { + await expect( + invoke({ + NODE_ENV: 'production', + SMS_PROVIDER: 'twilio', + SEND_SMS_DRY_RUN: 'treu', + }) + ).rejects.toThrow('Unsupported SMS provider: twilio'); + expect(global.fetch).not.toHaveBeenCalled(); }); it.each(['not-a-number', '5s'])( 'lets graphql-env omit malformed timeout %s before applying the fallback', - (timeout) => { - expect( - loadSmsOptions( - { - NODE_ENV: 'development', - SMS_PROVIDER: 'devsms', - SMS_REQUEST_TIMEOUT_MS: timeout - }, - tempDir - ) - ).toEqual({ - provider: 'devsms', - requestTimeoutMs: 5000, - dryRun: false + async (timeout) => { + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); + + await invoke({ + NODE_ENV: 'development', + SMS_PROVIDER: 'devsms', + DEVSMS_BASE_URL: 'http://devsms:4000', + SMS_REQUEST_TIMEOUT_MS: timeout, }); + + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 5000); } ); - it('preserves an explicit false dry-run value in production', () => { - expect( - loadSmsOptions( - { - NODE_ENV: 'production', - SMS_PROVIDER: 'twilio', - SEND_SMS_DRY_RUN: 'false' - }, - tempDir - ) - ).toEqual({ - provider: 'twilio', - requestTimeoutMs: 5000, - dryRun: false - }); + it('preserves an explicit false dry-run value in production', async () => { + await expect( + invoke({ + NODE_ENV: 'production', + SMS_PROVIDER: 'twilio', + SEND_SMS_DRY_RUN: 'false', + }) + ).rejects.toThrow('Unsupported SMS provider: twilio'); + expect(global.fetch).not.toHaveBeenCalled(); }); - it('preserves the final values merged by graphql-env', () => { + it('preserves final values merged by graphql-env', async () => { fs.writeFileSync( path.join(tempDir, 'pgpm.json'), JSON.stringify({ sms: { - provider: 'custom', + provider: 'devsms', senderId: 'ConfiguredSender', requestTimeoutMs: 3200, - dryRun: true, + dryRun: false, devsms: { - baseUrl: 'http://configured-devsms:4000' - } - } + baseUrl: 'http://configured-devsms:4000', + }, + }, }) ); + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); + + await invoke({ NODE_ENV: 'test' }); - expect(loadSmsOptions({ NODE_ENV: 'test' }, tempDir)).toEqual({ - provider: 'custom', - senderId: 'ConfiguredSender', - requestTimeoutMs: 3200, - dryRun: true, - devsms: { - baseUrl: 'http://configured-devsms:4000' - } - }); + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 3200); + expect(global.fetch).toHaveBeenCalledWith( + 'http://configured-devsms:4000/api/sms/send/twilio', + expect.objectContaining({ + body: JSON.stringify({ + From: 'ConfiguredSender', + To: '+14155550123', + Body: 'Your sign-in code is 123456. Do not share this code.', + }), + }) + ); }); }); diff --git a/functions/send-sms/__tests__/devsms.test.ts b/functions/send-sms/__tests__/devsms.test.ts deleted file mode 100644 index 62005f1d0..000000000 --- a/functions/send-sms/__tests__/devsms.test.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { DevSmsProvider } from '../providers/devsms'; -import type { SmsSendRequest } from '../providers/types'; - -const request: SmsSendRequest = { - to: '+14155550123', - body: 'Your sign-in code is 123456. Do not share this code.', - senderId: 'TestSender', - metadata: { - jobId: 'job-1', - databaseId: 'db-1', - purpose: 'sign_in_otp' - } -}; - -const jsonResponse = (body: unknown, status = 201): Response => - new Response(JSON.stringify(body), { - status, - headers: { 'content-type': 'application/json' } - }); - -describe('DevSmsProvider', () => { - it('sends the correct URL, method, headers, and request body', async () => { - const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ - id: 'row_1', - provider_message_id: 'SM123', - status: 'queued' - })); - const provider = new DevSmsProvider({ - baseUrl: 'http://devsms:4000', - requestTimeoutMs: 5000, - fetchImpl: fetchImpl as unknown as typeof fetch - }); - - await provider.send(request); - - expect(fetchImpl).toHaveBeenCalledWith( - 'http://devsms:4000/api/sms/send/twilio', - expect.objectContaining({ - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ - From: 'TestSender', - To: '+14155550123', - Body: 'Your sign-in code is 123456. Do not share this code.' - }) - }) - ); - }); - - it('maps provider responses to SmsSendResult', async () => { - const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ - id: 'row_1', - provider_message_id: 'SM123', - status: 'sent' - })); - const provider = new DevSmsProvider({ - baseUrl: 'http://devsms:4000/', - requestTimeoutMs: 5000, - fetchImpl: fetchImpl as unknown as typeof fetch - }); - - await expect(provider.send(request)).resolves.toEqual({ - provider: 'devsms', - messageId: 'SM123', - status: 'sent' - }); - }); - - it('throws for non-2xx responses without exposing the response body', async () => { - const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ - error: 'OTP 123456 failed for +14155550123' - }, 400)); - const provider = new DevSmsProvider({ - baseUrl: 'http://devsms:4000', - requestTimeoutMs: 5000, - fetchImpl: fetchImpl as unknown as typeof fetch - }); - - const error = await provider.send(request).then( - () => undefined, - (cause: unknown) => cause as Error - ); - - expect(error).toBeInstanceOf(Error); - expect(error?.message).toBe('DevSmsProvider request failed with 400'); - expect(error?.message).not.toContain('123456'); - expect(error?.message).not.toContain('+14155550123'); - }); - - it('throws on timeout', async () => { - jest.useFakeTimers(); - const fetchImpl = jest.fn((_url: string, init: RequestInit) => - new Promise((_resolve, reject) => { - init.signal?.addEventListener('abort', () => { - const error = new Error('aborted'); - error.name = 'AbortError'; - reject(error); - }); - }) - ); - const provider = new DevSmsProvider({ - baseUrl: 'http://devsms:4000', - requestTimeoutMs: 10, - fetchImpl: fetchImpl as unknown as typeof fetch - }); - - const promise = provider.send(request); - jest.advanceTimersByTime(10); - await expect(promise).rejects.toThrow('DevSmsProvider timed out after 10ms'); - jest.useRealTimers(); - }); - - it('throws for invalid JSON responses', async () => { - const fetchImpl = jest.fn().mockResolvedValue(new Response('not-json', { status: 201 })); - const provider = new DevSmsProvider({ - baseUrl: 'http://devsms:4000', - requestTimeoutMs: 5000, - fetchImpl: fetchImpl as unknown as typeof fetch - }); - - await expect(provider.send(request)).rejects.toThrow('DevSmsProvider returned invalid JSON'); - }); - - it('throws when the response is missing a message ID', async () => { - const fetchImpl = jest.fn().mockResolvedValue(jsonResponse({ status: 'queued' })); - const provider = new DevSmsProvider({ - baseUrl: 'http://devsms:4000', - requestTimeoutMs: 5000, - fetchImpl: fetchImpl as unknown as typeof fetch - }); - - await expect(provider.send(request)).rejects.toThrow('DevSmsProvider response missing message ID'); - }); -}); diff --git a/functions/send-sms/__tests__/handler.test.ts b/functions/send-sms/__tests__/handler.test.ts index 5617211ef..c747f4d4c 100644 --- a/functions/send-sms/__tests__/handler.test.ts +++ b/functions/send-sms/__tests__/handler.test.ts @@ -1,10 +1,10 @@ -let handler: any; import { createMockContext } from '../../../tests/helpers/mock-context'; +let handler: any; const mockGetEnvOptions = jest.fn(); jest.mock('@constructive-io/graphql-env', () => ({ - getEnvOptions: mockGetEnvOptions + getEnvOptions: mockGetEnvOptions, })); const smsConfig = (overrides: Record = {}) => ({ @@ -13,27 +13,37 @@ const smsConfig = (overrides: Record = {}) => ({ requestTimeoutMs: 5000, dryRun: false, devsms: { - baseUrl: 'http://devsms:4000' + baseUrl: 'http://devsms:4000', }, - ...overrides + ...overrides, }); -const mockSuccessfulFetch = () => { - const fetchMock = jest.fn().mockResolvedValue( - new Response(JSON.stringify({ - id: 'row_123', - provider: 'twilio', - provider_message_id: 'SM123', - status: 'queued' - }), { - status: 201, - headers: { 'content-type': 'application/json' } - }) - ); +const jsonResponse = (body: unknown, status = 201): Response => + new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); + +const mockSuccessfulFetch = ( + body: Record = { + id: 'row_123', + provider: 'twilio', + provider_message_id: 'SM123', + status: 'queued', + } +) => { + const fetchMock = jest.fn().mockResolvedValue(jsonResponse(body)); global.fetch = fetchMock as unknown as typeof fetch; return fetchMock; }; +const otpPayload = (overrides: Record = {}) => ({ + sms_type: 'sms_otp_code', + phone: '+14155550123', + code: '123456', + ...overrides, +}); + describe('send-sms handler', () => { beforeEach(() => { jest.resetModules(); @@ -44,27 +54,39 @@ describe('send-sms handler', () => { handler = require('../handler').default; }); - it('sends sms_otp_code payloads through DevSmsProvider', async () => { + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + it('sends sms_otp_code payloads through DevSms', async () => { const ctx = createMockContext({ env: { SMS_PROVIDER: 'devsms', - DEVSMS_BASE_URL: 'http://devsms:4000' - } + DEVSMS_BASE_URL: 'http://devsms:4000', + }, }); - const result = await handler({ - sms_type: 'sms_otp_code', - phone: '+1 (415) 555-0123', - code: '123456' - }, ctx as any); + const result = await handler( + { + sms_type: 'sms_otp_code', + phone: '+1 (415) 555-0123', + code: '123456', + }, + ctx as any + ); expect(result).toEqual({ complete: true, provider: 'devsms', messageId: 'SM123', - status: 'queued' + status: 'queued', }); - expect(mockGetEnvOptions).toHaveBeenCalledWith({}, expect.any(String), ctx.env); + expect(mockGetEnvOptions).toHaveBeenCalledWith( + {}, + expect.any(String), + ctx.env + ); expect(global.fetch).toHaveBeenCalledWith( 'http://devsms:4000/api/sms/send/twilio', expect.objectContaining({ @@ -73,89 +95,143 @@ describe('send-sms handler', () => { body: JSON.stringify({ From: 'TestSender', To: '+14155550123', - Body: 'Your sign-in code is 123456. Do not share this code.' - }) + Body: 'Your sign-in code is 123456. Do not share this code.', + }), }) ); }); - it('sends mfa_verification_code payloads through DevSmsProvider', async () => { - const result = await handler({ - sms_type: 'mfa_verification_code', - user_id: 'user-1', - phone_cc: '44', - phone_number: '020 7946 0018', - code: '654321' - }, createMockContext() as any); + it('sends mfa_verification_code payloads through DevSms', async () => { + const result = await handler( + { + sms_type: 'mfa_verification_code', + user_id: 'user-1', + phone_cc: '44', + phone_number: '020 7946 0018', + code: '654321', + }, + createMockContext() as any + ); - expect(result).toEqual(expect.objectContaining({ + expect(result).toEqual({ complete: true, provider: 'devsms', - messageId: 'SM123' - })); + messageId: 'SM123', + status: 'queued', + }); expect(global.fetch).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ body: JSON.stringify({ From: 'TestSender', To: '+442079460018', - Body: 'Your verification code is 654321. Do not share this code.' - }) + Body: 'Your verification code is 654321. Do not share this code.', + }), + }) + ); + }); + + it('normalizes another country code with a leading national zero', async () => { + await handler( + { + sms_type: 'mfa_verification_code', + user_id: 'user-1', + phone_cc: '81', + phone_number: '03-1234-5678', + code: '654321', + }, + createMockContext() as any + ); + + expect(global.fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: expect.stringContaining('"To":"+81312345678"'), }) ); }); it('preserves a six-digit code with leading zero', async () => { - await handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '012345' - }, createMockContext() as any); + await handler(otpPayload({ code: '012345' }), createMockContext() as any); expect(global.fetch).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ - body: expect.stringContaining('012345') + body: expect.stringContaining('012345'), }) ); }); it('throws for unknown sms_type', async () => { - await expect(handler({ - sms_type: 'unknown', - phone: '+14155550123', - code: '123456' - } as any, createMockContext() as any)).rejects.toThrow('Unsupported sms_type: unknown'); + await expect( + handler( + { + sms_type: 'unknown', + phone: '+14155550123', + code: '123456', + }, + createMockContext() as any + ) + ).rejects.toThrow('Unsupported sms_type: unknown'); }); - it('throws for missing code', async () => { - await expect(handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123' - } as any, createMockContext() as any)).rejects.toThrow('code must be a six-digit string'); + it.each([ + [undefined, 'missing'], + ['12345', 'too short'], + ['1234567', 'too long'], + ['12345a', 'non-numeric'], + [123456, 'not a string'], + ])('rejects an invalid verification code (%s: %s)', async (code) => { + await expect( + handler(otpPayload({ code }), createMockContext() as any) + ).rejects.toThrow('code must be a six-digit string'); + expect(global.fetch).not.toHaveBeenCalled(); }); - it('throws for invalid phone numbers', async () => { - await expect(handler({ - sms_type: 'sms_otp_code', - phone: 'not-a-phone', - code: '123456' - }, createMockContext() as any)).rejects.toThrow('Invalid phone number'); + it.each([ + [otpPayload({ phone: '' }), 'Missing required field: phone'], + [otpPayload({ phone: 'not-a-phone' }), 'Invalid phone number'], + [ + { + sms_type: 'mfa_verification_code', + user_id: 'user-1', + phone_cc: '1', + phone_number: '', + code: '123456', + }, + 'Missing required field: phone_number', + ], + [ + { + sms_type: 'mfa_verification_code', + user_id: 'user-1', + phone_cc: '1', + phone_number: '415-555-ABCD', + code: '123456', + }, + 'Invalid phone number', + ], + ])('rejects invalid phone input', async (payload, expectedError) => { + await expect(handler(payload, createMockContext() as any)).rejects.toThrow( + expectedError + ); + expect(global.fetch).not.toHaveBeenCalled(); }); it.each([ ['no sms configuration', {}], - ['an empty sms configuration', { sms: {} }] - ])('defaults dry-run to false and requires a provider with %s', async (_case, config) => { - mockGetEnvOptions.mockReturnValue(config); - - await expect(handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, createMockContext() as any)).rejects.toThrow('Missing SMS provider configuration'); - expect(global.fetch).not.toHaveBeenCalled(); - }); + ['an empty sms configuration', { sms: {} }], + ])( + 'defaults dry-run to false and requires a provider with %s', + async (_case, config) => { + mockGetEnvOptions.mockReturnValue(config); + + await expect( + handler(otpPayload(), createMockContext() as any) + ).rejects.toThrow('Missing SMS provider configuration'); + expect(global.fetch).not.toHaveBeenCalled(); + } + ); it('applies consumer defaults when timeout and dry-run are not configured', async () => { mockGetEnvOptions.mockReturnValue({ @@ -163,49 +239,37 @@ describe('send-sms handler', () => { provider: 'devsms', senderId: 'TestSender', devsms: { - baseUrl: 'http://devsms:4000' - } - } + baseUrl: 'http://devsms:4000', + }, + }, }); const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); - await handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, createMockContext() as any); + await handler(otpPayload(), createMockContext() as any); expect(global.fetch).toHaveBeenCalledTimes(1); expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 5000); - setTimeoutSpy.mockRestore(); }); it('preserves a configured request timeout after graphql-env merging', async () => { mockGetEnvOptions.mockReturnValue({ - sms: smsConfig({ requestTimeoutMs: 1234 }) + sms: smsConfig({ requestTimeoutMs: 1234 }), }); const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); - await handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, createMockContext() as any); + await handler(otpPayload(), createMockContext() as any); expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 1234); - setTimeoutSpy.mockRestore(); }); it('rejects non-positive request timeouts', async () => { mockGetEnvOptions.mockReturnValue({ - sms: smsConfig({ requestTimeoutMs: 0 }) + sms: smsConfig({ requestTimeoutMs: 0 }), }); - await expect(handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, createMockContext() as any)).rejects.toThrow( + await expect( + handler(otpPayload(), createMockContext() as any) + ).rejects.toThrow( 'options.sms.requestTimeoutMs must be a positive integer' ); expect(global.fetch).not.toHaveBeenCalled(); @@ -215,16 +279,14 @@ describe('send-sms handler', () => { mockGetEnvOptions.mockReturnValue({ sms: { provider: 'twilio', - requestTimeoutMs: 5000 - } + requestTimeoutMs: 5000, + }, }); const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); - await expect(handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, ctx as any)).rejects.toThrow('SEND_SMS_DRY_RUN'); + await expect(handler(otpPayload(), ctx as any)).rejects.toThrow( + 'SEND_SMS_DRY_RUN' + ); expect(global.fetch).not.toHaveBeenCalled(); }); @@ -232,54 +294,196 @@ describe('send-sms handler', () => { mockGetEnvOptions.mockReturnValue({ sms: { requestTimeoutMs: 5000, - dryRun: true - } + dryRun: true, + }, }); const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); - const result = await handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, ctx as any); + await expect(handler(otpPayload(), ctx as any)).resolves.toEqual({ + complete: true, + dryRun: true, + }); + expect(global.fetch).not.toHaveBeenCalled(); + }); - expect(result).toEqual({ complete: true, dryRun: true }); + it('rejects DevSms in production', async () => { + const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); + + await expect(handler(otpPayload(), ctx as any)).rejects.toThrow( + 'SMS_PROVIDER=devsms is not allowed' + ); expect(global.fetch).not.toHaveBeenCalled(); }); - it('rejects devsms in production', async () => { + it('preserves production DevSms validation before the dry-run branch', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: smsConfig({ dryRun: true }), + }); const ctx = createMockContext({ env: { NODE_ENV: 'production' } }); - await expect(handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, ctx as any)).rejects.toThrow('SMS_PROVIDER=devsms is not allowed'); + await expect(handler(otpPayload(), ctx as any)).rejects.toThrow( + 'SMS_PROVIDER=devsms is not allowed' + ); expect(global.fetch).not.toHaveBeenCalled(); }); it('dry-run validates and renders without sending', async () => { mockGetEnvOptions.mockReturnValue({ sms: { dryRun: true } }); - const result = await handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '123456' - }, createMockContext() as any); + await expect( + handler(otpPayload(), createMockContext() as any) + ).resolves.toEqual({ + complete: true, + dryRun: true, + }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it.each(['twilio', 'sns', 'custom'])( + 'rejects unsupported provider %s', + async (provider) => { + mockGetEnvOptions.mockReturnValue({ + sms: smsConfig({ provider }), + }); + + await expect( + handler(otpPayload(), createMockContext() as any) + ).rejects.toThrow(`Unsupported SMS provider: ${provider}`); + expect(global.fetch).not.toHaveBeenCalled(); + } + ); - expect(result).toEqual({ complete: true, dryRun: true }); + it('requires a DevSms base URL', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: smsConfig({ devsms: undefined }), + }); + + await expect( + handler(otpPayload(), createMockContext() as any) + ).rejects.toThrow('Missing DevSms base URL configuration'); expect(global.fetch).not.toHaveBeenCalled(); }); + it('omits From when no sender ID is configured', async () => { + mockGetEnvOptions.mockReturnValue({ + sms: smsConfig({ senderId: undefined }), + }); + + await handler(otpPayload(), createMockContext() as any); + + expect(global.fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: JSON.stringify({ + To: '+14155550123', + Body: 'Your sign-in code is 123456. Do not share this code.', + }), + }) + ); + }); + + it.each([ + ['provider_message_id', { provider_message_id: 'provider-message-id' }], + ['providerMessageId', { providerMessageId: 'provider-message-id' }], + ['sid', { sid: 'provider-message-id' }], + ['id', { id: 'provider-message-id' }], + ])('accepts the %s message ID response field', async (_field, response) => { + mockSuccessfulFetch(response); + + await expect( + handler(otpPayload(), createMockContext() as any) + ).resolves.toEqual({ + complete: true, + provider: 'devsms', + messageId: 'provider-message-id', + status: 'unknown', + }); + }); + + it.each([ + ['queued', 'queued'], + ['sent', 'sent'], + ['delivered', 'delivered'], + ['failed', 'unknown'], + [undefined, 'unknown'], + ])('normalizes provider status %s to %s', async (status, expected) => { + mockSuccessfulFetch({ + provider_message_id: 'SM123', + status, + }); + + await expect( + handler(otpPayload(), createMockContext() as any) + ).resolves.toEqual(expect.objectContaining({ status: expected })); + }); + + it('times out DevSms requests', async () => { + jest.useFakeTimers(); + mockGetEnvOptions.mockReturnValue({ + sms: smsConfig({ requestTimeoutMs: 10 }), + }); + global.fetch = jest.fn( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => { + const error = new Error('aborted'); + error.name = 'AbortError'; + reject(error); + }); + }) + ) as unknown as typeof fetch; + + const promise = handler(otpPayload(), createMockContext() as any); + jest.advanceTimersByTime(10); + + await expect(promise).rejects.toThrow( + 'DevSmsProvider timed out after 10ms' + ); + }); + + it('rejects non-2xx responses without exposing the response body', async () => { + global.fetch = jest + .fn() + .mockResolvedValue( + jsonResponse({ error: 'OTP 123456 failed for +14155550123' }, 400) + ) as unknown as typeof fetch; + + const error = await handler(otpPayload(), createMockContext() as any).then( + () => undefined, + (cause: unknown) => cause as Error + ); + + expect(error).toBeInstanceOf(Error); + expect(error?.message).toBe('DevSmsProvider request failed with 400'); + expect(error?.message).not.toContain('123456'); + expect(error?.message).not.toContain('+14155550123'); + }); + + it('rejects invalid JSON responses', async () => { + global.fetch = jest + .fn() + .mockResolvedValue( + new Response('not-json', { status: 201 }) + ) as unknown as typeof fetch; + + await expect( + handler(otpPayload(), createMockContext() as any) + ).rejects.toThrow('DevSmsProvider returned invalid JSON'); + }); + + it('rejects responses without a message ID', async () => { + mockSuccessfulFetch({ status: 'queued' }); + + await expect( + handler(otpPayload(), createMockContext() as any) + ).rejects.toThrow('DevSmsProvider response missing message ID'); + }); + it('does not log code, body, or full phone number', async () => { mockGetEnvOptions.mockReturnValue({ sms: { dryRun: true } }); const ctx = createMockContext(); - await handler({ - sms_type: 'sms_otp_code', - phone: '+14155550123', - code: '012345' - }, ctx as any); + await handler(otpPayload({ code: '012345' }), ctx as any); const logOutput = JSON.stringify((ctx.log.info as jest.Mock).mock.calls); expect(logOutput).not.toContain('012345'); diff --git a/functions/send-sms/__tests__/phone.test.ts b/functions/send-sms/__tests__/phone.test.ts deleted file mode 100644 index c0d9f390e..000000000 --- a/functions/send-sms/__tests__/phone.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { normalizePhone, normalizePhoneParts } from '../phone'; - -describe('phone normalization', () => { - it('normalizes international phone strings to E.164', () => { - expect(normalizePhone('+1 (415) 555-0123')).toBe('+14155550123'); - }); - - it('normalizes phone_cc + national phone_number with trunk prefix', () => { - expect(normalizePhoneParts('+44', '020 7946 0018')).toBe('+442079460018'); - }); - - it('normalizes another country code with a leading national zero', () => { - expect(normalizePhoneParts('81', '03-1234-5678')).toBe('+81312345678'); - }); - - it('rejects empty phone values', () => { - expect(() => normalizePhone('')).toThrow('Missing required field: phone'); - expect(() => normalizePhoneParts('1', '')).toThrow('Missing required field: phone_number'); - }); - - it('rejects invalid phone characters', () => { - expect(() => normalizePhone('+1 415 CALL NOW')).toThrow('Invalid phone number'); - expect(() => normalizePhoneParts('1', '415-555-ABCD')).toThrow('Invalid phone number'); - }); -}); diff --git a/functions/send-sms/config.ts b/functions/send-sms/config.ts deleted file mode 100644 index 5299b3486..000000000 --- a/functions/send-sms/config.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { getEnvOptions, type SmsOptions } from '@constructive-io/graphql-env'; -import { - bool, - devDefault, - env as validateEnv, - makeValidator, - withDefault -} from '12factor-env'; - -export type { SmsOptions } from '@constructive-io/graphql-env'; - -export type ResolvedSmsOptions = Omit< - SmsOptions, - 'requestTimeoutMs' | 'dryRun' -> & { - requestTimeoutMs: number; - dryRun: boolean; -}; - -const DEFAULT_SMS_REQUEST_TIMEOUT_MS = 5000; - -const positiveInteger = makeValidator((value: string) => { - const raw = value as unknown; - if (typeof raw === 'number' && Number.isSafeInteger(raw) && raw > 0) { - return raw; - } - - const normalized = String(raw).trim(); - if (!/^[1-9]\d*$/.test(normalized)) { - throw new Error('options.sms.requestTimeoutMs must be a positive integer'); - } - - const parsed = Number(normalized); - if (!Number.isSafeInteger(parsed)) { - throw new Error('options.sms.requestTimeoutMs must be a positive integer'); - } - return parsed; -}); - -const toEnvString = ( - value: number | boolean | undefined -): string | undefined => (value === undefined ? undefined : String(value)); - -/** - * Apply defaults and production requirements only after graphql-env has merged - * pgpm.json, environment variables, and runtime overrides. 12factor-env accepts - * flat string inputs, so the final typed SMS values are adapted back to that - * boundary rather than reparsing the raw SMS_* variables. - */ -export const resolveSmsOptions = ( - options: SmsOptions, - inputEnv: Record -): ResolvedSmsOptions => { - const resolved = validateEnv( - { - NODE_ENV: inputEnv.NODE_ENV, - GITHUB_ACTIONS: inputEnv.GITHUB_ACTIONS, - SMS_REQUEST_TIMEOUT_MS: toEnvString(options.requestTimeoutMs), - SEND_SMS_DRY_RUN: toEnvString(options.dryRun) - }, - {}, - { - SMS_REQUEST_TIMEOUT_MS: withDefault( - positiveInteger, - DEFAULT_SMS_REQUEST_TIMEOUT_MS - ), - // Local and test environments send to explicitly configured development - // providers by default. Production must explicitly choose true or false. - SEND_SMS_DRY_RUN: devDefault(bool, false) - } - ); - - return { - ...options, - requestTimeoutMs: resolved.SMS_REQUEST_TIMEOUT_MS, - dryRun: resolved.SEND_SMS_DRY_RUN - }; -}; - -export const loadSmsOptions = ( - inputEnv: Record, - cwd: string = process.cwd() -): ResolvedSmsOptions => { - const options = getEnvOptions({}, cwd, inputEnv); - return resolveSmsOptions(options.sms ?? {}, inputEnv); -}; diff --git a/functions/send-sms/handler.ts b/functions/send-sms/handler.ts index ae0c3821e..d0f07db12 100644 --- a/functions/send-sms/handler.ts +++ b/functions/send-sms/handler.ts @@ -1,10 +1,18 @@ import type { FunctionHandler } from '@constructive-io/fn-runtime'; - -import { loadSmsOptions } from './config'; -import { maskPhone, normalizePhone, normalizePhoneParts } from './phone'; -import { assertProviderAllowed, createSmsProvider } from './providers/factory'; -import type { SmsPurpose, SmsSendRequest } from './providers/types'; -import { renderSmsBody, type SmsType } from './templates'; +import { getEnvOptions, type SmsOptions } from '@constructive-io/graphql-env'; +import { + bool, + devDefault, + env as validateEnv, + makeValidator, + withDefault, +} from '12factor-env'; +import type { CountryCode } from 'libphonenumber-js'; +import { + getCountries, + getCountryCallingCode, + parsePhoneNumberFromString, +} from 'libphonenumber-js'; type SendSmsParams = | { @@ -20,11 +28,192 @@ type SendSmsParams = code: string; }; +type SmsType = SendSmsParams['sms_type']; + type NormalizedSmsJob = { smsType: SmsType; to: string; code: string; - purpose: SmsPurpose; +}; + +type ResolvedSmsOptions = Omit & { + requestTimeoutMs: number; + dryRun: boolean; +}; + +type SmsSendStatus = 'queued' | 'sent' | 'delivered' | 'unknown'; + +type DevSmsResponse = { + id?: string; + provider_message_id?: string; + providerMessageId?: string; + sid?: string; + status?: string; +}; + +type DevSmsResult = { + provider: 'devsms'; + messageId: string; + status: SmsSendStatus; +}; + +const DEFAULT_SMS_REQUEST_TIMEOUT_MS = 5000; +const DEVSMS_TWILIO_SEND_PATH = '/api/sms/send/twilio'; + +const SMS_TEMPLATES: Record = { + sms_otp_code: 'Your sign-in code is {code}. Do not share this code.', + mfa_verification_code: + 'Your verification code is {code}. Do not share this code.', +}; + +const positiveInteger = makeValidator((value: string) => { + const raw = value as unknown; + if (typeof raw === 'number' && Number.isSafeInteger(raw) && raw > 0) { + return raw; + } + + const normalized = String(raw).trim(); + if (!/^[1-9]\d*$/.test(normalized)) { + throw new Error('options.sms.requestTimeoutMs must be a positive integer'); + } + + const parsed = Number(normalized); + if (!Number.isSafeInteger(parsed)) { + throw new Error('options.sms.requestTimeoutMs must be a positive integer'); + } + return parsed; +}); + +const toEnvString = ( + value: number | boolean | undefined +): string | undefined => (value === undefined ? undefined : String(value)); + +const resolveSmsOptions = ( + options: SmsOptions, + inputEnv: Record +): ResolvedSmsOptions => { + const resolved = validateEnv( + { + NODE_ENV: inputEnv.NODE_ENV, + GITHUB_ACTIONS: inputEnv.GITHUB_ACTIONS, + SMS_REQUEST_TIMEOUT_MS: toEnvString(options.requestTimeoutMs), + SEND_SMS_DRY_RUN: toEnvString(options.dryRun), + }, + {}, + { + SMS_REQUEST_TIMEOUT_MS: withDefault( + positiveInteger, + DEFAULT_SMS_REQUEST_TIMEOUT_MS + ), + // Local and test environments send to explicitly configured development + // providers by default. Production must explicitly choose true or false. + SEND_SMS_DRY_RUN: devDefault(bool, false), + } + ); + + return { + ...options, + requestTimeoutMs: resolved.SMS_REQUEST_TIMEOUT_MS, + dryRun: resolved.SEND_SMS_DRY_RUN, + }; +}; + +const loadSmsOptions = ( + inputEnv: Record +): ResolvedSmsOptions => { + // Apply consumer defaults only after graphql-env has merged pgpm.json, + // environment variables, and runtime overrides. + const options = getEnvOptions({}, process.cwd(), inputEnv); + return resolveSmsOptions(options.sms ?? {}, inputEnv); +}; + +const hasInvalidPhoneCharacters = (value: string): boolean => + /[^\d+().\s-]/.test(value); + +const normalizeCountryCallingCode = (phoneCc: unknown): string => { + if (typeof phoneCc !== 'string' || phoneCc.trim().length === 0) { + throw new Error('Missing required field: phone_cc'); + } + + const digits = phoneCc.replace(/\D/g, ''); + if (!/^[1-9]\d{0,2}$/.test(digits)) { + throw new Error('Invalid phone country code'); + } + return digits; +}; + +const countryForCallingCode = (callingCode: string): CountryCode | undefined => + getCountries().find( + (country) => getCountryCallingCode(country) === callingCode + ); + +const ensureValidE164 = (value: string): string => { + if (!/^\+[1-9]\d{7,14}$/.test(value)) { + throw new Error('Invalid phone number'); + } + return value; +}; + +const normalizePhone = (phone: unknown): string => { + if (typeof phone !== 'string' || phone.trim().length === 0) { + throw new Error('Missing required field: phone'); + } + if (hasInvalidPhoneCharacters(phone)) { + throw new Error('Invalid phone number'); + } + + const parsed = parsePhoneNumberFromString(phone); + if (parsed?.isValid()) { + return parsed.number; + } + + const compact = phone.replace(/[().\s-]/g, ''); + return ensureValidE164(compact); +}; + +const normalizePhoneParts = ( + phoneCc: unknown, + phoneNumber: unknown +): string => { + const callingCode = normalizeCountryCallingCode(phoneCc); + if (typeof phoneNumber !== 'string' || phoneNumber.trim().length === 0) { + throw new Error('Missing required field: phone_number'); + } + if (hasInvalidPhoneCharacters(phoneNumber) || phoneNumber.includes('+')) { + throw new Error('Invalid phone number'); + } + + const country = countryForCallingCode(callingCode); + if (country) { + const parsedNational = parsePhoneNumberFromString(phoneNumber, country); + if ( + parsedNational?.isValid() && + parsedNational.countryCallingCode === callingCode + ) { + return parsedNational.number; + } + } + + const nationalDigits = phoneNumber.replace(/\D/g, ''); + const candidates = [nationalDigits, nationalDigits.replace(/^0+/, '')].filter( + (candidate, index, all) => candidate && all.indexOf(candidate) === index + ); + + for (const candidate of candidates) { + const parsed = parsePhoneNumberFromString(`+${callingCode}${candidate}`); + if (parsed?.isValid()) { + return parsed.number; + } + } + + return ensureValidE164(`+${callingCode}${candidates[0] ?? ''}`); +}; + +const maskPhone = (phone: string): string => { + if (phone.length <= 8) { + return `${phone.slice(0, 2)}***${phone.slice(-2)}`; + } + return `${phone.slice(0, 5)}***${phone.slice(-4)}`; }; const isRecord = (value: unknown): value is Record => @@ -32,7 +221,9 @@ const isRecord = (value: unknown): value is Record => const requireSixDigitCode = (value: unknown): string => { if (typeof value !== 'string' || !/^\d{6}$/.test(value)) { - throw new Error('Invalid SMS verification code: code must be a six-digit string'); + throw new Error( + 'Invalid SMS verification code: code must be a six-digit string' + ); } return value; }; @@ -48,35 +239,103 @@ const normalizePayload = (params: unknown): NormalizedSmsJob => { smsType: 'sms_otp_code', to: normalizePhone(params.phone), code: requireSixDigitCode(params.code), - purpose: 'sign_in_otp' }; case 'mfa_verification_code': return { smsType: 'mfa_verification_code', to: normalizePhoneParts(params.phone_cc, params.phone_number), code: requireSixDigitCode(params.code), - purpose: 'mfa_verification' }; default: throw new Error(`Unsupported sms_type: ${String(params.sms_type)}`); } }; -const buildSmsRequest = ( - normalized: NormalizedSmsJob, - senderId: string | undefined, - jobId: string | undefined, - databaseId: string | undefined -): SmsSendRequest => ({ - to: normalized.to, - body: renderSmsBody(normalized.smsType, normalized.code), - ...(senderId && { senderId }), - metadata: { - jobId, - databaseId, - purpose: normalized.purpose +const renderSmsBody = (smsType: SmsType, code: string): string => + SMS_TEMPLATES[smsType].replace('{code}', code); + +const normalizeBaseUrl = (baseUrl: string): string => { + const trimmed = baseUrl.trim(); + if (!trimmed) { + throw new Error('DevSmsProvider requires sms.devsms.baseUrl'); } -}); + return trimmed.endsWith('/') ? trimmed.slice(0, -1) : trimmed; +}; + +const normalizeStatus = (status?: string): SmsSendStatus => { + if (status === 'queued' || status === 'sent' || status === 'delivered') { + return status; + } + return 'unknown'; +}; + +const getMessageId = (body: DevSmsResponse): string | undefined => + body.provider_message_id ?? body.providerMessageId ?? body.sid ?? body.id; + +const sendDevSms = async (options: { + baseUrl: string; + requestTimeoutMs: number; + to: string; + body: string; + senderId?: string; +}): Promise => { + const baseUrl = normalizeBaseUrl(options.baseUrl); + const controller = new AbortController(); + const timeout = setTimeout( + () => controller.abort(), + options.requestTimeoutMs + ); + + let response: Response; + try { + response = await fetch(`${baseUrl}${DEVSMS_TWILIO_SEND_PATH}`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + }, + body: JSON.stringify({ + ...(options.senderId && { From: options.senderId }), + To: options.to, + Body: options.body, + }), + signal: controller.signal, + }); + } catch (err) { + const error = err as Error; + if (error.name === 'AbortError') { + throw new Error( + `DevSmsProvider timed out after ${options.requestTimeoutMs}ms` + ); + } + throw new Error(`DevSmsProvider request failed: ${error.message}`); + } finally { + clearTimeout(timeout); + } + + if (!response.ok) { + // Provider responses may echo the request body, OTP, or recipient. + // Keep failures safe for job and CI logs by reporting status only. + throw new Error(`DevSmsProvider request failed with ${response.status}`); + } + + let responseBody: DevSmsResponse; + try { + responseBody = (await response.json()) as DevSmsResponse; + } catch { + throw new Error('DevSmsProvider returned invalid JSON'); + } + + const messageId = getMessageId(responseBody); + if (!messageId) { + throw new Error('DevSmsProvider response missing message ID'); + } + + return { + provider: 'devsms', + messageId, + status: normalizeStatus(responseBody.status), + }; +}; const handler: FunctionHandler = async (params, context) => { const normalized = normalizePayload(params); @@ -85,39 +344,67 @@ const handler: FunctionHandler = async (params, context) => { const databaseId = context.job?.databaseId; const maskedPhone = maskPhone(normalized.to); const providerName = options.provider; - const dryRun = options.dryRun; - assertProviderAllowed(options, { nodeEnv: context.env.NODE_ENV }); + // Preserve the existing policy order: production DevSms is rejected even + // when SEND_SMS_DRY_RUN=true. + if (context.env.NODE_ENV === 'production' && providerName === 'devsms') { + throw new Error( + 'SMS_PROVIDER=devsms is not allowed when NODE_ENV=production' + ); + } context.log.info('[send-sms] Processing request', { jobId, databaseId, smsType: normalized.smsType, maskedPhone, - provider: providerName + provider: providerName, }); - const request = buildSmsRequest(normalized, options.senderId, jobId, databaseId); + const body = renderSmsBody(normalized.smsType, normalized.code); - if (dryRun) { + if (options.dryRun) { context.log.info('[send-sms] Dry run complete; provider send skipped', { jobId, databaseId, smsType: normalized.smsType, maskedPhone, provider: providerName, - dryRun: true + dryRun: true, }); return { complete: true, - dryRun: true + dryRun: true, }; } - const provider = createSmsProvider(options, { - nodeEnv: context.env.NODE_ENV - }); - const result = await provider.send(request); + if (!providerName) { + throw new Error('Missing SMS provider configuration: options.sms.provider'); + } + + let result: DevSmsResult; + switch (providerName) { + case 'devsms': { + const baseUrl = options.devsms?.baseUrl; + if (!baseUrl) { + throw new Error( + 'Missing DevSms base URL configuration: options.sms.devsms.baseUrl' + ); + } + result = await sendDevSms({ + baseUrl, + requestTimeoutMs: options.requestTimeoutMs, + to: normalized.to, + body, + senderId: options.senderId, + }); + break; + } + case 'twilio': + case 'sns': + default: + throw new Error(`Unsupported SMS provider: ${String(providerName)}`); + } context.log.info('[send-sms] Provider accepted message', { jobId, @@ -126,17 +413,15 @@ const handler: FunctionHandler = async (params, context) => { maskedPhone, provider: result.provider, providerMessageId: result.messageId, - status: result.status + status: result.status, }); return { complete: true, provider: result.provider, messageId: result.messageId, - status: result.status + status: result.status, }; }; -export { buildSmsRequest, normalizePayload }; -export type { SendSmsParams }; export default handler; diff --git a/functions/send-sms/phone.ts b/functions/send-sms/phone.ts deleted file mode 100644 index 83c6e1b16..000000000 --- a/functions/send-sms/phone.ts +++ /dev/null @@ -1,86 +0,0 @@ -import type { CountryCode } from 'libphonenumber-js'; -import { - getCountries, - getCountryCallingCode, - parsePhoneNumberFromString -} from 'libphonenumber-js'; - -const hasInvalidPhoneCharacters = (value: string): boolean => - /[^\d+().\s-]/.test(value); - -const normalizeCountryCallingCode = (phoneCc: unknown): string => { - if (typeof phoneCc !== 'string' || phoneCc.trim().length === 0) { - throw new Error('Missing required field: phone_cc'); - } - - const digits = phoneCc.replace(/\D/g, ''); - if (!/^[1-9]\d{0,2}$/.test(digits)) { - throw new Error('Invalid phone country code'); - } - return digits; -}; - -const countryForCallingCode = (callingCode: string): CountryCode | undefined => - getCountries().find((country) => getCountryCallingCode(country) === callingCode); - -const ensureValidE164 = (value: string): string => { - if (!/^\+[1-9]\d{7,14}$/.test(value)) { - throw new Error('Invalid phone number'); - } - return value; -}; - -export const normalizePhone = (phone: unknown): string => { - if (typeof phone !== 'string' || phone.trim().length === 0) { - throw new Error('Missing required field: phone'); - } - if (hasInvalidPhoneCharacters(phone)) { - throw new Error('Invalid phone number'); - } - - const parsed = parsePhoneNumberFromString(phone); - if (parsed?.isValid()) { - return parsed.number; - } - - const compact = phone.replace(/[().\s-]/g, ''); - return ensureValidE164(compact); -}; - -export const normalizePhoneParts = (phoneCc: unknown, phoneNumber: unknown): string => { - const callingCode = normalizeCountryCallingCode(phoneCc); - if (typeof phoneNumber !== 'string' || phoneNumber.trim().length === 0) { - throw new Error('Missing required field: phone_number'); - } - if (hasInvalidPhoneCharacters(phoneNumber) || phoneNumber.includes('+')) { - throw new Error('Invalid phone number'); - } - - const country = countryForCallingCode(callingCode); - if (country) { - const parsedNational = parsePhoneNumberFromString(phoneNumber, country); - if (parsedNational?.isValid() && parsedNational.countryCallingCode === callingCode) { - return parsedNational.number; - } - } - - const nationalDigits = phoneNumber.replace(/\D/g, ''); - const candidates = [nationalDigits, nationalDigits.replace(/^0+/, '')] - .filter((candidate, index, all) => candidate && all.indexOf(candidate) === index); - - for (const candidate of candidates) { - const parsed = parsePhoneNumberFromString(`+${callingCode}${candidate}`); - if (parsed?.isValid()) { - return parsed.number; - } - } - - return ensureValidE164(`+${callingCode}${candidates[0] ?? ''}`); -}; - -export const maskPhone = (phone: string): string => { - if (phone.length <= 8) { - return `${phone.slice(0, 2)}***${phone.slice(-2)}`; - } - return `${phone.slice(0, 5)}***${phone.slice(-4)}`; -}; diff --git a/functions/send-sms/providers/devsms.ts b/functions/send-sms/providers/devsms.ts deleted file mode 100644 index b98e38bb6..000000000 --- a/functions/send-sms/providers/devsms.ts +++ /dev/null @@ -1,103 +0,0 @@ -import type { SmsProvider, SmsSendRequest, SmsSendResult } from './types'; - -export type DevSmsProviderOptions = { - baseUrl: string; - requestTimeoutMs: number; - fetchImpl?: typeof fetch; -}; - -type DevSmsResponse = { - id?: string; - provider?: string; - provider_message_id?: string; - providerMessageId?: string; - sid?: string; - status?: string; -}; - -const DEVSMS_TWILIO_SEND_PATH = '/api/sms/send/twilio'; - -const normalizeBaseUrl = (baseUrl: string): string => { - const trimmed = baseUrl.trim(); - if (!trimmed) { - throw new Error('DevSmsProvider requires sms.devsms.baseUrl'); - } - return trimmed.endsWith('/') ? trimmed.slice(0, -1) : trimmed; -}; - -const normalizeStatus = (status?: string): SmsSendResult['status'] => { - if (status === 'queued' || status === 'sent' || status === 'delivered') { - return status; - } - return 'unknown'; -}; - -const getMessageId = (body: DevSmsResponse): string | undefined => - body.provider_message_id ?? body.providerMessageId ?? body.sid ?? body.id; - -export class DevSmsProvider implements SmsProvider { - private readonly baseUrl: string; - private readonly requestTimeoutMs: number; - private readonly fetchImpl: typeof fetch; - - constructor(options: DevSmsProviderOptions) { - this.baseUrl = normalizeBaseUrl(options.baseUrl); - this.requestTimeoutMs = options.requestTimeoutMs; - this.fetchImpl = options.fetchImpl ?? fetch; - } - - async send(request: SmsSendRequest): Promise { - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), this.requestTimeoutMs); - - let response: Response; - try { - response = await this.fetchImpl(`${this.baseUrl}${DEVSMS_TWILIO_SEND_PATH}`, { - method: 'POST', - headers: { - 'content-type': 'application/json' - }, - body: JSON.stringify({ - ...(request.senderId && { From: request.senderId }), - To: request.to, - Body: request.body - }), - signal: controller.signal - }); - } catch (err) { - const error = err as Error; - if (error.name === 'AbortError') { - throw new Error(`DevSmsProvider timed out after ${this.requestTimeoutMs}ms`); - } - throw new Error(`DevSmsProvider request failed: ${error.message}`); - } finally { - clearTimeout(timeout); - } - - if (!response.ok) { - // Provider responses may echo the request body, OTP, or recipient. - // Keep failures safe for job and CI logs by reporting status only. - throw new Error(`DevSmsProvider request failed with ${response.status}`); - } - - let body: DevSmsResponse; - try { - body = await response.json() as DevSmsResponse; - } catch { - throw new Error('DevSmsProvider returned invalid JSON'); - } - - const messageId = getMessageId(body); - if (!messageId) { - throw new Error('DevSmsProvider response missing message ID'); - } - - return { - provider: 'devsms', - messageId, - status: normalizeStatus(body.status) - }; - } -} - -export { DEVSMS_TWILIO_SEND_PATH }; diff --git a/functions/send-sms/providers/factory.ts b/functions/send-sms/providers/factory.ts deleted file mode 100644 index fffac09d3..000000000 --- a/functions/send-sms/providers/factory.ts +++ /dev/null @@ -1,47 +0,0 @@ -import type { ResolvedSmsOptions, SmsOptions } from '../config'; -import { DevSmsProvider } from './devsms'; -import type { SmsProvider } from './types'; - -export type SmsProviderFactoryOptions = { - nodeEnv?: string; - fetchImpl?: typeof fetch; -}; - -export const assertProviderAllowed = ( - options: SmsOptions, - factoryOptions: SmsProviderFactoryOptions = {} -): void => { - if (factoryOptions.nodeEnv === 'production' && options.provider === 'devsms') { - throw new Error('SMS_PROVIDER=devsms is not allowed when NODE_ENV=production'); - } -}; - -export const createSmsProvider = ( - options: ResolvedSmsOptions, - factoryOptions: SmsProviderFactoryOptions = {} -): SmsProvider => { - assertProviderAllowed(options, factoryOptions); - - if (!options.provider) { - throw new Error('Missing SMS provider configuration: options.sms.provider'); - } - - switch (options.provider) { - case 'devsms': { - const baseUrl = options.devsms?.baseUrl; - if (!baseUrl) { - throw new Error('Missing DevSms base URL configuration: options.sms.devsms.baseUrl'); - } - return new DevSmsProvider({ - baseUrl, - requestTimeoutMs: options.requestTimeoutMs, - fetchImpl: factoryOptions.fetchImpl - }); - } - case 'twilio': - case 'sns': - throw new Error(`Unsupported SMS provider: ${options.provider}`); - default: - throw new Error(`Unsupported SMS provider: ${String(options.provider)}`); - } -}; diff --git a/functions/send-sms/providers/types.ts b/functions/send-sms/providers/types.ts deleted file mode 100644 index 81c3047e4..000000000 --- a/functions/send-sms/providers/types.ts +++ /dev/null @@ -1,22 +0,0 @@ -export type SmsPurpose = 'sign_in_otp' | 'mfa_verification'; - -export type SmsSendRequest = { - to: string; - body: string; - senderId?: string; - metadata?: { - jobId?: string; - databaseId?: string; - purpose: SmsPurpose; - }; -}; - -export type SmsSendResult = { - provider: string; - messageId: string; - status: 'queued' | 'sent' | 'delivered' | 'unknown'; -}; - -export interface SmsProvider { - send(request: SmsSendRequest): Promise; -} diff --git a/functions/send-sms/templates.ts b/functions/send-sms/templates.ts deleted file mode 100644 index 7f485ee12..000000000 --- a/functions/send-sms/templates.ts +++ /dev/null @@ -1,9 +0,0 @@ -export type SmsType = 'sms_otp_code' | 'mfa_verification_code'; - -const SMS_TEMPLATES: Record = { - sms_otp_code: 'Your sign-in code is {code}. Do not share this code.', - mfa_verification_code: 'Your verification code is {code}. Do not share this code.' -}; - -export const renderSmsBody = (smsType: SmsType, code: string): string => - SMS_TEMPLATES[smsType].replace('{code}', code); diff --git a/packages/fn-generator/src/builders/package.ts b/packages/fn-generator/src/builders/package.ts index ad9b9009a..327f5ddbe 100644 --- a/packages/fn-generator/src/builders/package.ts +++ b/packages/fn-generator/src/builders/package.ts @@ -8,7 +8,7 @@ import type { FunctionInfo, Manifest } from '../types'; * Build all manifests for a single function: * - per-template files (placeholders applied, package.json/tsconfig.json processed) * - shared template files (same processing) - * - symlinks for handler.{ts,py} and supporting TypeScript/Python sources + * - symlinks for handler.{ts,py}, *.d.ts, and any other *.py * * `templateDir` is the resolved type-specific dir (`templates/node-graphql/` etc.). */ @@ -56,40 +56,17 @@ export const buildPackageManifests = ( out.push({ kind: 'symlink', path: path.join(args.genDir, 'handler.py'), target: handlerPy }); } - // 4. Supporting source symlinks. Richer functions may split implementation - // across nested TypeScript/Python files; mirror scripts/generate.ts exactly. - const addSupportSymlinks = (base = ''): void => { - const entries = fs.readdirSync(path.join(args.fnDir, base)); - for (const entry of entries) { - if (entry === '__tests__') continue; - - const relPath = path.join(base, entry); - const sourcePath = path.join(args.fnDir, relPath); - const stat = fs.statSync(sourcePath); - - if (stat.isDirectory()) { - addSupportSymlinks(relPath); - continue; - } - - if ( - relPath === 'handler.ts' || - relPath === 'handler.py' || - !(/\.(ts|d\.ts|py)$/.test(entry)) || - entry.endsWith('.test.ts') - ) { - continue; - } - + // 4. Auxiliary symlinks: all *.d.ts and *.py (except handler.py already linked above) + const files = fs.readdirSync(args.fnDir); + for (const file of files) { + if (file.endsWith('.d.ts') || (file.endsWith('.py') && file !== 'handler.py')) { out.push({ kind: 'symlink', - path: path.join(args.genDir, relPath), - target: sourcePath, + path: path.join(args.genDir, file), + target: path.join(args.fnDir, file), }); } - }; - - addSupportSymlinks(); + } return out; }; diff --git a/scripts/generate.ts b/scripts/generate.ts index 1aaa134fa..1149dd234 100644 --- a/scripts/generate.ts +++ b/scripts/generate.ts @@ -282,37 +282,6 @@ function generateSkaffoldYaml(fns: FunctionInfo[]): void { } } -function symlinkFunctionSupportFiles(fnDir: string, genDir: string, base: string = ''): void { - const entries = fs.readdirSync(path.join(fnDir, base)) as string[]; - for (const entry of entries) { - if (entry === '__tests__') continue; - - const relPath = path.join(base, entry); - const sourcePath = path.join(fnDir, relPath); - const targetPath = path.join(genDir, relPath); - const stat = fs.statSync(sourcePath); - - if (stat.isDirectory()) { - symlinkFunctionSupportFiles(fnDir, genDir, relPath); - continue; - } - - if ( - relPath === 'handler.ts' || - relPath === 'handler.py' || - !(/\.(ts|d\.ts|py)$/.test(entry)) || - entry.endsWith('.test.ts') - ) { - continue; - } - - const targetDir = path.dirname(targetPath); - if (!fs.existsSync(targetDir)) fs.mkdirSync(targetDir, { recursive: true }); - const linked = ensureSymlink(sourcePath, targetPath); - if (linked) console.log(` - ${relPath} -> functions/${path.basename(fnDir)}/${relPath}`); - } -} - // --- Main --- function main(): void { @@ -391,8 +360,15 @@ function main(): void { if (linked) console.log(` - handler.py -> functions/${fnName}/handler.py`); } - // Symlink additional support source files/directories used by richer functions. - symlinkFunctionSupportFiles(fnDir, genDir); + // Symlink any .d.ts or .py files (excluding handler.py which is handled above) + const files = fs.readdirSync(fnDir) as string[]; + for (const file of files) { + if (file.endsWith('.d.ts') || (file.endsWith('.py') && file !== 'handler.py')) { + const target = path.join(fnDir, file); + const linked = ensureSymlink(target, path.join(genDir, file)); + if (linked) console.log(` - ${file} -> functions/${fnName}/${file}`); + } + } } // --packages-only: stop here, only workspace packages were needed From 2bd8f9d85300bddb3ea7fcc664ce5f1233d94729 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 28 Jul 2026 09:20:42 +0800 Subject: [PATCH 09/12] refactor: align SMS E2E ownership --- AGENTS.md | 2 +- CLAUDE.md | 4 -- DEVELOPMENT.md | 9 +++- functions/python-example/handler.json | 3 +- functions/sql-example/handler.json | 1 + .../constructive/knative-job-service.yaml | 4 +- k8s/base/functions/send-sms.yaml | 3 ++ k8s/base/kustomization.yaml | 1 - k8s/overlays/local-simple/config.yaml | 5 +- k8s/overlays/local-simple/devsms-local.yaml | 42 --------------- k8s/overlays/local-simple/kustomization.yaml | 1 - k8s/overlays/local/constructive/config.yaml | 3 -- .../constructive/knative-job-service.yaml | 4 +- k8s/overlays/local/devsms-local.yaml | 42 --------------- k8s/overlays/local/kustomization.yaml | 15 +----- packages/fn-generator/src/discovery.ts | 4 +- scripts/generate.ts | 6 +-- tests/e2e/README.md | 3 +- tests/e2e/__tests__/job-processing.test.ts | 3 +- tests/e2e/__tests__/send-sms.e2e.test.ts | 53 +++++++++++++++++++ .../send-verification-link.e2e.test.ts | 4 +- 21 files changed, 83 insertions(+), 129 deletions(-) delete mode 100644 k8s/overlays/local-simple/devsms-local.yaml delete mode 100644 k8s/overlays/local/devsms-local.yaml create mode 100644 tests/e2e/__tests__/send-sms.e2e.test.ts diff --git a/AGENTS.md b/AGENTS.md index a2cf4af1f..2438f0d2b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,7 +129,7 @@ pnpm build # Recompile ## Key Details -- Each function declares its port in `handler.json` (`send-email` 8081, `send-verification-link` 8082, `knative-job-example` 8083, `python-example` 8084); the job service uses 8080 +- Each function declares its port in `handler.json` (`send-email` 8081, `send-verification-link` 8082, `knative-job-example` 8083, `python-example` 8084, `sql-example` 8085, `send-sms` 8086); the job service uses 8080 - Email functions support dry-run via `SEND_EMAIL_DRY_RUN` / `SEND_VERIFICATION_LINK_DRY_RUN` (legacy `SIMPLE_EMAIL_DRY_RUN` / `SEND_EMAIL_LINK_DRY_RUN` still honored as fallback) - `loadFunctionApp()` in job/service resolves modules by name (e.g. `@constructive-io/send-email-fn`) - GraphQL clients require `GRAPHQL_URL` env var and `X-Database-Id` header diff --git a/CLAUDE.md b/CLAUDE.md index 21974314f..d04d52e4a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,8 +94,6 @@ Edit `functions//handler.ts` → Skaffold syncs the file into the containe | send-email | 8081 | | send-verification-link | 8082 | | send-sms | 8086 | -| DevSms API | 4000 | -| DevSms UI | 5153 | ## Debugging K8s Pods @@ -140,8 +138,6 @@ kubectl port-forward -n constructive-functions svc/knative-job-service 8080:8080 kubectl port-forward -n constructive-functions svc/send-email 8081:80 kubectl port-forward -n constructive-functions svc/send-verification-link 8082:80 kubectl port-forward -n constructive-functions svc/send-sms 8086:80 -kubectl port-forward -n constructive-functions svc/devsms 4000:4000 -kubectl port-forward -n constructive-functions svc/devsms 5153:5153 kubectl port-forward -n constructive-functions svc/constructive-server 3002:3000 ``` diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 149684e85..f8ebdfc72 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -276,11 +276,15 @@ make skaffold-dev This runs `skaffold dev -p local-simple` which: 1. Builds the `constructive-functions` Docker image from `Dockerfile.dev` 2. Deploys infrastructure (postgres, minio, constructive-server, constructive-server-admin, db-setup, job-service) via kustomize -3. Deploys functions (send-email, send-verification-link) via generated rawYaml manifests +3. Deploys functions (send-email, send-sms, send-verification-link) via generated rawYaml manifests 4. Sets up port-forwarding automatically 5. Watches `functions/**/*.ts` — edits are synced into running containers 6. `tsx --watch` inside each function container detects changes and restarts +Kubernetes profiles run `send-sms` with `SEND_SMS_DRY_RUN=true`; they verify +job routing and handler completion without deploying DevSms. The Hub +cross-repository E2E owns DevSms delivery and message-content assertions. + ### Option B: Knative Uses Knative Serving for functions (production parity). Requires Knative + Kourier. @@ -309,6 +313,7 @@ Changes to runtime packages (`packages/fn-runtime`, `packages/fn-app`) or `packa |---------|------------| | send-email | 8081 | | send-verification-link | 8082 | +| send-sms | 8086 | | knative-job-example | 8083 | | python-example | 8084 | | Job Service | 8080 | @@ -383,7 +388,7 @@ Stop any existing services using the ports: ```bash make dev-down -lsof -ti:5432,3002,1025,8025,8080,8081,8082 | xargs kill -9 +lsof -ti:5432,3002,4000,5153,1025,8025,8080,8081,8082,8086 | xargs kill -9 ``` **Functions can't connect to GraphQL** diff --git a/functions/python-example/handler.json b/functions/python-example/handler.json index 66b984102..46a23deb5 100644 --- a/functions/python-example/handler.json +++ b/functions/python-example/handler.json @@ -2,5 +2,6 @@ "name": "python-example", "version": "0.1.0", "description": "Example Python function", - "type": "python" + "type": "python", + "port": 8084 } diff --git a/functions/sql-example/handler.json b/functions/sql-example/handler.json index 1717196fa..d7c7788aa 100644 --- a/functions/sql-example/handler.json +++ b/functions/sql-example/handler.json @@ -2,5 +2,6 @@ "name": "sql-example", "version": "1.0.0", "type": "node-sql", + "port": 8085, "description": "Example function using node-sql template for direct PostgreSQL access" } diff --git a/k8s/base/constructive/knative-job-service.yaml b/k8s/base/constructive/knative-job-service.yaml index 031ef1f6d..cf0fcbeba 100644 --- a/k8s/base/constructive/knative-job-service.yaml +++ b/k8s/base/constructive/knative-job-service.yaml @@ -49,7 +49,7 @@ spec: - name: JOBS_SUPPORT_ANY value: "false" - name: JOBS_SUPPORTED - value: "email:send_email,email:send_verification_link,sms:send_verification_code" + value: "email:send_email,email:send_verification_link" - name: JOBS_CALLBACK_HOST value: "knative-job-service.interweb.svc.cluster.local" - name: JOBS_CALLBACK_BASE_URL @@ -65,7 +65,7 @@ spec: value: "development" - name: INTERNAL_GATEWAY_DEVELOPMENT_MAP - value: '{"email:send_email":"http://send-email.interweb.svc.cluster.local","email:send_verification_link":"http://send-verification-link.interweb.svc.cluster.local","sms:send_verification_code":"http://send-sms.interweb.svc.cluster.local"}' + value: '{"email:send_email":"http://send-email.interweb.svc.cluster.local","email:send_verification_link":"http://send-verification-link.interweb.svc.cluster.local"}' # Optional: stable hostname for logging/coordination - name: HOSTNAME diff --git a/k8s/base/functions/send-sms.yaml b/k8s/base/functions/send-sms.yaml index a413ebbf1..94df8282d 100644 --- a/k8s/base/functions/send-sms.yaml +++ b/k8s/base/functions/send-sms.yaml @@ -1,3 +1,6 @@ +# This manifest is intentionally consumed only by the local Knative overlay, +# which patches the function into development dry-run mode. Do not add it to +# k8s/base/kustomization.yaml until a production SMS provider is available. apiVersion: serving.knative.dev/v1 kind: Service metadata: diff --git a/k8s/base/kustomization.yaml b/k8s/base/kustomization.yaml index fcb0f633a..d7c2eec61 100644 --- a/k8s/base/kustomization.yaml +++ b/k8s/base/kustomization.yaml @@ -23,7 +23,6 @@ resources: # Function workloads - ./functions/send-email.yaml - ./functions/send-verification-link.yaml - - ./functions/send-sms.yaml # Required Secrets are intentionally not committed in the shared base. Create # pg-credentials, postgres-superuser, pgadmin-credentials, mailgun-credentials, diff --git a/k8s/overlays/local-simple/config.yaml b/k8s/overlays/local-simple/config.yaml index 77ac5fe9b..58bb0b057 100644 --- a/k8s/overlays/local-simple/config.yaml +++ b/k8s/overlays/local-simple/config.yaml @@ -22,9 +22,6 @@ data: LOG_TIMESTAMP: "true" - SMS_PROVIDER: "devsms" - SMS_SENDER_ID: "Constructive" - DEVSMS_BASE_URL: "http://devsms.constructive-functions.svc.cluster.local:4000" - SEND_EMAIL_DRY_RUN: "true" + SEND_SMS_DRY_RUN: "true" SEND_VERIFICATION_LINK_DRY_RUN: "true" diff --git a/k8s/overlays/local-simple/devsms-local.yaml b/k8s/overlays/local-simple/devsms-local.yaml deleted file mode 100644 index c9d9a0462..000000000 --- a/k8s/overlays/local-simple/devsms-local.yaml +++ /dev/null @@ -1,42 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: devsms - labels: - app: devsms -spec: - replicas: 1 - selector: - matchLabels: - app: devsms - template: - metadata: - labels: - app: devsms - spec: - containers: - - name: devsms - image: ghcr.io/mrmeaow/devsms:latest - ports: - - containerPort: 4000 - name: api - - containerPort: 5153 - name: web ---- -apiVersion: v1 -kind: Service -metadata: - name: devsms - labels: - app: devsms -spec: - type: ClusterIP - selector: - app: devsms - ports: - - name: api - port: 4000 - targetPort: api - - name: web - port: 5153 - targetPort: web diff --git a/k8s/overlays/local-simple/kustomization.yaml b/k8s/overlays/local-simple/kustomization.yaml index 4cf188c0f..12a50871f 100644 --- a/k8s/overlays/local-simple/kustomization.yaml +++ b/k8s/overlays/local-simple/kustomization.yaml @@ -7,7 +7,6 @@ resources: # Infrastructure - ./postgres-local.yaml - ./minio-local.yaml - - ./devsms-local.yaml # Secrets & config - ./config.yaml - ./pg-secret.yaml diff --git a/k8s/overlays/local/constructive/config.yaml b/k8s/overlays/local/constructive/config.yaml index 6c20560f4..c3fc0f43e 100644 --- a/k8s/overlays/local/constructive/config.yaml +++ b/k8s/overlays/local/constructive/config.yaml @@ -4,6 +4,3 @@ metadata: name: constructive data: PGHOST: "postgres.constructive-functions.svc.cluster.local" - SMS_PROVIDER: "devsms" - SMS_SENDER_ID: "Constructive" - DEVSMS_BASE_URL: "http://devsms.constructive-functions.svc.cluster.local:4000" diff --git a/k8s/overlays/local/constructive/knative-job-service.yaml b/k8s/overlays/local/constructive/knative-job-service.yaml index 7dcbc4c20..ae82d6f6a 100644 --- a/k8s/overlays/local/constructive/knative-job-service.yaml +++ b/k8s/overlays/local/constructive/knative-job-service.yaml @@ -10,6 +10,8 @@ spec: env: - name: NODE_ENV value: "development" + - name: JOBS_SUPPORTED + value: "email:send_email,email:send_verification_link,sms:send_verification_code" - name: KNATIVE_SERVICE_URL value: "constructive-functions.svc.cluster.local" - name: INTERNAL_JOBS_CALLBACK_URL @@ -21,7 +23,7 @@ spec: - name: INTERNAL_GATEWAY_URL value: "http://send-verification-link.constructive-functions.svc.cluster.local" - name: INTERNAL_GATEWAY_DEVELOPMENT_MAP - value: '{"email:send_email":"http://send-email.constructive-functions.svc.cluster.local","email:send_verification_link":"http://send-verification-link.constructive-functions.svc.cluster.local"}' + value: '{"email:send_email":"http://send-email.constructive-functions.svc.cluster.local","email:send_verification_link":"http://send-verification-link.constructive-functions.svc.cluster.local","sms:send_verification_code":"http://send-sms.constructive-functions.svc.cluster.local"}' resources: requests: cpu: "50m" diff --git a/k8s/overlays/local/devsms-local.yaml b/k8s/overlays/local/devsms-local.yaml deleted file mode 100644 index c9d9a0462..000000000 --- a/k8s/overlays/local/devsms-local.yaml +++ /dev/null @@ -1,42 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: devsms - labels: - app: devsms -spec: - replicas: 1 - selector: - matchLabels: - app: devsms - template: - metadata: - labels: - app: devsms - spec: - containers: - - name: devsms - image: ghcr.io/mrmeaow/devsms:latest - ports: - - containerPort: 4000 - name: api - - containerPort: 5153 - name: web ---- -apiVersion: v1 -kind: Service -metadata: - name: devsms - labels: - app: devsms -spec: - type: ClusterIP - selector: - app: devsms - ports: - - name: api - port: 4000 - targetPort: api - - name: web - port: 5153 - targetPort: web diff --git a/k8s/overlays/local/kustomization.yaml b/k8s/overlays/local/kustomization.yaml index b9f202158..3a132fc06 100644 --- a/k8s/overlays/local/kustomization.yaml +++ b/k8s/overlays/local/kustomization.yaml @@ -7,7 +7,6 @@ resources: - ./namespace.yaml - ./postgres-local.yaml - ./minio-local.yaml - - ./devsms-local.yaml # Core Constructive config - ../../base/constructive/config.yaml - ../../base/constructive/dashboard-config.yaml @@ -104,18 +103,8 @@ patches: - op: add path: /spec/template/spec/containers/0/env/- value: - name: SMS_PROVIDER - value: "devsms" - - op: add - path: /spec/template/spec/containers/0/env/- - value: - name: SMS_SENDER_ID - value: "Constructive" - - op: add - path: /spec/template/spec/containers/0/env/- - value: - name: DEVSMS_BASE_URL - value: "http://devsms.constructive-functions.svc.cluster.local:4000" + name: SEND_SMS_DRY_RUN + value: "true" target: group: serving.knative.dev version: v1 diff --git a/packages/fn-generator/src/discovery.ts b/packages/fn-generator/src/discovery.ts index 421bc7168..6553c88af 100644 --- a/packages/fn-generator/src/discovery.ts +++ b/packages/fn-generator/src/discovery.ts @@ -52,9 +52,7 @@ export const assignAndValidatePorts = ( const usedPorts = new Set( manifests.filter((m) => m.port).map((m) => m.port as number) ); - // Fill the lowest available local function port first. Explicit high ports - // must not shift existing functions that rely on automatic assignment. - let nextPort = 8081; + let nextPort = usedPorts.size > 0 ? Math.max(...usedPorts) + 1 : 8081; for (const m of manifests) { if (!m.port) { while (usedPorts.has(nextPort)) nextPort++; diff --git a/scripts/generate.ts b/scripts/generate.ts index 1149dd234..c4f038520 100644 --- a/scripts/generate.ts +++ b/scripts/generate.ts @@ -385,11 +385,9 @@ function main(): void { allManifests.push(readManifest(fnDir)); } - // Auto-assign ports for functions that don't have one, filling the lowest - // available local function port first so adding a high explicit port does not - // shift existing unpinned examples. + // Auto-assign ports for functions that don't have one const usedPorts = new Set(allManifests.filter((m) => m.port).map((m) => m.port!)); - let nextPort = 8081; + let nextPort = usedPorts.size > 0 ? Math.max(...usedPorts) + 1 : 8081; for (const m of allManifests) { if (!m.port) { while (usedPorts.has(nextPort)) nextPort++; diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 13d643d2b..b35b60d9b 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -9,7 +9,7 @@ All of the following must be running and accessible: - **PostgreSQL** — port-forwarded to `localhost:5432` - **constructive-server** — the GraphQL API server - **knative-job-service** — the job worker that picks up and dispatches jobs -- **send-email** / **send-verification-link** — the function deployments +- **send-email** / **send-sms** / **send-verification-link** — the function deployments - **Database seeded** — `constructive-db` job must have completed (schemas + pgpm packages deployed) The simplest way to get everything running: @@ -40,3 +40,4 @@ PGHOST=localhost PGPORT=5432 PGUSER=postgres PGPASSWORD="$POSTGRES_PASSWORD" PGD - **job-queue** — SQL-level tests: schema verification, `app_jobs.add_job`, job retrieval - **job-processing** — Full pipeline: insert a job → job-service picks it up → function processes it → job completes/fails +- **send-sms** — dry-run routing smoke test; Hub owns DevSms delivery assertions diff --git a/tests/e2e/__tests__/job-processing.test.ts b/tests/e2e/__tests__/job-processing.test.ts index 6f69f8fa4..b3ec96af8 100644 --- a/tests/e2e/__tests__/job-processing.test.ts +++ b/tests/e2e/__tests__/job-processing.test.ts @@ -2,7 +2,8 @@ * Job Queue Inspection (end-to-end via k8s) * * Shared utility test for inspecting queue state. Individual function e2e tests - * are in per-function files: send-email.e2e.test.ts, send-verification-link.e2e.test.ts + * are in per-function files such as send-email.e2e.test.ts, + * send-sms.e2e.test.ts, and send-verification-link.e2e.test.ts. */ import { getTestConnections, diff --git a/tests/e2e/__tests__/send-sms.e2e.test.ts b/tests/e2e/__tests__/send-sms.e2e.test.ts new file mode 100644 index 000000000..32be217e6 --- /dev/null +++ b/tests/e2e/__tests__/send-sms.e2e.test.ts @@ -0,0 +1,53 @@ +/** + * E2E: send-sms function + * + * Assumes Skaffold is running with send-sms deployed in dry-run mode: + * skaffold dev -p send-sms + * make skaffold-dev + * + * The cross-repository Hub E2E owns DevSms delivery assertions. This test only + * verifies the Functions-owned path: job-service routing and handler completion. + */ +import { + closeConnections, + getDatabaseId, + getTestConnections, + TestClient, +} from '../utils/db'; +import { addJob, waitForJobComplete } from '../utils/jobs'; + +const TEST_PHONE = '+12025550123'; + +describe('E2E: send-sms', () => { + let pg: TestClient; + let databaseId: string; + let jobId: string | undefined; + + beforeAll(async () => { + const connections = await getTestConnections(); + pg = connections.pg; + databaseId = await getDatabaseId(pg); + }); + + afterAll(async () => { + if (pg && jobId) { + await pg.result(`DELETE FROM app_jobs.jobs WHERE id = $1`, [jobId]); + } + await closeConnections(); + }); + + it('should process an sms:send_verification_code job in dry-run mode', async () => { + const job = await addJob(pg, databaseId, 'sms:send_verification_code', { + sms_type: 'sms_otp_code', + phone: TEST_PHONE, + code: '246810', + }); + jobId = job.id; + + expect(job.id).toBeDefined(); + + const result = await waitForJobComplete(pg, job.id, { timeout: 30000 }); + + expect(result.status).toBe('completed'); + }); +}); diff --git a/tests/e2e/__tests__/send-verification-link.e2e.test.ts b/tests/e2e/__tests__/send-verification-link.e2e.test.ts index 553f1edd2..c62d46f41 100644 --- a/tests/e2e/__tests__/send-verification-link.e2e.test.ts +++ b/tests/e2e/__tests__/send-verification-link.e2e.test.ts @@ -19,8 +19,6 @@ import { addJob, waitForJobComplete, deleteTestJobs } from '../utils/jobs'; const TEST_PREFIX = 'k8s-e2e-send-verification-link'; describe('E2E: send-verification-link', () => { - jest.setTimeout(90000); - let pg: TestClient; let databaseId: string; @@ -46,7 +44,7 @@ describe('E2E: send-verification-link', () => { expect(job.id).toBeDefined(); console.log(`Added email:send_verification_link job: ${job.id}`); - const result = await waitForJobComplete(pg, job.id, { timeout: 60000 }); + const result = await waitForJobComplete(pg, job.id, { timeout: 30000 }); console.log(`Job result: ${result.status}`, result.error || ''); From 25ac08932a3824e1aa6ffe153cbb05e151eb2709 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 28 Jul 2026 09:30:16 +0800 Subject: [PATCH 10/12] test: allow verification E2E callback to settle --- tests/e2e/__tests__/send-verification-link.e2e.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/e2e/__tests__/send-verification-link.e2e.test.ts b/tests/e2e/__tests__/send-verification-link.e2e.test.ts index c62d46f41..553f1edd2 100644 --- a/tests/e2e/__tests__/send-verification-link.e2e.test.ts +++ b/tests/e2e/__tests__/send-verification-link.e2e.test.ts @@ -19,6 +19,8 @@ import { addJob, waitForJobComplete, deleteTestJobs } from '../utils/jobs'; const TEST_PREFIX = 'k8s-e2e-send-verification-link'; describe('E2E: send-verification-link', () => { + jest.setTimeout(90000); + let pg: TestClient; let databaseId: string; @@ -44,7 +46,7 @@ describe('E2E: send-verification-link', () => { expect(job.id).toBeDefined(); console.log(`Added email:send_verification_link job: ${job.id}`); - const result = await waitForJobComplete(pg, job.id, { timeout: 30000 }); + const result = await waitForJobComplete(pg, job.id, { timeout: 60000 }); console.log(`Job result: ${result.status}`, result.error || ''); From ef53aff6794d61a7b87b01bed738344a4773639d Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 28 Jul 2026 10:49:49 +0800 Subject: [PATCH 11/12] refactor: keep SMS deployment in dry-run scope --- DEVELOPMENT.md | 14 ++-- docs/skills/local-dev-skaffold.md | 2 +- k8s/ARCHITECTURE.md | 3 +- k8s/base/functions/send-sms.yaml | 70 ------------------- .../constructive/knative-job-service.yaml | 4 +- k8s/overlays/local/kustomization.yaml | 24 ------- .../fn-generator/__tests__/snapshot.test.ts | 14 ++++ .../fn-generator/src/builders/skaffold.ts | 21 ++++++ scripts/generate.ts | 15 ++++ skaffold.yaml | 20 ------ templates/k8s/skaffold.yaml | 2 +- 11 files changed, 62 insertions(+), 127 deletions(-) delete mode 100644 k8s/base/functions/send-sms.yaml diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index f8ebdfc72..2589ff4bf 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -281,13 +281,16 @@ This runs `skaffold dev -p local-simple` which: 5. Watches `functions/**/*.ts` — edits are synced into running containers 6. `tsx --watch` inside each function container detects changes and restarts -Kubernetes profiles run `send-sms` with `SEND_SMS_DRY_RUN=true`; they verify -job routing and handler completion without deploying DevSms. The Hub -cross-repository E2E owns DevSms delivery and message-content assertions. +The plain-k8s profiles run `send-sms` with `SEND_SMS_DRY_RUN=true`; they +verify job routing and handler completion without deploying DevSms. SMS is +not included in the optional Knative profile while no production provider is +available. The Hub cross-repository E2E owns DevSms delivery and +message-content assertions. ### Option B: Knative -Uses Knative Serving for functions (production parity). Requires Knative + Kourier. +Uses Knative Serving for functions that have an explicit tracked Knative +manifest (currently the email functions). Requires Knative + Kourier. ```bash # One-time setup @@ -313,9 +316,6 @@ Changes to runtime packages (`packages/fn-runtime`, `packages/fn-app`) or `packa |---------|------------| | send-email | 8081 | | send-verification-link | 8082 | -| send-sms | 8086 | -| knative-job-example | 8083 | -| python-example | 8084 | | Job Service | 8080 | | PostgreSQL | 5432 | | Constructive Server | 3002 | diff --git a/docs/skills/local-dev-skaffold.md b/docs/skills/local-dev-skaffold.md index 315927712..84c052cce 100644 --- a/docs/skills/local-dev-skaffold.md +++ b/docs/skills/local-dev-skaffold.md @@ -65,7 +65,7 @@ Each function has its own Skaffold profile that deploys only the shared infrastr | `send-email` | infra + send-email | `skaffold dev -p send-email` | | `send-verification-link` | infra + send-verification-link | `skaffold dev -p send-verification-link` | | `local-simple` | infra + all functions | `skaffold dev -p local-simple` | -| `local` | Knative + all functions | `skaffold dev -p local` | +| `local` | Knative-enabled functions only | `skaffold dev -p local` | Profiles are auto-generated by `pnpm generate`. Adding a new function automatically creates its profile. diff --git a/k8s/ARCHITECTURE.md b/k8s/ARCHITECTURE.md index 16fb63b84..0882c611b 100644 --- a/k8s/ARCHITECTURE.md +++ b/k8s/ARCHITECTURE.md @@ -20,7 +20,8 @@ This document summarizes the Kubernetes layout under `k8s/`, highlights strength - `dashboard` (Next.js UI) — dev/staging. - `pgadmin` (DB admin UI) — dev/staging/local. - `knative-job-service` (job orchestration for functions). - - Function workloads (`send-email`, `send-verification-link`, `knative-job-example`, `python-example`) — Knative Services in `local`/`dev`/`staging`, plain Deployments in `local-simple`. + - Email function workloads (`send-email`, `send-verification-link`) — Knative Services in `local`/`dev`/`staging`. + - All generated function workloads — plain Deployments in `local-simple`. The repo-root `Makefile` wires this together via `make skaffold-dev` / `make skaffold-dev-knative` (local), and `k8s/scripts/setup` plus the kustomize overlays for remote environments. diff --git a/k8s/base/functions/send-sms.yaml b/k8s/base/functions/send-sms.yaml deleted file mode 100644 index 94df8282d..000000000 --- a/k8s/base/functions/send-sms.yaml +++ /dev/null @@ -1,70 +0,0 @@ -# This manifest is intentionally consumed only by the local Knative overlay, -# which patches the function into development dry-run mode. Do not add it to -# k8s/base/kustomization.yaml until a production SMS provider is available. -apiVersion: serving.knative.dev/v1 -kind: Service -metadata: - name: send-sms - labels: - app.kubernetes.io/name: send-sms - app.kubernetes.io/component: function - app.kubernetes.io/part-of: constructive-jobs - networking.knative.dev/visibility: cluster-local -spec: - template: - metadata: - labels: - app.kubernetes.io/name: send-sms - app.kubernetes.io/component: function - app.kubernetes.io/part-of: constructive-jobs - annotations: - autoscaling.knative.dev/minScale: "1" - autoscaling.knative.dev/maxScale: "10" - autoscaling.knative.dev/target: "50" - serving.knative.dev/timeout: "300s" - run.googleapis.com/cpu-throttling: "false" - spec: - containerConcurrency: 10 - timeoutSeconds: 300 - - containers: - - name: function - image: ghcr.io/constructive-io/constructive:e0b55cc - imagePullPolicy: Always - - command: ["node"] - args: ["functions/send-sms/dist/index.js"] - - ports: - - containerPort: 8080 - protocol: TCP - - env: - - name: NODE_ENV - value: "production" - - name: LOG_LEVEL - value: "debug" - - name: LOG_TIMESTAMP - value: "true" - - name: SMS_REQUEST_TIMEOUT_MS - value: "5000" - - resources: - requests: - memory: "128Mi" - cpu: "100m" - limits: - memory: "512Mi" - cpu: "500m" - - volumeMounts: - - name: tmp - mountPath: /tmp - - volumes: - - name: tmp - emptyDir: {} - - traffic: - - percent: 100 - latestRevision: true diff --git a/k8s/overlays/local/constructive/knative-job-service.yaml b/k8s/overlays/local/constructive/knative-job-service.yaml index ae82d6f6a..7dcbc4c20 100644 --- a/k8s/overlays/local/constructive/knative-job-service.yaml +++ b/k8s/overlays/local/constructive/knative-job-service.yaml @@ -10,8 +10,6 @@ spec: env: - name: NODE_ENV value: "development" - - name: JOBS_SUPPORTED - value: "email:send_email,email:send_verification_link,sms:send_verification_code" - name: KNATIVE_SERVICE_URL value: "constructive-functions.svc.cluster.local" - name: INTERNAL_JOBS_CALLBACK_URL @@ -23,7 +21,7 @@ spec: - name: INTERNAL_GATEWAY_URL value: "http://send-verification-link.constructive-functions.svc.cluster.local" - name: INTERNAL_GATEWAY_DEVELOPMENT_MAP - value: '{"email:send_email":"http://send-email.constructive-functions.svc.cluster.local","email:send_verification_link":"http://send-verification-link.constructive-functions.svc.cluster.local","sms:send_verification_code":"http://send-sms.constructive-functions.svc.cluster.local"}' + value: '{"email:send_email":"http://send-email.constructive-functions.svc.cluster.local","email:send_verification_link":"http://send-verification-link.constructive-functions.svc.cluster.local"}' resources: requests: cpu: "50m" diff --git a/k8s/overlays/local/kustomization.yaml b/k8s/overlays/local/kustomization.yaml index 3a132fc06..35f58afee 100644 --- a/k8s/overlays/local/kustomization.yaml +++ b/k8s/overlays/local/kustomization.yaml @@ -26,7 +26,6 @@ resources: # Functions - ../../base/functions/send-email.yaml - ../../base/functions/send-verification-link.yaml - - ../../base/functions/send-sms.yaml patches: # Skaffold image replacement: only rewrite function images, not server/dashboard/db-job @@ -48,15 +47,6 @@ patches: version: v1 kind: Service name: send-verification-link - - patch: |- - - op: replace - path: /spec/template/spec/containers/0/image - value: constructive-functions:local - target: - group: serving.knative.dev - version: v1 - kind: Service - name: send-sms - patch: |- - op: replace path: /spec/template/spec/containers/0/image @@ -96,17 +86,3 @@ patches: version: v1 kind: Service name: send-verification-link - - patch: |- - - op: replace - path: /spec/template/spec/containers/0/env/0/value - value: "development" - - op: add - path: /spec/template/spec/containers/0/env/- - value: - name: SEND_SMS_DRY_RUN - value: "true" - target: - group: serving.knative.dev - version: v1 - kind: Service - name: send-sms diff --git a/packages/fn-generator/__tests__/snapshot.test.ts b/packages/fn-generator/__tests__/snapshot.test.ts index 6f1c1f7bf..49189b01e 100644 --- a/packages/fn-generator/__tests__/snapshot.test.ts +++ b/packages/fn-generator/__tests__/snapshot.test.ts @@ -1,5 +1,6 @@ import * as fs from 'fs'; import * as path from 'path'; + import { FnGenerator } from '../src'; /** @@ -12,6 +13,7 @@ import { FnGenerator } from '../src'; const ROOT = path.resolve(__dirname, '..', '..', '..'); const FUNCTIONS_DIR = path.join(ROOT, 'functions'); const TEMPLATES_DIR = path.join(ROOT, 'templates'); +const K8S_DIR = path.join(ROOT, 'k8s'); const BASELINE_GENERATED = path.join(ROOT, 'generated'); const BASELINE_SKAFFOLD = path.join(ROOT, 'skaffold.yaml'); @@ -52,6 +54,7 @@ describe('FnGenerator snapshot vs scripts/generate.ts', () => { fs.symlinkSync(FUNCTIONS_DIR, path.join(tmpRoot, 'functions')); fs.symlinkSync(TEMPLATES_DIR, path.join(tmpRoot, 'templates')); + fs.symlinkSync(K8S_DIR, path.join(tmpRoot, 'k8s')); const gen = new FnGenerator({ rootDir: tmpRoot }); gen.generate(); @@ -88,4 +91,15 @@ describe('FnGenerator snapshot vs scripts/generate.ts', () => { const b = fs.readFileSync(BASELINE_SKAFFOLD, 'utf-8'); expect(a).toBe(b); }); + + it('keeps generated functions in plain k8s without exposing them through Knative', () => { + const skaffold = fs.readFileSync(path.join(tmpRoot, 'skaffold.yaml'), 'utf-8'); + const localProfile = skaffold.slice(skaffold.indexOf(' - name: local\n')); + + expect(skaffold).toContain(' - name: send-sms\n'); + expect(skaffold).toContain('generated/send-sms/k8s/local-deployment.yaml'); + expect(localProfile).toContain('resourceName: send-email'); + expect(localProfile).toContain('resourceName: send-verification-link'); + expect(localProfile).not.toContain('resourceName: send-sms'); + }); }); diff --git a/packages/fn-generator/src/builders/skaffold.ts b/packages/fn-generator/src/builders/skaffold.ts index c0d306975..f1dac373c 100644 --- a/packages/fn-generator/src/builders/skaffold.ts +++ b/packages/fn-generator/src/builders/skaffold.ts @@ -1,5 +1,6 @@ import * as fs from 'fs'; import * as path from 'path'; + import { renderTemplate } from '../placeholders'; import type { FunctionInfo, Manifest } from '../types'; @@ -57,12 +58,32 @@ export const buildSkaffold = (args: { ) .join('\n'); + // local-simple deploys every generated function; the optional Knative + // profile only exposes functions with an explicit tracked manifest. + const knativePortForwards = args.fns + .filter((fn) => + fs.existsSync( + path.join(args.rootDir, 'k8s', 'base', 'functions', `${fn.name}.yaml`) + ) + ) + .map((fn) => + [ + ' - resourceType: service', + ` resourceName: ${fn.name}`, + ` namespace: ${args.namespace}`, + ' port: 80', + ` localPort: ${fn.port}`, + ].join('\n') + ) + .join('\n'); + const pythonArtifacts = args.fns.some((fn) => fn.type === 'python') ? PYTHON_ARTIFACTS : ''; const skaffold = renderTemplate(main, { per_function_profiles: perFnProfiles, all_raw_yaml: allRawYaml, all_port_forwards: allPortForwards, + knative_port_forwards: knativePortForwards, python_artifacts: pythonArtifacts, namespace: args.namespace, }); diff --git a/scripts/generate.ts b/scripts/generate.ts index c4f038520..eae089ad3 100644 --- a/scripts/generate.ts +++ b/scripts/generate.ts @@ -254,6 +254,20 @@ function generateSkaffoldYaml(fns: FunctionInfo[]): void { ` localPort: ${fn.port}`, ].join('\n')) .join('\n'); + // The plain-k8s profiles support every generated function. The optional + // Knative profile only exposes functions with an explicit tracked manifest. + const knativePortForwards = fns + .filter((fn) => + fs.existsSync(path.join(ROOT, 'k8s', 'base', 'functions', `${fn.name}.yaml`)) + ) + .map((fn) => [ + ' - resourceType: service', + ` resourceName: ${fn.name}`, + ` namespace: ${K8S_NAMESPACE}`, + ' port: 80', + ` localPort: ${fn.port}`, + ].join('\n')) + .join('\n'); // Add Python artifacts if there are Python functions const hasPython = fns.some((fn) => fn.type === 'python'); @@ -272,6 +286,7 @@ function generateSkaffoldYaml(fns: FunctionInfo[]): void { per_function_profiles: perFnProfiles, all_raw_yaml: allRawYaml, all_port_forwards: allPortForwards, + knative_port_forwards: knativePortForwards, python_artifacts: pythonArtifacts, namespace: K8S_NAMESPACE, }); diff --git a/skaffold.yaml b/skaffold.yaml index 350a55bdd..3921af0a2 100644 --- a/skaffold.yaml +++ b/skaffold.yaml @@ -387,36 +387,16 @@ profiles: paths: - k8s/overlays/local portForward: - - resourceType: service - resourceName: knative-job-example - namespace: constructive-functions - port: 80 - localPort: 8083 - - resourceType: service - resourceName: python-example - namespace: constructive-functions - port: 80 - localPort: 8084 - resourceType: service resourceName: send-email namespace: constructive-functions port: 80 localPort: 8081 - - resourceType: service - resourceName: send-sms - namespace: constructive-functions - port: 80 - localPort: 8086 - resourceType: service resourceName: send-verification-link namespace: constructive-functions port: 80 localPort: 8082 - - resourceType: service - resourceName: sql-example - namespace: constructive-functions - port: 80 - localPort: 8085 - resourceType: service resourceName: knative-job-service namespace: constructive-functions diff --git a/templates/k8s/skaffold.yaml b/templates/k8s/skaffold.yaml index c0e0e87e5..b819b5f33 100644 --- a/templates/k8s/skaffold.yaml +++ b/templates/k8s/skaffold.yaml @@ -71,7 +71,7 @@ profiles: paths: - k8s/overlays/local portForward: -{{all_port_forwards}} +{{knative_port_forwards}} - resourceType: service resourceName: knative-job-service namespace: {{namespace}} From 5990878f3b3d82569e4605a7560910777d19aef3 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 28 Jul 2026 11:04:25 +0800 Subject: [PATCH 12/12] Revert "refactor: keep SMS deployment in dry-run scope" This reverts commit ef53aff6794d61a7b87b01bed738344a4773639d. --- DEVELOPMENT.md | 14 ++-- docs/skills/local-dev-skaffold.md | 2 +- k8s/ARCHITECTURE.md | 3 +- k8s/base/functions/send-sms.yaml | 70 +++++++++++++++++++ .../constructive/knative-job-service.yaml | 4 +- k8s/overlays/local/kustomization.yaml | 24 +++++++ .../fn-generator/__tests__/snapshot.test.ts | 14 ---- .../fn-generator/src/builders/skaffold.ts | 21 ------ scripts/generate.ts | 15 ---- skaffold.yaml | 20 ++++++ templates/k8s/skaffold.yaml | 2 +- 11 files changed, 127 insertions(+), 62 deletions(-) create mode 100644 k8s/base/functions/send-sms.yaml diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 2589ff4bf..f8ebdfc72 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -281,16 +281,13 @@ This runs `skaffold dev -p local-simple` which: 5. Watches `functions/**/*.ts` — edits are synced into running containers 6. `tsx --watch` inside each function container detects changes and restarts -The plain-k8s profiles run `send-sms` with `SEND_SMS_DRY_RUN=true`; they -verify job routing and handler completion without deploying DevSms. SMS is -not included in the optional Knative profile while no production provider is -available. The Hub cross-repository E2E owns DevSms delivery and -message-content assertions. +Kubernetes profiles run `send-sms` with `SEND_SMS_DRY_RUN=true`; they verify +job routing and handler completion without deploying DevSms. The Hub +cross-repository E2E owns DevSms delivery and message-content assertions. ### Option B: Knative -Uses Knative Serving for functions that have an explicit tracked Knative -manifest (currently the email functions). Requires Knative + Kourier. +Uses Knative Serving for functions (production parity). Requires Knative + Kourier. ```bash # One-time setup @@ -316,6 +313,9 @@ Changes to runtime packages (`packages/fn-runtime`, `packages/fn-app`) or `packa |---------|------------| | send-email | 8081 | | send-verification-link | 8082 | +| send-sms | 8086 | +| knative-job-example | 8083 | +| python-example | 8084 | | Job Service | 8080 | | PostgreSQL | 5432 | | Constructive Server | 3002 | diff --git a/docs/skills/local-dev-skaffold.md b/docs/skills/local-dev-skaffold.md index 84c052cce..315927712 100644 --- a/docs/skills/local-dev-skaffold.md +++ b/docs/skills/local-dev-skaffold.md @@ -65,7 +65,7 @@ Each function has its own Skaffold profile that deploys only the shared infrastr | `send-email` | infra + send-email | `skaffold dev -p send-email` | | `send-verification-link` | infra + send-verification-link | `skaffold dev -p send-verification-link` | | `local-simple` | infra + all functions | `skaffold dev -p local-simple` | -| `local` | Knative-enabled functions only | `skaffold dev -p local` | +| `local` | Knative + all functions | `skaffold dev -p local` | Profiles are auto-generated by `pnpm generate`. Adding a new function automatically creates its profile. diff --git a/k8s/ARCHITECTURE.md b/k8s/ARCHITECTURE.md index 0882c611b..16fb63b84 100644 --- a/k8s/ARCHITECTURE.md +++ b/k8s/ARCHITECTURE.md @@ -20,8 +20,7 @@ This document summarizes the Kubernetes layout under `k8s/`, highlights strength - `dashboard` (Next.js UI) — dev/staging. - `pgadmin` (DB admin UI) — dev/staging/local. - `knative-job-service` (job orchestration for functions). - - Email function workloads (`send-email`, `send-verification-link`) — Knative Services in `local`/`dev`/`staging`. - - All generated function workloads — plain Deployments in `local-simple`. + - Function workloads (`send-email`, `send-verification-link`, `knative-job-example`, `python-example`) — Knative Services in `local`/`dev`/`staging`, plain Deployments in `local-simple`. The repo-root `Makefile` wires this together via `make skaffold-dev` / `make skaffold-dev-knative` (local), and `k8s/scripts/setup` plus the kustomize overlays for remote environments. diff --git a/k8s/base/functions/send-sms.yaml b/k8s/base/functions/send-sms.yaml new file mode 100644 index 000000000..94df8282d --- /dev/null +++ b/k8s/base/functions/send-sms.yaml @@ -0,0 +1,70 @@ +# This manifest is intentionally consumed only by the local Knative overlay, +# which patches the function into development dry-run mode. Do not add it to +# k8s/base/kustomization.yaml until a production SMS provider is available. +apiVersion: serving.knative.dev/v1 +kind: Service +metadata: + name: send-sms + labels: + app.kubernetes.io/name: send-sms + app.kubernetes.io/component: function + app.kubernetes.io/part-of: constructive-jobs + networking.knative.dev/visibility: cluster-local +spec: + template: + metadata: + labels: + app.kubernetes.io/name: send-sms + app.kubernetes.io/component: function + app.kubernetes.io/part-of: constructive-jobs + annotations: + autoscaling.knative.dev/minScale: "1" + autoscaling.knative.dev/maxScale: "10" + autoscaling.knative.dev/target: "50" + serving.knative.dev/timeout: "300s" + run.googleapis.com/cpu-throttling: "false" + spec: + containerConcurrency: 10 + timeoutSeconds: 300 + + containers: + - name: function + image: ghcr.io/constructive-io/constructive:e0b55cc + imagePullPolicy: Always + + command: ["node"] + args: ["functions/send-sms/dist/index.js"] + + ports: + - containerPort: 8080 + protocol: TCP + + env: + - name: NODE_ENV + value: "production" + - name: LOG_LEVEL + value: "debug" + - name: LOG_TIMESTAMP + value: "true" + - name: SMS_REQUEST_TIMEOUT_MS + value: "5000" + + resources: + requests: + memory: "128Mi" + cpu: "100m" + limits: + memory: "512Mi" + cpu: "500m" + + volumeMounts: + - name: tmp + mountPath: /tmp + + volumes: + - name: tmp + emptyDir: {} + + traffic: + - percent: 100 + latestRevision: true diff --git a/k8s/overlays/local/constructive/knative-job-service.yaml b/k8s/overlays/local/constructive/knative-job-service.yaml index 7dcbc4c20..ae82d6f6a 100644 --- a/k8s/overlays/local/constructive/knative-job-service.yaml +++ b/k8s/overlays/local/constructive/knative-job-service.yaml @@ -10,6 +10,8 @@ spec: env: - name: NODE_ENV value: "development" + - name: JOBS_SUPPORTED + value: "email:send_email,email:send_verification_link,sms:send_verification_code" - name: KNATIVE_SERVICE_URL value: "constructive-functions.svc.cluster.local" - name: INTERNAL_JOBS_CALLBACK_URL @@ -21,7 +23,7 @@ spec: - name: INTERNAL_GATEWAY_URL value: "http://send-verification-link.constructive-functions.svc.cluster.local" - name: INTERNAL_GATEWAY_DEVELOPMENT_MAP - value: '{"email:send_email":"http://send-email.constructive-functions.svc.cluster.local","email:send_verification_link":"http://send-verification-link.constructive-functions.svc.cluster.local"}' + value: '{"email:send_email":"http://send-email.constructive-functions.svc.cluster.local","email:send_verification_link":"http://send-verification-link.constructive-functions.svc.cluster.local","sms:send_verification_code":"http://send-sms.constructive-functions.svc.cluster.local"}' resources: requests: cpu: "50m" diff --git a/k8s/overlays/local/kustomization.yaml b/k8s/overlays/local/kustomization.yaml index 35f58afee..3a132fc06 100644 --- a/k8s/overlays/local/kustomization.yaml +++ b/k8s/overlays/local/kustomization.yaml @@ -26,6 +26,7 @@ resources: # Functions - ../../base/functions/send-email.yaml - ../../base/functions/send-verification-link.yaml + - ../../base/functions/send-sms.yaml patches: # Skaffold image replacement: only rewrite function images, not server/dashboard/db-job @@ -47,6 +48,15 @@ patches: version: v1 kind: Service name: send-verification-link + - patch: |- + - op: replace + path: /spec/template/spec/containers/0/image + value: constructive-functions:local + target: + group: serving.knative.dev + version: v1 + kind: Service + name: send-sms - patch: |- - op: replace path: /spec/template/spec/containers/0/image @@ -86,3 +96,17 @@ patches: version: v1 kind: Service name: send-verification-link + - patch: |- + - op: replace + path: /spec/template/spec/containers/0/env/0/value + value: "development" + - op: add + path: /spec/template/spec/containers/0/env/- + value: + name: SEND_SMS_DRY_RUN + value: "true" + target: + group: serving.knative.dev + version: v1 + kind: Service + name: send-sms diff --git a/packages/fn-generator/__tests__/snapshot.test.ts b/packages/fn-generator/__tests__/snapshot.test.ts index 49189b01e..6f1c1f7bf 100644 --- a/packages/fn-generator/__tests__/snapshot.test.ts +++ b/packages/fn-generator/__tests__/snapshot.test.ts @@ -1,6 +1,5 @@ import * as fs from 'fs'; import * as path from 'path'; - import { FnGenerator } from '../src'; /** @@ -13,7 +12,6 @@ import { FnGenerator } from '../src'; const ROOT = path.resolve(__dirname, '..', '..', '..'); const FUNCTIONS_DIR = path.join(ROOT, 'functions'); const TEMPLATES_DIR = path.join(ROOT, 'templates'); -const K8S_DIR = path.join(ROOT, 'k8s'); const BASELINE_GENERATED = path.join(ROOT, 'generated'); const BASELINE_SKAFFOLD = path.join(ROOT, 'skaffold.yaml'); @@ -54,7 +52,6 @@ describe('FnGenerator snapshot vs scripts/generate.ts', () => { fs.symlinkSync(FUNCTIONS_DIR, path.join(tmpRoot, 'functions')); fs.symlinkSync(TEMPLATES_DIR, path.join(tmpRoot, 'templates')); - fs.symlinkSync(K8S_DIR, path.join(tmpRoot, 'k8s')); const gen = new FnGenerator({ rootDir: tmpRoot }); gen.generate(); @@ -91,15 +88,4 @@ describe('FnGenerator snapshot vs scripts/generate.ts', () => { const b = fs.readFileSync(BASELINE_SKAFFOLD, 'utf-8'); expect(a).toBe(b); }); - - it('keeps generated functions in plain k8s without exposing them through Knative', () => { - const skaffold = fs.readFileSync(path.join(tmpRoot, 'skaffold.yaml'), 'utf-8'); - const localProfile = skaffold.slice(skaffold.indexOf(' - name: local\n')); - - expect(skaffold).toContain(' - name: send-sms\n'); - expect(skaffold).toContain('generated/send-sms/k8s/local-deployment.yaml'); - expect(localProfile).toContain('resourceName: send-email'); - expect(localProfile).toContain('resourceName: send-verification-link'); - expect(localProfile).not.toContain('resourceName: send-sms'); - }); }); diff --git a/packages/fn-generator/src/builders/skaffold.ts b/packages/fn-generator/src/builders/skaffold.ts index f1dac373c..c0d306975 100644 --- a/packages/fn-generator/src/builders/skaffold.ts +++ b/packages/fn-generator/src/builders/skaffold.ts @@ -1,6 +1,5 @@ import * as fs from 'fs'; import * as path from 'path'; - import { renderTemplate } from '../placeholders'; import type { FunctionInfo, Manifest } from '../types'; @@ -58,32 +57,12 @@ export const buildSkaffold = (args: { ) .join('\n'); - // local-simple deploys every generated function; the optional Knative - // profile only exposes functions with an explicit tracked manifest. - const knativePortForwards = args.fns - .filter((fn) => - fs.existsSync( - path.join(args.rootDir, 'k8s', 'base', 'functions', `${fn.name}.yaml`) - ) - ) - .map((fn) => - [ - ' - resourceType: service', - ` resourceName: ${fn.name}`, - ` namespace: ${args.namespace}`, - ' port: 80', - ` localPort: ${fn.port}`, - ].join('\n') - ) - .join('\n'); - const pythonArtifacts = args.fns.some((fn) => fn.type === 'python') ? PYTHON_ARTIFACTS : ''; const skaffold = renderTemplate(main, { per_function_profiles: perFnProfiles, all_raw_yaml: allRawYaml, all_port_forwards: allPortForwards, - knative_port_forwards: knativePortForwards, python_artifacts: pythonArtifacts, namespace: args.namespace, }); diff --git a/scripts/generate.ts b/scripts/generate.ts index eae089ad3..c4f038520 100644 --- a/scripts/generate.ts +++ b/scripts/generate.ts @@ -254,20 +254,6 @@ function generateSkaffoldYaml(fns: FunctionInfo[]): void { ` localPort: ${fn.port}`, ].join('\n')) .join('\n'); - // The plain-k8s profiles support every generated function. The optional - // Knative profile only exposes functions with an explicit tracked manifest. - const knativePortForwards = fns - .filter((fn) => - fs.existsSync(path.join(ROOT, 'k8s', 'base', 'functions', `${fn.name}.yaml`)) - ) - .map((fn) => [ - ' - resourceType: service', - ` resourceName: ${fn.name}`, - ` namespace: ${K8S_NAMESPACE}`, - ' port: 80', - ` localPort: ${fn.port}`, - ].join('\n')) - .join('\n'); // Add Python artifacts if there are Python functions const hasPython = fns.some((fn) => fn.type === 'python'); @@ -286,7 +272,6 @@ function generateSkaffoldYaml(fns: FunctionInfo[]): void { per_function_profiles: perFnProfiles, all_raw_yaml: allRawYaml, all_port_forwards: allPortForwards, - knative_port_forwards: knativePortForwards, python_artifacts: pythonArtifacts, namespace: K8S_NAMESPACE, }); diff --git a/skaffold.yaml b/skaffold.yaml index 3921af0a2..350a55bdd 100644 --- a/skaffold.yaml +++ b/skaffold.yaml @@ -387,16 +387,36 @@ profiles: paths: - k8s/overlays/local portForward: + - resourceType: service + resourceName: knative-job-example + namespace: constructive-functions + port: 80 + localPort: 8083 + - resourceType: service + resourceName: python-example + namespace: constructive-functions + port: 80 + localPort: 8084 - resourceType: service resourceName: send-email namespace: constructive-functions port: 80 localPort: 8081 + - resourceType: service + resourceName: send-sms + namespace: constructive-functions + port: 80 + localPort: 8086 - resourceType: service resourceName: send-verification-link namespace: constructive-functions port: 80 localPort: 8082 + - resourceType: service + resourceName: sql-example + namespace: constructive-functions + port: 80 + localPort: 8085 - resourceType: service resourceName: knative-job-service namespace: constructive-functions diff --git a/templates/k8s/skaffold.yaml b/templates/k8s/skaffold.yaml index b819b5f33..c0e0e87e5 100644 --- a/templates/k8s/skaffold.yaml +++ b/templates/k8s/skaffold.yaml @@ -71,7 +71,7 @@ profiles: paths: - k8s/overlays/local portForward: -{{knative_port_forwards}} +{{all_port_forwards}} - resourceType: service resourceName: knative-job-service namespace: {{namespace}}