Conversation
WalkthroughRemoved the Redis service and its volume from docker-compose.yml and dropped REDIS_URI and the redis dependency from the apostrophe service. Introduced a new createAposConfig() export in website/app.js that builds the Apostrophe configuration and conditionally wires a Redis-backed session store only when REDIS_URI is present; otherwise the session store is left undefined. Added a test to assert behavior when REDIS_URI is not set and retained tests for the Redis-backed case. Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Vulnerabilities of
|
| digest | sha256:3f69176bae2775f57ff828cf1482b4e131c4f648e531af9b102e6dd29794b011 |
| vulnerabilities | |
| platform | linux/amd64 |
| size | 173 MB |
| packages | 971 |
📦 Base Image node:24-alpine
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/utils/env.js`:
- Around line 10-20: The helper getOptionalEnv weakens the repo's fail-fast env
policy by allowing callers to bypass validation; remove getOptionalEnv from the
public exports in website/utils/env.js and keep any optional-REDIS_URI handling
local to website/app.js (move the optional lookup/ default logic into
website/app.js and continue to use getEnv for all other variables), or if you
prefer a shared helper implement an explicit allowlist in the utils module
(e.g., a restricted isOptional(name) check) and only permit listed names like
REDIS_URI to be fetched optionally; update callers accordingly so getEnv remains
the default for required variables and only the controlled exception exists in
website/app.js.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 524a9dbd-ae47-4101-9d6f-73edda0cfe6f
📒 Files selected for processing (5)
docker-compose.ymlwebsite/app.jswebsite/app.test.jswebsite/utils/env.jswebsite/utils/env.test.js
💤 Files with no reviewable changes (1)
- docker-compose.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/app.js`:
- Around line 6-19: The code silently falls back to MemoryStore when redisUri
(process.env.REDIS_URI) is missing; update the initialization around redisUri
and sessionConfig so that if NODE_ENV === 'production' and redisUri is falsy you
emit a clear warning (e.g., via console.warn or the app logger) stating that
Redis is not configured and MemoryStore will be used (memory leaks, lost
sessions, no clustering), while leaving the existing Redis-backed sessionConfig
logic (sessionConfig.store) unchanged when redisUri is present; reference
redisUri and sessionConfig to locate where to add the check and warning.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/app.js`:
- Around line 13-26: The test and comment are wrong because when REDIS_URI is
absent Apostrophe defaults to a MongoDB-backed session store; fix by either
making the app explicitly use an in-memory store or by updating the test to
expect MongoDB. To explicitly use an in-memory fallback, in the sessionConfig
block check when redisUri is falsy and set sessionConfig.store to an in-memory
implementation (e.g., require('connect-memorystore')(session) or another memory
store) with appropriate options; alternatively update the test name and
assertions to reflect that omitting sessionConfig.store results in the default
MongoDB-backed store. Reference the symbols redisUri, sessionConfig, and
sessionConfig.store (and the connect-redis usage) when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 205bdaee-fad5-43c5-8cb0-8b034f78a764
📒 Files selected for processing (2)
website/app.jswebsite/app.test.js
| const redisUri = process.env.REDIS_URI; | ||
|
|
||
| const sessionConfig = { | ||
| secret: getEnv('SESSION_SECRET'), | ||
| }; | ||
|
|
||
| if (redisUri) { | ||
| sessionConfig.store = { | ||
| connect: require('connect-redis'), | ||
| options: { | ||
| url: redisUri, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the current state of the code files mentioned
head -30 website/app.jsRepository: speedandfunction/website
Length of output: 882
🏁 Script executed:
# Check the test file to see what it expects
sed -n '60,75p' website/app.test.jsRepository: speedandfunction/website
Length of output: 485
🏁 Script executed:
# Find package.json to check `@apostrophecms/express` version
find . -name "package.json" -type f | head -5Repository: speedandfunction/website
Length of output: 117
🏁 Script executed:
# Search for `@apostrophecms/express` in dependencies
rg "@apostrophecms/express" --type jsonRepository: speedandfunction/website
Length of output: 50
🏁 Script executed:
cat website/package.json | grep -A 5 "@apostrophecms"Repository: speedandfunction/website
Length of output: 732
🏁 Script executed:
# Search for any documentation or comments about session behavior in the codebase
rg "session" -t js -i | head -20Repository: speedandfunction/website
Length of output: 1211
🏁 Script executed:
# Look for the full test context to understand expectations
sed -n '50,80p' website/app.test.jsRepository: speedandfunction/website
Length of output: 926
🏁 Script executed:
# Search for any configuration or setup that might define session defaults
rg "MemoryStore|sessionConfig" -t js -B 2 -A 2Repository: speedandfunction/website
Length of output: 639
🏁 Script executed:
# Check if there's an express module configuration
cat website/modules/@apostrophecms/express/index.jsRepository: speedandfunction/website
Length of output: 280
🏁 Script executed:
# Look for any documentation files or comments about session handling
find . -type f \( -name "*.md" -o -name "*.txt" \) | xargs rg -l "session" 2>/dev/null | head -5Repository: speedandfunction/website
Length of output: 50
🏁 Script executed:
# Check if there are any other session-related configurations
rg "store.*:" website/app.js -B 3 -A 3Repository: speedandfunction/website
Length of output: 50
🏁 Script executed:
# Search for any comments or documentation about the default session behavior
rg "MongoDB|MemoryStore|session.*default" -iRepository: speedandfunction/website
Length of output: 5942
🏁 Script executed:
# Check if there's any initialization code that sets up session handling
rg "express.*session|connect.*session" -i -t jsRepository: speedandfunction/website
Length of output: 338
🏁 Script executed:
# Look at the full createAposConfig function to see where sessionConfig is used
sed -n '13,100p' website/app.js | cat -nRepository: speedandfunction/website
Length of output: 2930
🏁 Script executed:
# Search for any references to how `@apostrophecms/express` handles session
rg "@apostrophecms/express" -A 5 -B 5 website/app.jsRepository: speedandfunction/website
Length of output: 348
🌐 Web query:
@apostrophecms/express session store default MongoDB
💡 Result:
@apostrophecms/express uses a MongoDB-backed session store by default—you don’t need to install/configure an additional store just to persist sessions (it stores them in MongoDB, reducing reliance on the in-memory default of express-session). It also notes this uses a “3.x mongo store” and its default session options are tuned accordingly (e.g., saveUninitialized: false, resave: false). Sessions are stored in the aposSessions MongoDB collection. [1]
If you want something else (e.g., Redis), you can configure session.store and provide the store package name + options (and install that package), e.g. connect-redis. [1]
Source
- ApostropheCMS docs:
@apostrophecms/expressmodule reference (session defaults + MongoDB session store). [1]
🌐 Web query:
apostrophecms session configuration store undefined default behavior
💡 Result:
If you leave @apostrophecms/express session store unset/undefined, Apostrophe’s default behavior is to store sessions in MongoDB (so you don’t need Redis or another external session store just to get sessions working). [1], [2]
In Apostrophe 3, the session option object is passed through to express-session, with Apostrophe applying defaults like saveUninitialized: false, resave: false, rolling: true, etc.; you only need to provide session.store if you want to override the default MongoDB-backed store (e.g., Redis). [1]
Sources:
[1] Apostrophe 3 docs — @apostrophecms/express module (session option and default MongoDB sessions)
[2] Apostrophe 2 docs — “Storing sessions in Redis and other session stores” (default MongoDB sessions)
🏁 Script executed:
# Check if there's any documentation in the node_modules or comments
rg "session.*store.*MongoDB|default.*session" -i -B 2 -A 2Repository: speedandfunction/website
Length of output: 50
🏁 Script executed:
# Let's verify the exact test expectation and name one more time
sed -n '64,72p' website/app.test.jsRepository: speedandfunction/website
Length of output: 326
🏁 Script executed:
# Check if there are any comments in the codebase about the session store behavior
rg "session.*store" -B 3 -A 3 website/app.js | head -30Repository: speedandfunction/website
Length of output: 477
🏁 Script executed:
# Check if there's an explicit MemoryStore or any other session store setup
rg "MemoryStore|connect-mongo|session.*default" -t jsRepository: speedandfunction/website
Length of output: 50
🏁 Script executed:
# Look at the apostrophe version to confirm it's version 4.x (modern)
grep '"apostrophe"' website/package.jsonRepository: speedandfunction/website
Length of output: 96
The test name and code comment are misleading—the fallback is MongoDB-backed, not in-memory.
When session.store is omitted, @apostrophecms/express does not implement an in-memory store. According to Apostrophe's official documentation, it defaults to a MongoDB-backed session store. The test at lines 64–72 is titled "uses in-memory session store when REDIS_URI is not set" but will actually use MongoDB by default, making the test name and assertion misleading. Either configure an explicit in-memory session store (e.g., connect-memorystore) if that is the intended fallback, or update the test name and PR description to reflect the actual MongoDB default behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/app.js` around lines 13 - 26, The test and comment are wrong because
when REDIS_URI is absent Apostrophe defaults to a MongoDB-backed session store;
fix by either making the app explicitly use an in-memory store or by updating
the test to expect MongoDB. To explicitly use an in-memory fallback, in the
sessionConfig block check when redisUri is falsy and set sessionConfig.store to
an in-memory implementation (e.g., require('connect-memorystore')(session) or
another memory store) with appropriate options; alternatively update the test
name and assertions to reflect that omitting sessionConfig.store results in the
default MongoDB-backed store. Reference the symbols redisUri, sessionConfig, and
sessionConfig.store (and the connect-redis usage) when making the change.
feat: make Redis session store optional
This PR makes the Redis session store optional by checking REDIS_URI: if unset, sessions fall back to in-memory storage. It introduces createAposConfig() to centralize configuration, adds getOptionalEnv helper and tests verifying both Redis and non-Redis behaviors, and simplifies docker-compose by removing the Redis service and related environment variables/volumes.