fix(evo-flow): reuse primary app context in action-node activities (EVO-1829)#75
Merged
Conversation
…VO-1829) Journey/campaign action-node activities run outside the Nest DI container and each bootstrapped a full second AppModule via createApplicationContext(AppModule.forRoot()) to resolve services. In single-mode that boots a redundant Temporal worker + Kafka consumers + scheduled jobs in the same process and silently freezes the app on the first action node. Stash the primary application context at boot (app-context.holder) and resolve services from it via getAppContext() in the 4 journey action nodes (add-label, remove-label, update-custom-attribute, transfer-journey) and campaign-execution activities — no second AppModule is ever booted. Adds a regression guard spec forbidding the bootstrap pattern under activities/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideReuses the primary Nest application context for Temporal action-node and campaign-execution activities via a shared holder set at bootstrap, removing per-activity AppModule bootstraps and adding a regression test guard against reintroducing that pattern. Sequence diagram for reusing the primary Nest app context in Temporal activitiessequenceDiagram
participant Bootstrap as main_ts_bootstrap
participant NestFactory as NestFactory
participant AppContext as INestApplicationContext
participant Holder as app_context_holder
participant AddLabelNode as AddLabelNode
participant LabelsService as LabelsService
Bootstrap->>NestFactory: create()
NestFactory-->>Bootstrap: INestApplicationContext
Bootstrap->>Holder: setAppContext(AppContext)
AddLabelNode->>AddLabelNode: getServices()
AddLabelNode->>Holder: getAppContext()
Holder-->>AddLabelNode: AppContext
AddLabelNode->>AppContext: get(LabelsService)
AppContext-->>AddLabelNode: LabelsService
AddLabelNode->>LabelsService: addLabel(...)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
app-context.holderrelies on a mutable module-level singleton; consider makingsetAppContextdefensive (e.g., throw or warn if called more than once) to surface accidental double-bootstraps or reuse across tests early. - In the node classes (
AddLabelNode,RemoveLabelNode,UpdateCustomAttributeNode,TransferJourneyNode), the service fields are still typed asany; now that they resolve directly from the main DI context, you can tighten these to their concrete service types to catch wiring issues at compile time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `app-context.holder` relies on a mutable module-level singleton; consider making `setAppContext` defensive (e.g., throw or warn if called more than once) to surface accidental double-bootstraps or reuse across tests early.
- In the node classes (`AddLabelNode`, `RemoveLabelNode`, `UpdateCustomAttributeNode`, `TransferJourneyNode`), the service fields are still typed as `any`; now that they resolve directly from the main DI context, you can tighten these to their concrete service types to catch wiring issues at compile time.
## Individual Comments
### Comment 1
<location path="src/shared/app-context.holder.ts" line_range="20-22" />
<code_context>
+ */
+let appContext: INestApplicationContext | null = null;
+
+export function setAppContext(context: INestApplicationContext): void {
+ appContext = context;
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider guarding against multiple calls to `setAppContext` to detect unintended re-bootstrap/overwrites.
As written, a second Nest bootstrap calling `setAppContext` will silently replace the existing context, potentially masking bugs by swapping the DI graph at runtime. Consider guarding against this by throwing or at least logging when `appContext` is already set, e.g.:
```ts
if (appContext && appContext !== context) {
throw new Error('App context already initialized');
}
appContext = context;
```
```suggestion
export function setAppContext(context: INestApplicationContext): void {
if (appContext && appContext !== context) {
throw new Error('App context already initialized');
}
appContext = context;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+20
to
+22
| export function setAppContext(context: INestApplicationContext): void { | ||
| appContext = context; | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Consider guarding against multiple calls to setAppContext to detect unintended re-bootstrap/overwrites.
As written, a second Nest bootstrap calling setAppContext will silently replace the existing context, potentially masking bugs by swapping the DI graph at runtime. Consider guarding against this by throwing or at least logging when appContext is already set, e.g.:
if (appContext && appContext !== context) {
throw new Error('App context already initialized');
}
appContext = context;
Suggested change
| export function setAppContext(context: INestApplicationContext): void { | |
| appContext = context; | |
| } | |
| export function setAppContext(context: INestApplicationContext): void { | |
| if (appContext && appContext !== context) { | |
| throw new Error('App context already initialized'); | |
| } | |
| appContext = context; | |
| } |
dpaes
approved these changes
Jun 18, 2026
dpaes
left a comment
dpaes
left a comment
There was a problem hiding this comment.
✅ Approved — EVO-1829
Clean, well-scoped fix. The redundant per-node createApplicationContext(AppModule.forRoot()) bootstrap is fully removed; action-node and campaign-execution activities now resolve DI from the primary context stashed at boot (app-context.holder + setAppContext in main.ts), and a recursive guard spec forbids the pattern from returning.
Acceptance criteria
- AC1 (no freeze): ✅ root cause (second bootstrap) removed; live smoke confirms
trigger → add-label → exitwith the app staying up. - AC2 (DI still resolves): ✅ both old and new code build from the same
AppModule.forRoot()(no args, deterministic per process).Labels/Contacts/CustomAttributes/Journeys/Broker/Audience/Campaignsare all inbaseImports(loaded in every RUN_MODE), so the primary context exposes exactly the providers the second bootstrap did — including in split/worker mode. - AC3 (no 2nd worker/consumer/job): ✅ no
createApplicationContext/NestFactoryremains in the temporal tree; guard spec blocks regressions. - AC4 (visible failure, no process death): ✅ worst case is now a clear thrown error from
getAppContext()→ Temporal retry, not a silent freeze.
Adversarial checks (all pass)
- Single entrypoint: every mode (incl.
start:prod→dist/main) runsbootstrap()→setAppContext; no alternate worker entrypoint, so split/prod also populate the holder. - Timing:
setAppContextruns beforeapp.listen()/app.init()triggers the worker'sonModuleInit, so the holder is always populated before any activity dispatch. - Node specs: mock
getServicesdirectly and never touch the holder → unaffected; onlycampaign-execution.activities.spec.tsneeded updating, and it correctly mocks the holder now. - No orphan import;
transfer-journey.node.tsis clean. - Out-of-scope
journey-trackingnew KafkaService()(producer-only) correctly left untouched and documented in the holder.
Non-blocking
getAppContext()incampaign-execution.activities.tsis now a trivial async passthrough — could be inlined/sync. Cosmetic.- evo-flow CI is Sourcery-only (no jest/tsc/build gate), so
build/jest 64/64/prettierare self-reported — worth a localnpx jestconfirmation.
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
2 participants
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
Journey/campaign action nodes (Add Label, Remove Label, Update Custom Attribute, Transfer Journey) + campaign-execution activities run outside the Nest DI container. Each one bootstrapped a full second
AppModuleviaNestFactory.createApplicationContext(AppModule.forRoot())just to resolve a DI service. In single-mode that boots a redundant Temporal worker + Kafka consumers + ScheduleModule jobs in the same process — they collide and the app silently freezes the first time an action node runs per boot (HTTP server dies, session stuckactiveon the node). Same family as EVO-1737, different path.Fix (chosen via a design evaluation of 3 approaches): stash the primary application context at boot in a tiny module-level holder, and resolve services from it via
getAppContext(). No secondAppModuleis ever booted. The primary context already provides every service these activities need and is guaranteed to exist before any activity dispatch.Changes
src/shared/app-context.holder.ts(new):setAppContext/getAppContext(type-only import; fail-fast throw, never a silent hang).src/main.ts:setAppContext(app)right afterNestFactory.create()(before init/worker creation in all RUN_MODEs).campaign-execution.activities.ts: resolve DI viagetAppContext(); drop the per-nodecreateApplicationContext(AppModule.forRoot())bootstrap + dead imports.no-app-bootstrap.guard.spec.ts(new): regression guard forbidding the bootstrap pattern underactivities/.journey-tracking.activities.tsnew KafkaService()(deliberate producer-only force-init).Validation
evo-flow: npm run build→ cleanevo-flow: npx jest(guard + campaign + add/remove-label + update-custom-attribute + conditional + split) → 64/64evo-flow: npx prettier --check→ new files + main + campaign spec clean (the 4 node files + campaign activity had pre-existing prettier drift on develop, intentionally not reformatted to keep the diff focused)journey-trigger → add-label (started → completed) → exit, sessioncompleted, label applied, app stays up, and no second AppModule bootstrap appears in the logs.Acceptance Criteria
Changed Files
src/shared/app-context.holder.ts(new)src/main.tssrc/modules/temporal/activities/nodes/{add-label,remove-label,update-custom-attribute,transfer-journey}.node.tssrc/modules/temporal/activities/campaign-execution.activities.tssrc/modules/temporal/activities/campaign-execution.activities.spec.tssrc/modules/temporal/activities/no-app-bootstrap.guard.spec.ts(new)Linked Issue
🤖 Generated with Claude Code
Summary by Sourcery
Reuse the primary Nest application context for Temporal campaign and action-node activities instead of bootstrapping a second AppModule, preventing single-mode freezes and redundant workers.
Bug Fixes:
Enhancements:
Tests: