Stubbed AdapterCacheRedis.keys() with an IncorrectUsageError

[allouis]

Why

AdapterCacheRedis.keys() is async — it has to be, since it's an O(n) SCAN against redis — but its only consumer, settings-cache.getAll(), calls it without await and treats the result as a sync array. So if anything ever wired Redis to the settings cache, every GET /settings and PUT /settings would crash with keys.forEach is not a function. Nothing does today, so the path is unreachable, but it's a latent footgun.

The method is also documented as transitional in @tryghost/adapter-base-cache:

NOTE: "keys" method is only here to provide smooth migration from deprecated "getAll" method
once use of "getAll" is eradicated, can also remove the "keys" method form the interface

We can't just delete keys() though. BaseCacheAdapter.requiredFns (defined in the external package) lists it, and adapter-manager validates every loaded adapter against that list. Removing the method would crash Ghost at boot.

So this PR swaps the public keys() for a stub that throws IncorrectUsageError. The stub is intentionally sync, not async, so the throw surfaces directly to the same sync caller pattern that exposed the original bug, rather than becoming an unhandled rejection.

Cleanup path (follow-up, not in this PR)

1. Update settings-cache.getAll() so it no longer depends on cacheStore.keys().
2. Bump @tryghost/adapter-base-cache to drop "keys" from requiredFns, then delete the stub.
3. Once Used a key prefix to enable immediate cache resets #19518's prefix-rotation reset() lands, the SCAN helpers (#getKeys, #scanNodeForKeys, #getPrimaryRedisNode) become dead and can go too.

Notes for reviewers

• Adapter-manager validation still passes at runtime (the method exists), and a direct call to cache.keys() throws IncorrectUsageError synchronously.
• Existing 11 AdapterCacheRedis unit tests still pass; an additional unit test asserts the throw.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1af3e631-89bc-404e-bc2d-8f3e8c1dd56e

📥 Commits

Reviewing files that changed from the base of the PR and between ca1c409 and 23b605a.

📒 Files selected for processing (2)

• ghost/core/core/server/adapters/lib/redis/AdapterCacheRedis.js
• ghost/core/test/unit/server/adapters/lib/redis/adapter-cache-redis.test.js

---

Walkthrough

The AdapterCacheRedis.keys() method implementation was changed to immediately throw an IncorrectUsageError instead of attempting to enumerate and return Redis keys. The @tryghost/errors dependency was added to support this error type. JSDoc was updated to mark the method as deprecated and clarify that key enumeration is unsupported. The internal #getKeys() method remains available for internal operations like reset. Unit tests were added to verify that calling keys() throws the expected error type.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: stubbing the keys() method with an IncorrectUsageError.
Description check	✅ Passed	The description thoroughly explains why the change is needed (latent bug with async/sync mismatch), the cleanup path, and notes for reviewers, all directly relevant to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch redis-cache-no-keys

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

ghost/core/core/server/adapters/lib/redis/AdapterCacheRedis.js (1)

274-278: Docstring overstates _keys() role in reset flow

The comment says _keys() is preserved for reset flow, but reset() currently uses #getKeys() directly (Line 257). Tightening this wording will avoid confusion during later cleanup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ghost/core/core/server/adapters/lib/redis/AdapterCacheRedis.js` around lines
274 - 278, The docstring for _keys() is misleading because reset() calls
getKeys() directly rather than relying on _keys(); update the comment above
AdapterCacheRedis._keys() to remove the claim that it’s preserved for the
SCAN-based reset() flow and instead state that _keys() is deprecated/kept only
for backward compatibility (or legacy SCAN implementations) while reset()
currently uses getKeys(); reference the symbols _keys(), getKeys(), and reset()
in the updated wording so future readers understand the actual call-path.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghost/core/core/server/adapters/lib/redis/AdapterCacheRedis.js`:
- Around line 280-284: The keys() method in AdapterCacheRedis is declared async
so throwing there produces a rejected Promise instead of a synchronous throw;
change the method signature to a synchronous function (remove the async keyword
from AdapterCacheRedis.prototype.keys / keys()) so it throws IncorrectUsageError
immediately for sync callers (e.g., cache-manager.js using const keys =
this.settingsCache.keys()), ensuring the intended error is raised instead of a
TypeError from calling forEach on a Promise.

---

Nitpick comments:
In `@ghost/core/core/server/adapters/lib/redis/AdapterCacheRedis.js`:
- Around line 274-278: The docstring for _keys() is misleading because reset()
calls getKeys() directly rather than relying on _keys(); update the comment
above AdapterCacheRedis._keys() to remove the claim that it’s preserved for the
SCAN-based reset() flow and instead state that _keys() is deprecated/kept only
for backward compatibility (or legacy SCAN implementations) while reset()
currently uses getKeys(); reference the symbols _keys(), getKeys(), and reset()
in the updated wording so future readers understand the actual call-path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 83ba2b2f-63df-4137-8e24-7215d8089e8f

📥 Commits

Reviewing files that changed from the base of the PR and between 511a438 and ca1c409.

📒 Files selected for processing (1)

• ghost/core/core/server/adapters/lib/redis/AdapterCacheRedis.js

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud