Redis repository package#35
Conversation
Add @fedify/botkit-redis with a RedisRepository implementation for shared persistent BotKit storage across Deno and Node.js. The repository covers key pairs, messages, followers, follows, followees, quote authorizations, quote authorization references, poll votes, and bot-scoped views. Protect Redis read-modify-write paths with token-checked distributed locks, including lock renewal for long-running async message updates. Add Redis-backed integration and race regression tests, CI Redis service configuration, documentation, build wiring, and changelog notes. Fixes fedify-dev#12 fedify-dev#35 Assisted-by: Codex:gpt-5.5
|
@codex review |
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a new ChangesRedis Repository Package
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant RedisRepository
participant Redis
Caller->>RedisRepository: updateMessage(identifier, id, updater)
RedisRepository->>Redis: SET lock NX PX
Redis-->>RedisRepository: lock acquired
RedisRepository->>Redis: GET existing message JSON
RedisRepository->>RedisRepository: apply updater()
RedisRepository->>Redis: SET updated message JSON
RedisRepository->>Redis: EVAL conditional lock release
RedisRepository-->>Caller: result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the @fedify/botkit-redis package, which implements a Redis-backed repository (RedisRepository) for BotKit, complete with documentation, build configurations, and integration tests. Feedback on the implementation suggests addressing a potential unhandled promise rejection if the Redis client fails to connect on startup, ensuring close() handles connection failures gracefully without throwing, making the lock TTL and renewal intervals configurable to prevent spamming Redis, and optimizing sequential asynchronous operations in loops (during JWK exports and vote counting) by executing them concurrently using Promise.all.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report❌ Patch coverage is
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf6a470dc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/botkit-redis/src/mod.ts (2)
269-281: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLock acquisition polls indefinitely with no wait timeout.
withRedisLockspins onSET NX PX+delay(20ms)until the lock is free. A stale/held lock only clears via the 30s PX expiry, so under contention or a stuck holder this loops indefinitely with no overall deadline or backoff. Consider a bounded wait (max attempts/deadline) that throws a specific error, and/or exponential backoff to reduce Redis load.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/botkit-redis/src/mod.ts` around lines 269 - 281, The withRedisLock lock-acquisition loop in mod.ts can spin forever because it retries SET NX PX with a fixed delay and no overall deadline. Update the lock-wait logic around the token/while loop to use a bounded timeout or max-attempts limit and throw a specific error when the lock cannot be acquired in time; consider adding exponential backoff to the delay path to reduce Redis load under contention.
143-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the
as unknown ascast here. Use a minimalRedisClientLikeadapter for the methods this code actually needs instead of bypassing type checking with a double assertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/botkit-redis/src/mod.ts` around lines 143 - 151, The Redis client setup in the `else` branch is bypassing type safety with a double assertion on `createClient`; replace the `as unknown as RedisClientLike` cast in `mod.ts` with a minimal `RedisClientLike` adapter that exposes only the methods this class uses. Keep the `createClient` call and `client.connect()` flow in `this.client`, `this.ownsClient`, and `this.ready`, but adapt the returned client shape explicitly instead of asserting it.Source: Coding guidelines
packages/botkit-redis/src/mod.test.ts (3)
138-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated harness/cleanup boilerplate across several tests.
createHarness()covers the common case, but "does not close injected clients", "renews Redis locks during long message updates", "serializes concurrent quote authorization inserts", and "preserves quote authorization indexes during stale cleanup" all re-implement the same prefix/client-creation/try-finally-cleanup pattern by hand instead of extending the harness (e.g. accepting an optional pre-built client or a client-wrapping function).Extracting a shared helper (e.g.
createHarness({ wrapClient })returning{ prefix, repository, cleanup }and optionally exposing the underlying real client(s) for direct manipulation) would remove ~80 lines of near-identical duplication and reduce the chance of divergent cleanup bugs.Also applies to: 278-292, 383-405, 694-716, 762-787
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/botkit-redis/src/mod.test.ts` around lines 138 - 150, The test setup in createHarness and the related RedisRepository test cases is duplicating the same prefix/client setup and try/finally cleanup logic. Refactor the shared harness so it can accept an optional pre-built client or client wrapper (for example via a wrapClient option) while still returning prefix, repository, and cleanup, and then update the affected tests to build on that helper instead of re-creating the Redis client and cleanup flow manually. Use createHarness, RedisRepository, and cleanupPrefix as the main entry points when consolidating the duplicated test boilerplate.
213-234: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFragile positional patch of the lock
SETcommand.
createShortLockTtlClientrewrites the TTL by assuming a fixed argument layout (command[4] === "PX", then blindly overwritingcommand[5]). If the lock-acquisition command inmod.tsever reorders itsSEToptions (e.g. addsGETor changes flag order), this check silently fails to match, the TTL patch becomes a no-op, and the "renews Redis locks" test would then exercise the real default TTL instead of the intended short one — without failing loudly, just testing something other than what it claims.Match by searching for
"PX"anywhere in the array instead of a fixed index:🔧 More robust PX detection
sendCommand(args: readonly string[]): Promise<unknown> { const command = [...args]; - if ( - command[0] === "SET" && command[1] === lockKey && - command[4] === "PX" - ) { - command[5] = ttlMs.toString(); - } + if (command[0] === "SET" && command[1] === lockKey) { + const pxIndex = command.indexOf("PX"); + if (pxIndex !== -1) command[pxIndex + 1] = ttlMs.toString(); + } return client.sendCommand(command); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/botkit-redis/src/mod.test.ts` around lines 213 - 234, `createShortLockTtlClient` uses a fragile fixed-index check for the lock `SET` command, so the TTL rewrite can silently stop working if `mod.ts` changes option ordering. Update the `sendCommand` logic to detect the `"PX"` flag by searching the command arguments rather than assuming it is always at a specific position, and then overwrite the TTL value based on that match. Keep the behavior scoped to the lock-acquisition path identified by `lockKey` so the "renews Redis locks" test still forces a short TTL regardless of `SET` option order.
139-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a specific error type instead of generic
Error.As per coding guidelines,
**/*.{ts,tsx}: "Prefer specific error types over genericError; use built-in errors likeTypeError,RangeError, orSyntaxError". These three call sites throw a genericErrorfor a missing environment variable.🔧 Suggested fix
- if (redisUrl == null) throw new Error("REDIS_URL is not set."); + if (redisUrl == null) throw new TypeError("REDIS_URL is not set.");Also applies to: 279-279, 458-458
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/botkit-redis/src/mod.test.ts` at line 139, The test helper in mod.test.ts throws a generic Error when REDIS_URL is missing; replace that with a more specific built-in error type, such as TypeError, at each environment-variable check in the affected call sites. Update the throw statements in the redisUrl validation paths so the message stays the same but the error class matches the project guideline and is consistent across the three occurrences.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/botkit-redis/src/mod.ts`:
- Around line 126-137: Add the missing JSDoc `@throws` tags for every method that
can throw TypeError: the RedisRepository constructor, addFollower, and
addQuoteAuthorization in the Redis repository module. Update each method’s JSDoc
to explicitly document the TypeError condition, keeping the comments aligned
with the existing symbols so the throwing behavior is covered by the API docs.
---
Nitpick comments:
In `@packages/botkit-redis/src/mod.test.ts`:
- Around line 138-150: The test setup in createHarness and the related
RedisRepository test cases is duplicating the same prefix/client setup and
try/finally cleanup logic. Refactor the shared harness so it can accept an
optional pre-built client or client wrapper (for example via a wrapClient
option) while still returning prefix, repository, and cleanup, and then update
the affected tests to build on that helper instead of re-creating the Redis
client and cleanup flow manually. Use createHarness, RedisRepository, and
cleanupPrefix as the main entry points when consolidating the duplicated test
boilerplate.
- Around line 213-234: `createShortLockTtlClient` uses a fragile fixed-index
check for the lock `SET` command, so the TTL rewrite can silently stop working
if `mod.ts` changes option ordering. Update the `sendCommand` logic to detect
the `"PX"` flag by searching the command arguments rather than assuming it is
always at a specific position, and then overwrite the TTL value based on that
match. Keep the behavior scoped to the lock-acquisition path identified by
`lockKey` so the "renews Redis locks" test still forces a short TTL regardless
of `SET` option order.
- Line 139: The test helper in mod.test.ts throws a generic Error when REDIS_URL
is missing; replace that with a more specific built-in error type, such as
TypeError, at each environment-variable check in the affected call sites. Update
the throw statements in the redisUrl validation paths so the message stays the
same but the error class matches the project guideline and is consistent across
the three occurrences.
In `@packages/botkit-redis/src/mod.ts`:
- Around line 269-281: The withRedisLock lock-acquisition loop in mod.ts can
spin forever because it retries SET NX PX with a fixed delay and no overall
deadline. Update the lock-wait logic around the token/while loop to use a
bounded timeout or max-attempts limit and throw a specific error when the lock
cannot be acquired in time; consider adding exponential backoff to the delay
path to reduce Redis load under contention.
- Around line 143-151: The Redis client setup in the `else` branch is bypassing
type safety with a double assertion on `createClient`; replace the `as unknown
as RedisClientLike` cast in `mod.ts` with a minimal `RedisClientLike` adapter
that exposes only the methods this class uses. Keep the `createClient` call and
`client.connect()` flow in `this.client`, `this.ownsClient`, and `this.ready`,
but adapt the returned client shape explicitly instead of asserting it.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5391cf8-edac-43cb-9c6c-c51f83f60be1
⛔ Files ignored due to path filters (2)
deno.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/workflows/main.yamlCHANGES.mddocs/.vitepress/config.mtsdocs/concepts/repository.mddocs/package.jsonmise.tomlpackages/botkit-redis/README.mdpackages/botkit-redis/deno.jsonpackages/botkit-redis/package.jsonpackages/botkit-redis/src/mod.test.tspackages/botkit-redis/src/mod.tspackages/botkit-redis/tsdown.config.ts
Address review findings in the Redis repository by making connection startup failures handled, making lock timings configurable, and using a less aggressive default lock renewal interval. Also guard message removal with the same lock as message updates, handle zero-limit follower pagination, and parallelize independent Redis/JWK work. Document the new lock timing options and add the missing throws notes for public methods that validate caller input. The Redis test suite now covers the message delete/update race, zero-limit follower pagination, and lock timing option validation. fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces the new '@fedify/botkit-redis' package, which provides 'RedisRepository', a Redis-backed implementation of BotKit's 'Repository' interface. The implementation supports both URL-managed and caller-managed Redis clients, configurable key prefixes, bot-scoped storage, quote authorization storage, reverse lookups, and poll votes. It includes comprehensive unit tests and documentation updates. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2c25541bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Protect message creation and followee creation with the same Redis locks used by their removal paths. This keeps value writes and index writes atomic with respect to deletes, preventing phantom message and followee reverse-index entries under concurrent add/remove races. Add regression tests that delay the indexing write while a concurrent remove runs, then verify the record and index are both gone after the serialized operations complete. fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the @fedify/botkit-redis package, which provides RedisRepository, a Redis-backed implementation of BotKit's Repository interface supporting both Deno and Node.js. It includes comprehensive tests, documentation, and configuration updates. Feedback on the implementation highlights a potential resource leak in the close() method of RedisRepository when the initial connection attempt fails, as the early return prevents the owned client from being closed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f95cd7b501
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owned node-redis clients can emit error events during connection and reconnection failures. Attach an error listener before connecting so a repository-managed client does not terminate the Node.js process on a Redis outage. Also continue through close() after the initial ready promise rejects. That lets the repository attempt client cleanup and log cleanup failures instead of returning before the owned client has been given a chance to release sockets or timers. fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces the @fedify/botkit-redis package, which provides RedisRepository, a Redis-backed implementation of BotKit's Repository interface for Deno and Node.js. It supports key prefixing, bot-scoped storage, quote authorization, and poll votes. Feedback is provided regarding the lock renewal mechanism in RedisRepository, suggesting that the renewal interval should be cleared if the lock is lost (when the EVAL script returns 0) to avoid wasting Redis resources.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8eb27fb9ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Quote authorization reference updates need the same per-reference lock for both add and remove operations. Without that, a remove could delete a fresh reverse-index member written by a concurrent add, leaving the stored reference unreachable through the identifier index. The Redis lock renewal loop now also stops once the renewal script reports that the lock is no longer owned. That avoids continuing to issue renewal commands after another process has acquired or released the lock. fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces the new @fedify/botkit-redis package, which provides RedisRepository, a Redis-backed implementation of BotKit's Repository interface for Deno and Node.js. It supports key namespacing, shared persistent storage, and full repository features such as quote authorizations and poll votes. Feedback on the implementation highlights a potential issue where a failed initial connection permanently caches the connection error on the this.ready promise, preventing subsequent commands from retrying the connection.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4722f4591e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owned Redis clients no longer keep a rejected initial connection promise forever. When the eager connection fails, the cached readiness promise is cleared so the next repository command can start a fresh connect attempt. Quote authorization removal now uses the same interacting-object index lock as insertion before deleting the stored authorization or reverse index. A small reverse lookup from authorization ID to interacting object lets removal acquire that lock before inspecting the stored authorization, which prevents a replacement insertion from observing stale state and being dropped. fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
Injected Redis clients now make close() a complete no-op, including avoiding any cached readiness promise. Corrupted stored message JSON during update is handled as a failed update instead of invoking the updater or leaking a parse exception. Follower removal is retryable after a partial delete of the follow request marker: a retry can still remove the follow ID from the follower request set and run follower cleanup. Quote authorization reference attribution lookup also validates parsed JSON before reading object properties. fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the new @fedify/botkit-redis package, which provides RedisRepository, a Redis-backed implementation of BotKit's Repository interface for Deno and Node.js. It includes comprehensive support for managing key pairs, messages, followers, follows, quote authorizations, and poll votes directly in Redis. The feedback suggests optimizing several database operations by combining sequential DEL commands into single calls to reduce Redis roundtrips and improve overall efficiency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7efa3dced1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The owned Redis client is now assigned through the repository's compatible client interface directly, so TypeScript verifies the structural contract without erasing the node-redis type through an unknown double assertion. Stale quote authorization cleanup now deletes the stale forward and reverse index keys with a single Redis DEL command. This keeps the locked cleanup path atomic in behavior while avoiding an unnecessary roundtrip. fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces the new @fedify/botkit-redis package, providing RedisRepository as a Redis-backed storage implementation of BotKit's Repository interface, along with corresponding documentation, tests, and dependency updates. The review feedback highlights two performance bottlenecks in getMessages and getFollowers where sequential Redis lookups in loops can be optimized using MGET. Additionally, a potential runtime issue was identified in findQuoteAuthorizationReference when parsing JSON values that could be plain strings or null, which should be handled more safely.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Message and follower listing now fetches the selected Redis payloads with one MGET command instead of issuing one GET per listed identifier. That keeps the sorted-set range lookup behavior unchanged while avoiding sequential network roundtrips on paginated list reads. The Redis repository tests now record commands during those list reads and assert that the batched MGET path is used without falling back to per-item GET commands. fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
The follower list test now checks that the Redis MGET command receives exactly the follower payload keys. This keeps the regression coverage tied to the batched lookup behavior requested in review, not only to the presence of any MGET command. fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the @fedify/botkit-redis package, which implements a Redis-backed repository (RedisRepository) for BotKit. The feedback focuses on improving robustness and performance: adding defensive checks for null/undefined options and parsed JSON values, implementing a closed lifecycle flag to prevent operations after the repository is closed, and chunking MGET operations into parallel batches to avoid performance bottlenecks during message and follower retrieval.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The Redis repository now rejects missing constructor options and marks itself closed before returning from close(), including when the Redis client is injected and must not be closed by the repository. Commands check that state before touching Redis, which prevents accidental reconnects or command execution after disposal. List payload reads now use a shared batched MGET helper. The helper splits large key sets into bounded chunks and resolves those chunks in parallel, so message and follower pagination avoid both sequential GET loops and oversized single commands. Quote authorization reference lookups also validate parsed JSON before reading messageId from it. fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/botkit-redis/src/mod.test.ts (1)
483-490: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: rename the
.every()param for clarity.In
mgets.every((command) => command.length <= 101),commandis actually a full MGET argument array (["MGET", ...keys]), which reads confusingly next to thefilter(([command]) => ...)above wherecommandis the command name. Renaming tomgetmakes the batch-size intent clearer.♻️ Suggested rename
- assert.ok(mgets.every((command) => command.length <= 101)); + assert.ok(mgets.every((mget) => mget.length <= 101));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/botkit-redis/src/mod.test.ts` around lines 483 - 490, The `mgets.every(...)` callback in `mod.test.ts` uses `command` for the full MGET argument array, which is confusing next to the earlier `filter(([command]) => ...)` usage where `command` means the command name. Rename the `.every()` callback parameter in the `mgets` assertion to something like `mget` so the intent of checking each MGET batch’s size is clear and the test reads consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/botkit-redis/src/mod.test.ts`:
- Around line 483-490: The `mgets.every(...)` callback in `mod.test.ts` uses
`command` for the full MGET argument array, which is confusing next to the
earlier `filter(([command]) => ...)` usage where `command` means the command
name. Rename the `.every()` callback parameter in the `mgets` assertion to
something like `mget` so the intent of checking each MGET batch’s size is clear
and the test reads consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5759b3d-cbb2-4c54-b068-681618e11d03
📒 Files selected for processing (2)
packages/botkit-redis/src/mod.test.tspackages/botkit-redis/src/mod.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/botkit-redis/src/mod.ts
There was a problem hiding this comment.
Code Review
This pull request introduces the new @fedify/botkit-redis package, which implements a Redis-backed repository (RedisRepository) for BotKit, complete with unit tests, documentation, and build configurations. The review feedback highlights two key improvements: preventing background interval leaks in the Redis lock renewal mechanism when the repository is closed, and adding missing external dependencies (@fedify/fedify and @logtape/logtape) to the JSR import map in deno.json to ensure proper resolution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Rename the MGET batch assertion callback variable so it does not reuse command in a context where the surrounding test uses that name for the Redis command verb. fedify-dev#35 (review) Assisted-by: Codex:gpt-5.5
Redis lock renewal now stops as soon as the repository is closed. This prevents background renewal callbacks from issuing commands against a closed repository and repeatedly logging expected closure errors. The regression test closes a repository while an update still holds a lock, then verifies that no renewal warning is emitted after close. fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
Declare the Redis package's direct Fedify and LogTape dependencies in its own Deno import map. This keeps the package self-contained for JSR resolution instead of relying on the workspace root import map. The lockfile now records those package-level dependencies as well. fedify-dev#35 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces the new '@fedify/botkit-redis' package, which implements a Redis-backed 'RedisRepository' for BotKit. The repository supports storing key pairs, messages, followers, follows, followees, quote authorizations, and poll votes directly in Redis data structures, and features cross-runtime compatibility (Deno and Node.js), configurable key prefixes, and a robust locking mechanism to serialize concurrent updates. The PR also includes comprehensive integration tests, documentation updates, and build configurations. As there are no review comments provided, I have no additional feedback to offer.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What changed
This PR adds @fedify/botkit-redis, a Redis-backed
Repositoryimplementation for BotKit. It also wires the package into CI, Node builds, documentation, and the unreleased changelog.Closes #12.
How it works
The new package lives under packages/botkit-redis/ and exports
RedisRepositoryfrom packages/botkit-redis/src/mod.ts. The repository stores BotKit data directly in Redis using strings, sets, and sorted sets rather than going through Fedify's generic KV abstraction. It supports both URL-managed clients and caller-managed node-redis compatible clients: URL-created repositories own their Redis connection and close it throughclose()orSymbol.asyncDispose, while injected-client repositories leave lifecycle control to the caller.Data is namespaced under a configurable Redis key prefix, with bot-specific data scoped under each bot identifier. Messages are stored by UUID and indexed in a sorted set for outbox ordering. Followers use request and follower indexes so multiple follow requests for the same actor can coexist and so replacement and removal flows can clean up the correct rows. Followees and quote authorization references maintain reverse indexes that allow BotKit to route incoming activities back to the bot identifiers that own them. Poll votes are represented as voter and option sets, which gives idempotent vote recording and straightforward counts.
The implementation includes Redis-backed support for key pairs, messages, followers, sent follows, followees, quote authorizations, quote authorization references, poll votes, and
forIdentifier()scoped views. Tests in packages/botkit-redis/src/mod.test.ts exercise the repository through the same public behavior expected by the core repository contract, including persistence across repository instances and bot isolation.Why it works this way
Redis deployments are commonly shared across multiple Node.js or Deno worker processes, so the repository cannot rely on in-process synchronization. The implementation protects read-modify-write paths with Redis locks built from
SET NX PX, token-checked Lua release, and token-checked TTL renewal. This is used for message updates, follower request/index mutations, and quote authorization index mutations because those paths can otherwise lose updates or leave reverse indexes inconsistent when multiple workers handle related ActivityPub activities concurrently.The lock renewal exists because BotKit's message update API accepts async updaters. A valid updater can take longer than a fixed lock TTL, so the lock is renewed while the operation is still active rather than allowing another worker to acquire the same key and update stale data. The renewal and release scripts both check the lock token before touching the key, so an expired or replaced lock is not accidentally extended or deleted by an older holder.
Quote authorizations deliberately keep only the first stored authorization for a given interacting object. That duplicate-suppression behavior depends on the interacting-object reverse index, so both insertion and stale-index cleanup run under the same per-index lock. This prevents a stale cleanup from deleting a fresh index written by another worker and prevents concurrent inserts from leaving multiple authorizations addressable by ID.
The package follows the existing storage-package shape used by @fedify/botkit-sqlite and @fedify/botkit-postgres: it has its own deno.json, package.json, tsdown.config.ts, README, tests, and a
build:botkit-redistask in mise.toml. The GitHub Actions workflow in .github/workflows/main.yaml starts a Redis service and exposesREDIS_URLso Redis integration tests run in CI alongside the existing PostgreSQL tests.Documentation and release notes
The user-facing repository documentation in docs/concepts/repository.md now includes a
RedisRepositorysection with installation instructions and constructor options. The package README in packages/botkit-redis/README.md includes a workingcreateBot()example with the requiredkvoption, plus an injected-client example for applications that already manage a node-redis client.The VitePress reference list in docs/.vitepress/config.mts includes @fedify/botkit-redis, and docs/package.json includes the workspace package and
redisdependency needed for documentation type checking. The unreleased section of CHANGES.md describes the new package and references issue #12 and PR #35.Verification
I verified the Redis package against a local Redis server at
redis://localhost:6379/0:REDIS_URL=redis://localhost:6379/0 deno test --allow-env --allow-net=localhost,127.0.0.1 packages/botkit-redis/src/mod.test.tsREDIS_URL=redis://localhost:6379/0 pnpm --filter @fedify/botkit-redis run testmise run checkgit diff --checkThe commit hook also ran
mise run checksuccessfully when creating the commit.Summary by CodeRabbit