fix(evo-flow): preserve journey variables across the cache + refresh on write (EVO-1836)

[nickoliveira23]

Summary

Part of EVO-1836 (cross-repo — journey variables broken). The backend root cause of "variables never display": the journey cache reconstruction dropped variables.

• The journey cache is L1 (in-memory). journey-cache.service.ts stores variables (it's on CachedJourney + the set() mapping), but journeys.service.ts findOne() / findAll() / findActive() hand-rebuild the Journey from cache and omitted variables. So once a journey was cached (editor load), getJourneyVariables did journey.variables || [] → [].
• updateJourneyVariables saved to the DB but never refreshed the cache → stale [] served forever.

Fix:

• Include variables: cached.variables in all three cache reconstructions (findOne, findAll, findActive).
• updateJourneyVariables: cache.set(savedJourney) after save (matching update()/toggleActive()), instead of invalidate() — review found invalidate() removed the journey from the active/list index, dropping it from findActive() (event trigger matching) and findAll() until re-fetched.

Validation

• npm run build → clean
• npx jest journeys.service.spec → 5/5 (incl. a regression spec asserting findOne preserves cached variables)
• npx prettier --check → new lines clean (pre-existing any/format debt untouched)
• Live: API POST /journeys/:id/variables → GET returns the variable, including on a cache hit.
• Adversarial multi-agent review: the invalidate→set regression and the findActive omission were caught and fixed here; partial-save data-loss refuted.

Changed Files

• src/modules/journeys/journeys.service.ts
• src/modules/journeys/journeys.service.spec.ts

Related PRs

• evo-ai-frontend-community#168 (frontend half: envelope unwrap) — same EVO-1836.

Linked Issue

• EVO-1836

🤖 Generated with Claude Code

Summary by Sourcery

Preserve journey variables when serving cached journeys and refresh the cache after variable updates.

Bug Fixes:

• Ensure cached journeys returned by findOne/findAll/findActive include variables from the cache so journey variables are not dropped on cache hits.
• Refresh the journey cache with the updated entity after updating variables so subsequent reads return the latest variables instead of stale data.

Tests:

• Add a regression test verifying getJourneyVariables returns variables from a cached journey on a cache hit.

[sourcery-ai]

Reviewer's Guide

Ensures journey variables are preserved when serving journeys from the in-memory cache and that the cache is refreshed (not invalidated) after variable updates, preventing stale or empty variables on cache hits.

Sequence diagram for getJourneyVariables cache-hit preserving variables

sequenceDiagram
  actor Client
  participant JourneysService
  participant JourneyCacheService

  Client->>JourneysService: getJourneyVariables(journeyId)
  JourneysService->>JourneyCacheService: get(journeyId)
  JourneyCacheService-->>JourneysService: CachedJourney(with variables)
  JourneysService-->>Client: variables (cachedJourney.variables)

Loading

Sequence diagram for updateJourneyVariables refreshing cache

sequenceDiagram
  actor Client
  participant JourneysService
  participant JourneyRepository
  participant JourneyCacheService

  Client->>JourneysService: updateJourneyVariables(id, variables)
  JourneysService->>JourneyRepository: save(journey)
  JourneyRepository-->>JourneysService: updatedJourney(with variables)
  JourneysService->>JourneyCacheService: set(updatedJourney)
  JourneysService-->>Client: updatedJourney.variables || []

Loading

File-Level Changes

Change	Details	Files
Preserve variables when reconstructing Journey entities from cached journeys across all query paths.	Extend cache-to-entity reconstruction in findAll to include variables from the cached journey object. Extend cache-to-entity reconstruction in findOne to include variables from the cached journey object. Extend cache-to-entity reconstruction in findActive to include variables from the cached journey object.	src/modules/journeys/journeys.service.ts
Refresh journey cache on variable update instead of invalidating it, and return variables from the updated entity.	Capture the result of journeyRepository.save into a new updatedJourney variable when updating variables. Call journeyCacheService.set(updatedJourney) after saving to keep the cache and active/list indices in sync. Return updatedJourney.variables
Add regression test to ensure getJourneyVariables returns cached variables on a cache hit.	Create a new describe block for EVO-1836 validating getJourneyVariables behavior on cache hit. Mock JourneyCacheService.get to return a cached journey with variables populated. Instantiate JourneysService with the mocked cache, suppress logger.error noise, and assert getJourneyVariables returns the cached vars and calls cache.get with the journey id.	src/modules/journeys/journeys.service.spec.ts

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The cache-to-Journey reconstruction logic is now duplicated in three places with the new variables mapping; consider extracting a shared helper to reduce the chance of future field omissions or inconsistencies.
• In getJourneyVariables the behavior now relies on cached.variables being defined; if callers expect an array, you might want to normalize with a default (e.g., cached.variables ?? []) when writing to or reading from the cache.
• The new spec spies on (service as any).logger, which couples the test to a private implementation detail; consider exposing a logger via constructor injection or a dedicated method so tests can interact with it without any/private access.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The cache-to-Journey reconstruction logic is now duplicated in three places with the new `variables` mapping; consider extracting a shared helper to reduce the chance of future field omissions or inconsistencies.
- In `getJourneyVariables` the behavior now relies on `cached.variables` being defined; if callers expect an array, you might want to normalize with a default (e.g., `cached.variables ?? []`) when writing to or reading from the cache.
- The new spec spies on `(service as any).logger`, which couples the test to a private implementation detail; consider exposing a logger via constructor injection or a dedicated method so tests can interact with it without `any`/private access.

## Individual Comments

### Comment 1
<location path="src/modules/journeys/journeys.service.ts" line_range="419-423" />
<code_context>
       journey.variables = variables;

-      await this.journeyRepository.save(journey);
+      const updatedJourney = await this.journeyRepository.save(journey);

-      return journey.variables || [];
+      await this.journeyCacheService.set(updatedJourney);
+
+      return updatedJourney.variables || [];
     } catch (error) {
       this.logger.error(
</code_context>
<issue_to_address>
**question (bug_risk):** Clarify whether a cache write failure after a successful DB save should fail the whole updateVariables call.

Because `journeyCacheService.set` now runs after a successful DB save, a transient cache outage will cause `updateJourneyVariables` to throw even though the write has committed. This can produce confusing errors and make retries unsafe. If strict write-through semantics aren’t required, consider wrapping the cache write in a try/catch, logging failures, and still returning `updatedJourney.variables`. If strict DB–cache consistency is intentional, then keeping the current behavior is fine but should be confirmed with the expected semantics.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

question (bug_risk): Clarify whether a cache write failure after a successful DB save should fail the whole updateVariables call.

Because journeyCacheService.set now runs after a successful DB save, a transient cache outage will cause updateJourneyVariables to throw even though the write has committed. This can produce confusing errors and make retries unsafe. If strict write-through semantics aren’t required, consider wrapping the cache write in a try/catch, logging failures, and still returning updatedJourney.variables. If strict DB–cache consistency is intentional, then keeping the current behavior is fine but should be confirmed with the expected semantics.

[dpaes]

Review — EVO-1836 (approved ✅)

Independent cross-repo verification (evo-ai-frontend-community #168 + this PR). Verdict: correct and complete, no blockers.

Both fixes verified.

1. findOne / findAll / findActive now include variables in the cache-hit reconstruction. Confirmed these are the only 3 hand-rebuild as Journey sites in the repo; the Temporal activities consume CachedJourney directly (not lossy). The Journey entity has no tenant/account column, so the reconstruction enumerates every persisted column → no data-loss risk when updateJourneyVariables saves the cache-hit object (same findOne → save → set pattern already used by update()/toggleActive() in production).
2. invalidate() → set() in updateJourneyVariables: correct, and it fixes a real latent bug beyond the display issue. invalidate did srem on the :index set; since getAll() only falls back to the DB when the whole index is empty, an edited journey was silently dropped from findActive() / the trigger processor until the 12h TTL expired. set() rewrites the body (with fresh variables) and re-adds the id to :index in one shot.

Non-blocking note. The headline invalidate → set switch (the AC4 "reopen → still there" mechanism) has no test — reverting that line would pass all current evo-flow specs. The findOne cache-hit fix is genuinely pinned (the added spec fails on revert; constructor arg order is correct). Given Sourcery-only CI, consider a regression test asserting set() (not invalidate) is called after save.

Approving + squash-merging.