refactor: rename email functions to send-verification-link and send-email#44
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Renames the two email-related functions to purpose-driven names and introduces a taskIdentifier field so the job worker can dispatch using namespaced task identifiers (e.g., email:send_email) while keeping k8s service/dir names unchanged. This aligns constructive-functions with upstream job identifiers and updates codegen, local dev, k8s manifests, and tests accordingly.
Changes:
- Renamed
simple-email→send-emailandsend-email-link→send-verification-linkacross functions, k8s, skaffold, docs, and tests. - Added optional
taskIdentifierpropagation through manifests/configmaps soJOBS_SUPPORTED+ gateway maps can use namespaced identifiers. - Switched
job/servicefunction package deps toworkspace:^so local generated packages are used instead of upstream npm versions.
Reviewed changes
Copilot reviewed 55 out of 57 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/job-registry.test.ts | Updates registry expectations for renamed functions/packages. |
| tests/e2e/README.md | Updates e2e prerequisites to new function names. |
| tests/e2e/tests/send-verification-link.e2e.test.ts | Updates e2e job identifier to email:send_verification_link. |
| tests/e2e/tests/send-email.e2e.test.ts | Updates e2e job identifier to email:send_email. |
| tests/e2e/tests/job-processing.test.ts | Updates references to per-function e2e filenames. |
| skaffold.yaml | Renames profiles/rawYaml paths/services/ports for the two functions. |
| scripts/generate.ts | Adds taskIdentifier support to configmap generation (JOBS_SUPPORTED + gateway map). |
| scripts/docker-build.ts | Updates usage examples to renamed functions. |
| scripts/dev.ts | Adds taskIdentifier handling for job-service env + updates dry-run env vars. |
| README.md | Updates function list, env vars, and rename references. |
| pnpm-lock.yaml | Updates generated workspace importers and job/service deps to new local workspaces. |
| packages/fn-types/src/registry.ts | Adds taskIdentifier to registry entry type docs. |
| packages/fn-types/src/manifest.ts | Adds taskIdentifier to HandlerManifest. |
| packages/fn-types/src/index.ts | Reorders exports and includes updated config exports. |
| packages/fn-generator/src/builders/manifest.ts | Emits taskIdentifier into generated functions-manifest.json when present. |
| packages/fn-generator/src/builders/configmap.ts | Uses taskIdentifier for JOBS_SUPPORTED + gateway map keys. |
| packages/fn-generator/README.md | Updates generator example to new function name. |
| packages/fn-client/tests/client.test.ts | Updates discovery test to expect renamed functions. |
| Makefile | Updates skaffold/docker build helper comments to new names. |
| k8s/SECRETS-ARCHITECTURE.md | Updates function name examples. |
| k8s/overlays/local/kustomization.yaml | Updates function resources/patch targets/env var names. |
| k8s/overlays/local/constructive/knative-job-service.yaml | Updates INTERNAL_GATEWAY_* map and base URL to namespaced task IDs. |
| k8s/overlays/local-simple/config.yaml | Updates dry-run env var keys for renamed functions. |
| k8s/overlays/ci/kustomization.yaml | Updates comment referencing long-lived function for e2e. |
| k8s/overlays/ci/delete-send-email.yaml | Renames the deleted Knative Service from simple-email to send-email. |
| k8s/manifests/interweb-staging.yaml | Updates service names/args and job routing env to namespaced task IDs. |
| k8s/manifests/interweb-local.yaml | Updates service names/env vars and job routing env to namespaced task IDs. |
| k8s/manifests/interweb-dev.yaml | Updates service names/args and job routing env to namespaced task IDs. |
| k8s/manifests/interweb-ci.yaml | Updates service names/env vars and job routing env to namespaced task IDs. |
| k8s/base/kustomization.yaml | Points base kustomization to renamed function manifests. |
| k8s/base/functions/send-verification-link.yaml | Renames Knative Service and container args path. |
| k8s/base/functions/send-email.yaml | Renames Knative Service and container args path. |
| k8s/base/constructive/knative-job-service.yaml | Updates JOBS_SUPPORTED + INTERNAL_GATEWAY_* for namespaced task IDs. |
| k8s/ARCHITECTURE.md | Updates architecture doc references and example job names. |
| job/service/src/registry.ts | Updates inline example to renamed function. |
| job/service/src/index.ts | Import reordering (eslint). |
| job/service/package.json | Switches email function deps to workspace:^ for local generated packages. |
| functions/send-verification-link/types.d.ts | Adds untyped module stub for @launchql/mjml. |
| functions/send-verification-link/handler.ts | Renames log tags and updates dry-run env var (with legacy fallback). |
| functions/send-verification-link/handler.json | Renames function and adds taskIdentifier: email:send_verification_link. |
| functions/send-verification-link/tests/handler.test.ts | Updates test suite naming to renamed function. |
| functions/send-verification-link/tests/handler.graphql.test.ts | Updates test description naming to renamed function. |
| functions/send-email/handler.ts | Renames payload type/log tag and updates dry-run env var (with legacy fallback). |
| functions/send-email/handler.json | Renames function and adds taskIdentifier: email:send_email. |
| functions/send-email/tests/handler.test.ts | Updates env var usage and suite naming for renamed function. |
| docs/spec/function-templating.md | Updates examples and references to renamed functions and env vars. |
| docs/skills/local-dev-skaffold.md | Updates skaffold profiles, ports, and example commands for renamed functions. |
| docs/skills/adding-functions.md | Updates reference implementations and examples to renamed functions. |
| docs/portable-functions-toolkit.md | Updates hub integration checklist item to renamed function. |
| docs/plan/04-env-handling.md | Updates plan doc examples to renamed functions/env vars. |
| docs/plan/03-function-registry.md | Updates plan doc examples to renamed functions. |
| docs/plan/02-testing-strategy.md | Updates plan doc references/examples to renamed functions/env vars. |
| docs/plan/01-docker-ci.md | Updates plan doc examples to renamed functions and image names. |
| docs/plan/00-overview.md | Updates plan overview list of included functions. |
| DEVELOPMENT.md | Updates dev guide references/paths/ports for renamed functions. |
| CLAUDE.md | Updates repo summary/commands to renamed functions. |
| AGENTS.md | Updates examples and key details to renamed functions and new env vars. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
docs/skills/adding-functions.md:100
- This section instructs readers to add an entry to a hardcoded
functionRegistryinjob/service/src/index.ts, but the job service now loads its registry from the generated manifest vialoadFunctionRegistry(). Please update the instructions to reflect the manifest-driven flow (e.g., add handler.json + runpnpm generate).
Update `job/service/src/index.ts` — add an entry to `functionRegistry`:
```typescript
'<name>': {
moduleName: '@constructive-io/<name>-fn',
defaultPort: <port>
},
</details>
---
💡 <a href="/constructive-io/constructive-functions/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
Comment on lines
1
to
8
| export type { | ||
| FunctionHandler, | ||
| FunctionContext, | ||
| FunctionLogger, | ||
| ServerOptions | ||
| } from './runtime'; | ||
|
|
||
| export type { HandlerManifest } from './manifest'; | ||
|
|
||
| export type { FnRegistry, FnRegistryEntry } from './registry'; | ||
|
|
||
| export type { | ||
| DockerOptions, | ||
| FnConfig, | ||
| FnPreset, | ||
| K8sTarget, | ||
| K8sOptions, | ||
| K8sResourceQuantities, | ||
| DockerOptions | ||
| } from './config'; | ||
|
|
||
| K8sTarget} from './config'; | ||
| export { defineConfig } from './config'; |
Comment on lines
+61
to
+62
| | `generated/send-verification-link/dist/` | Send-email-link function server | | ||
| | `generated/send-email/dist/` | Simple-email function server | |
Comment on lines
306
to
312
| ## Compatibility | ||
|
|
||
| The generated packages maintain full backward compatibility with: | ||
|
|
||
| - **job/service `loadFunctionApp()`**: Expects module to export an app with `.listen()`. The generated `index.ts` exports exactly this via `createFunctionServer()` which wraps `createJobApp()`. | ||
| - **Package names**: `@constructive-io/simple-email-fn`, `@constructive-io/send-email-link-fn` remain unchanged. | ||
| - **Package names**: `@constructive-io/send-email-fn`, `@constructive-io/send-verification-link-fn` remain unchanged. | ||
| - **K8s manifests**: `node generated/<name>/dist/index.js` works as container CMD. |
Comment on lines
13
to
18
| **send-email** (`functions/send-email/handler.ts` lines 30-31): | ||
| ```typescript | ||
| const isDryRun = parseEnvBoolean(process.env.SIMPLE_EMAIL_DRY_RUN) ?? false; | ||
| const isDryRun = parseEnvBoolean(process.env.SEND_EMAIL_DRY_RUN) ?? false; | ||
| const useSmtp = parseEnvBoolean(process.env.EMAIL_SEND_USE_SMTP) ?? false; | ||
| ``` | ||
| Also reads: `process.env.SMTP_FROM`, `process.env.MAILGUN_FROM` (lines 45-50). |
Comment on lines
20
to
25
| **send-verification-link** (`functions/send-verification-link/handler.ts` lines 73-74): | ||
| ```typescript | ||
| const isDryRun = parseEnvBoolean(env.SEND_EMAIL_LINK_DRY_RUN) ?? false; | ||
| const isDryRun = parseEnvBoolean(env.SEND_VERIFICATION_LINK_DRY_RUN) ?? false; | ||
| const useSmtp = parseEnvBoolean(env.EMAIL_SEND_USE_SMTP) ?? false; | ||
| ``` | ||
| Also reads: `env.GRAPHQL_URL` (via fn-runtime), `env.LOCAL_APP_PORT` (line 152). |
Comment on lines
85
to
91
| ```typescript | ||
| export type FunctionName = 'simple-email' | 'send-email-link' | '<name>'; | ||
| export type FunctionName = 'send-email' | 'send-verification-link' | '<name>'; | ||
| ``` |
Comment on lines
+11
to
+16
| const functionRegistry: Record<FunctionName, FunctionRegistryEntry> = { | ||
| 'simple-email': { | ||
| moduleName: '@constructive-io/simple-email-fn', | ||
| 'send-email': { |
Comment on lines
27
to
33
| And the `FunctionName` type is a hardcoded union: | ||
| ```typescript | ||
| // job/service/src/types.ts line 1 | ||
| export type FunctionName = 'simple-email' | 'send-email-link'; | ||
| export type FunctionName = 'send-email' | 'send-verification-link'; | ||
| ``` | ||
|
|
||
| Functions are loaded in-process via `createRequire` (`index.ts:56-71`). Adding a new function requires editing 3 files. The current architecture tightly couples the service to specific function packages. |
Comment on lines
38
to
41
| | `send-email` | 8081 | node-graphql | `ghcr.io/constructive-io/constructive-functions/send-email:latest` | | ||
| | `send-verification-link` | 8082 | node-graphql | `ghcr.io/constructive-io/constructive-functions/send-verification-link:latest` | | ||
| | `knative-job-example` | 8083 | node-graphql | `ghcr.io/constructive-io/constructive-functions/knative-job-example:latest` | | ||
| | `python-example` | 8084 | python | `ghcr.io/constructive-io/constructive-functions/python-example:latest` | |
Comment on lines
117
to
120
| docker push ghcr.io/constructive-io/constructive-functions/send-email:latest | ||
| docker push ghcr.io/constructive-io/constructive-functions/send-verification-link:latest | ||
| docker push ghcr.io/constructive-io/constructive-functions/knative-job-example:latest | ||
| docker push ghcr.io/constructive-io/constructive-functions/python-example:latest |
Anmol1696
force-pushed
the
feat/rename-send-verification-link
branch
from
829face to
55f8874
Compare
…mail Match upstream constructive's purpose-driven names. Function dirs and k8s service names use plain identifiers; namespaced task identifiers (email:send_verification_link, email:send_email) live in worker env config (JOBS_SUPPORTED, INTERNAL_GATEWAY_DEVELOPMENT_MAP) and DB-inserted jobs from constructive-db. - functions/send-email-link -> functions/send-verification-link - functions/simple-email -> functions/send-email - handler.json: add optional taskIdentifier field; generator now keys JOBS_SUPPORTED and the gateway map on taskIdentifier (defaults to name) - HandlerManifest + FnRegistryEntry types: add taskIdentifier - job/service: workspace:^ deps on local generated packages so symlinks win over upstream npm versions - k8s/base + overlays + interweb flat manifests: rename services, fix env vars, set namespaced task identifiers - e2e + integration tests: rename, assert on namespaced task identifiers - docs sweep: README, CLAUDE.md, AGENTS.md, DEVELOPMENT.md, docs/, k8s/ - handlers keep backward-compat env-var fallbacks (SIMPLE_EMAIL_DRY_RUN, SEND_EMAIL_LINK_DRY_RUN) during the transition
Anmol1696
force-pushed
the
feat/rename-send-verification-link
branch
from
55f8874 to
86719b1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 53 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
functions/send-verification-link/handler.ts:88
- The helper is still named
sendEmailLink, which matches the old function name and is now misleading in stack traces/logs. Rename it (and corresponding call sites) to reflectsend-verification-linkfor clarity and maintainability.
Comment on lines
149
to
152
| SELECT * FROM app_jobs.add_job( | ||
| (SELECT id FROM metaschema_public.database LIMIT 1), | ||
| 'simple-email'::text, | ||
| 'send-email'::text, | ||
| '{"to":"test@example.com","subject":"test","html":"<p>hello</p>"}'::json |
Comment on lines
163
to
167
| -- Manually insert a test job | ||
| SELECT * FROM app_jobs.add_job( | ||
| 'simple-email'::text, | ||
| 'send-email'::text, | ||
| '{"to":"test@example.com","subject":"test","html":"<p>hello</p>"}'::json | ||
| ); |
Comment on lines
1267
to
+1269
| '@ungap/structured-clone@1.3.0': | ||
| resolution: {integrity: sha512-WmoN8qaIAo7WTYWbAZuG8PYEhn5fkz7dZrqTBZ7dtt//lL2Gwms1IcnQ5yHqjDfX8Ft5j4YzDM23f87zBfDe9g==} | ||
| deprecated: Potential CWE-502 - Update to 1.3.1 or higher |
Contributor
Author
|
Heads-up: repository history was rewritten on 2026-05-12 to scrub leaked secrets (Postgres/pgAdmin default passwords, an AWS access key ID, generated This PR shows "DIRTY" / merge-conflict status because git fetch --all --prune
git checkout <this-branch>
git reset --hard origin/<this-branch> # your local branch tip moved too — pick up the rewritten version
git rebase origin/main # rebase onto rewritten main
# resolve conflicts (usually trivial — mostly secret-placeholder + the 4 deleted interweb-*.yaml files)
git push --force-with-leaseOr merge instead of rebase if the branch is shared with others: git merge origin/main
git pushNotes:
|
The previous E2E failures were unrelated to this PR. constructive-db .dockerignore had excluded application/ since commit a1ac9a5508 (2026-05-06), which made constructive-db-job:latest ship without the constructive PG extension's source. pgpm fell back to CREATE EXTENSION constructive on a postgres image that didn't have the .control file, so every PR's CI on this workflow had been red since 2026-05-10. Fix landed in constructive-io/constructive-db#1138. constructive-db-job :latest has been rebuilt; manual workflow_dispatch run 25839642302 went green on all three E2E jobs. This empty commit just retriggers PR-event checks so the PR status reflects reality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
functions/send-email-link→functions/send-verification-linkandfunctions/simple-email→functions/send-emailto match upstreamconstructive's purpose-driven names (commitc3e0dee06).taskIdentifierfield inhandler.jsonso the generator can emit namespaced task identifiers (email:send_verification_link,email:send_email) inJOBS_SUPPORTEDandINTERNAL_GATEWAY_DEVELOPMENT_MAPwhile keeping plain dir/svc names for k8s. Defaults tonamewhen omitted, so it's a no-op for existing non-email functions.job/servicetoworkspace:^for the local function packages so the generated workspace shadows match (was resolving to upstream npm1.11.2/2.11.2).Why now
constructive-dbmigrations nowadd_job('email:send_verification_link', …)/add_job('email:send_email', …). Without this PR, the job-worker inconstructive-functionsignores those rows because it's hardcoded to supportsend-email-link/simple-email— and tries to dispatch to a function path that no longer exists.Layers touched
handler.json.name, addtaskIdentifier, log tags, test descriptions. Handlers retain backward-compat env-var fallbacks (SIMPLE_EMAIL_DRY_RUN,SEND_EMAIL_LINK_DRY_RUN) during transition.scripts/generate.ts+packages/fn-generator/src/builders/{configmap,manifest}.tspropagatetaskIdentifierthroughfunctions-manifest.jsonandfunctions-configmap.yaml.packages/fn-typesexposestaskIdentifieronHandlerManifest+FnRegistryEntry.interweb-{local,ci,dev,staging}.yamlupdated for renamed services, env vars,JOBS_SUPPORTED, and gateway map.email:send_verification_link/email:send_email. Integration test for the registry updated to new names.docs/**,k8s/*.md.Test plan
pnpm generateproducesfunctions-configmap.yamlwithJOBS_SUPPORTED: \"…,email:send_email,email:send_verification_link\"and namespaced gateway-map keys.pnpm installsymlinks@constructive-io/send-email-fn→generated/send-email(not upstream npm).pnpm build— all packages build.pnpm lint— clean (existing warnings unchanged).pnpm test:unit— 19 passed, 6 todo.pnpm test:integration— 14 passed.app_jobs.add_job('email:send_verification_link', …)job is picked up by the worker and POSTs tohttp://send-verification-link.../.Refs
constructiverename: commitc3e0dee06(PR #1124)constructive-dbtask-identifier rename: commit0ea649304c