refactor(redis): move distributed runtime behind explicit extension - #3216
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a9cb820c8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 281f7bd617
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81f12cf11d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6da9dbefc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09d3fa0a15
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51b4ced1f9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
48b803d to
b38ed5b
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors Redis support so core remains provider-neutral while Redis runtime implementations, third-party packages (redis, @redis/client), connection lifecycle, and Pub/Sub live behind an explicitly activated @veryfront/ext-redis extension. It introduces new Redis runtime contracts (RedisRuntimeProvider), adds lifecycle-owned connection helpers (shared + owned clients), and updates core features (render cache store, rate limiting, Claude Code event publishing, proxy routing invalidation, and platform Redis adapter module loading) to resolve Redis functionality through the active provider.
Changes:
- Introduces provider-neutral Redis runtime contracts (
RedisRuntimeProvider, client/handle types, module adapter capture) and core helpers (ensureRedisRuntimeProvider,OwnedRedisClientConnection, shared client facade updates). - Adds a new explicit extension package
extensions/ext-redisthat owns Redis dependencies, runtime clients, Pub/Sub publisher implementation, and lifecycle-safe teardown behavior. - Strengthens build/lint/audit tooling to enforce dependency and capability boundaries (SBOM ratchets, extension capability audits, AST-based core dependency audit improvements), and updates public exports/import-map entries accordingly.
Verification (recommended):
- Not run in this review environment.
- Suggested commands:
deno task verify:quickdeno test --no-check --allow-all --paralleldeno task test -- tests/docs/(docs were updated)
Reviewed changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/workflow/claude-code/event-publisher.ts | Routes Redis event publishing through the active Redis runtime provider contract. |
| src/workflow/claude-code/event-publisher.test.ts | Updates Claude Code event publisher tests to use contract registration and provider stubs. |
| src/utils/redis-client.ts | Refactors shared Redis client facade to resolve/track provider owners and disconnect across provider generations. |
| src/utils/redis-client.test.ts | Adds tests for shared client facade owner tracking, in-flight acquisition handling, and retryable disconnect. |
| src/rendering/cache/stores/redis-store.ts | Switches render cache Redis store to an extension-owned connection handle and object-shaped SCAN results. |
| src/rendering/cache/stores/redis-store.test.ts | Adds tests validating owned connection close behavior and SCAN shape handling. |
| src/proxy/routing-invalidation-redis.ts | Aligns routing invalidation Redis client typing with provider-neutral Pub/Sub surfaces. |
| src/platform/adapters/redis/types.ts | Re-exports Redis module/client structural types from the provider-neutral contract. |
| src/platform/adapters/redis/node.ts | Adjusts Node Redis adapter disconnect semantics to match destroy(): void contract. |
| src/platform/adapters/redis/modules.ts | Replaces direct Redis imports with provider-owned module loading + generation-safe caching and invalidation. |
| src/platform/adapters/redis/modules.test.ts | Expands module-loading tests for provider replacement, in-flight invalidation, and post-close behavior. |
| src/middleware/builtin/security/redis-rate-limit.ts | Refactors Redis rate limit store to use an owned Redis connection handle. |
| src/middleware/builtin/security/redis-rate-limit.test.ts | Updates tests to validate owned-connection cleanup and client reuse behavior. |
| src/extensions/recommendations.ts | Adds an extension recommendation mapping for RedisRuntimeProvider. |
| src/extensions/index.ts | Exposes provider-neutral Redis distributed runtime contracts via the public extensions surface. |
| src/extensions/distributed/index.ts | Adds the distributed runtime barrel export for Redis runtime contracts. |
| src/extensions/distributed/defaults.ts | Adds provider resolution helpers for explicit orchestration and active-provider lookup. |
| src/extensions/distributed/redis-runtime-provider.ts | Introduces the provider-neutral Redis runtime contract and safe capture/snapshot wrappers. |
| src/extensions/distributed/redis-runtime-provider.test.ts | Adds tests for provider capture correctness, accessor rejection, and fail-closed behavior. |
| src/extensions/distributed/owned-redis-client.ts | Adds lifecycle-owned Redis connection helper with single-flight open/close and abort-aware cleanup. |
| src/extensions/distributed/owned-redis-client.test.ts | Adds tests for connection cancellation, idempotent close, and listener isolation across replacements. |
| scripts/lint/extension-source-metadata.ts | Extends known contract constant auditing to include RedisRuntimeProviderName. |
| scripts/lint/audit-extension-contracts.test.ts | Updates contract audit tests to include Redis runtime provider contracts. |
| scripts/lint/audit-extension-capabilities.ts | Adds sensitive capability policy for the Redis runtime extension. |
| scripts/lint/audit-extension-capabilities.test.ts | Adds tests ensuring ext-redis capability requirements are enforced. |
| scripts/lint/audit-dependency-boundaries.test.ts | Adds ext-redis dependency boundary expectations for redis and @redis/client. |
| scripts/lint/audit-core-deps.ts | Replaces regex-based import detection with AST-based analysis for static/dynamic third-party imports. |
| scripts/lint/audit-core-deps.test.ts | Expands tests for new AST-based third-party import detection behaviors. |
| scripts/build/npm-package-metadata.ts | Moves Redis packages into extension-owned dependencies and removes root optional peer for redis. |
| scripts/build/npm-package-metadata.test.ts | Updates root package metadata expectations after removing the redis peer. |
| scripts/build/npm-extension-package-metadata.test.ts | Adds test ensuring ext-redis is externalized through dependency-free leaf exports. |
| scripts/build/generate-sbom.ts | Adds SBOM sensitive dependency boundary for ext-redis packages. |
| scripts/build/generate-sbom.test.ts | Adds SBOM ratchet test coverage for the ext-redis boundary. |
| scripts/build/compile-binary.ts | Ensures compiled binary includes ext-redis source for opt-in activation. |
| scripts/build/compile-binary.test.ts | Adds test validating compiled CLI embeds ext-redis. |
| extensions/README.md | Documents ext-redis in the extensions catalog and capability policy tables. |
| extensions/ext-redis/deno.json | Defines the ext-redis explicit-activation manifest, contracts, capabilities, and import mappings. |
| extensions/ext-redis/README.md | Adds user-facing documentation for installing and explicitly enabling ext-redis. |
| extensions/ext-redis/src/index.ts | Adds ext-redis factory that registers the RedisRuntimeProvider and declares required capabilities. |
| extensions/ext-redis/src/index.test.ts | Tests extension factory contract/capability declaration and teardown/setup behavior. |
| extensions/ext-redis/src/connection-config.ts | Adds Redis URL validation helper for safe, canonical connection config. |
| extensions/ext-redis/src/connection-config.test.ts | Tests Redis URL validation behavior and rejection cases. |
| extensions/ext-redis/src/redis-client-manager.ts | Adds extension-owned Redis shared-client manager with option capture, retries, and safe cleanup. |
| extensions/ext-redis/src/redis-client-manager.test.ts | Tests option capture, single-flight acquisition, cancellation, retries, and cleanup behavior. |
| extensions/ext-redis/src/event-publisher.ts | Adds extension-owned Redis Pub/Sub event publisher with safe init/close and listener lifecycle. |
| extensions/ext-redis/src/event-publisher.test.ts | Tests Pub/Sub publisher initialization single-flight, listener isolation, and teardown failure modes. |
| extensions/ext-redis/src/redis-runtime-provider.ts | Implements RedisRuntimeProvider using redis + @redis/client, including module adapter and owned clients. |
| extensions/ext-redis/src/redis-runtime-provider.test.ts | Tests provider lifecycle behaviors: module invalidation, owned clients, connect/close edge cases, retries. |
| docs/api-reference/veryfront/workflow.md | Updates generated API reference links/locations affected by refactors and type moves. |
| docs/api-reference/veryfront/middleware.md | Updates generated API reference links/locations for Redis rate limit changes. |
| docs/api-reference/veryfront/extensions.md | Updates generated API reference exports for new distributed runtime contracts. |
| docs/api-reference/veryfront/errors.md | Updates generated API reference deep-import sections for error registry barrels. |
| deno.lock | Records ext-redis npm dependency entries for redis and @redis/client. |
| deno.json | Adds ext-redis workspace member and new public import-map entries for distributed contracts and deep imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 55 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/workflow/claude-code/event-publisher.ts:136
RedisEventPublisher.close()only clearsthis.implementationafter a successfulimplementation.close(). Ifclose()rejects (for example, one underlying client fails to close), the publisher keeps the previous implementation and laterpublish()/subscribe()calls can reuse a partially closed or failed implementation instead of creating a fresh one.
7384369 to
f0dde4d
Compare
f0dde4d to
d64a557
Compare
d64a557 to
5e9ca2f
Compare
a5fd100 to
78b42b6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 56 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/extensions/distributed/redis-runtime-provider.ts:414
- The captured
ttlwrapper usesPromise.resolve(Reflect.apply(...)), which still throws synchronously if the underlyingttlthrows. Since callers treatttlas returning a Promise, wrap the call inPromise.resolve().then(...)so sync throws become rejections, matching the other async method wrappers.
extensions/ext-redis/src/redis-runtime-provider.ts:217 closeClient(force)resetsclosed = false, which makes the forced cleanup path non-idempotent. If a provisional handle was already closed successfully, a later forced cleanup (for example from an observed cleanup callback) will attemptclient.disconnect()again and can throw or cause extra network work.forceshould only ensure the handle remains tracked for teardown and retry after failures, not reopen a closed handle.
if (force) {
closed = false;
clientHandles.add(handle);
}
if (closed) {
src/extensions/distributed/redis-runtime-provider.ts:288
captureAsyncMethodsbuilds wrappers withPromise.resolve(Reflect.apply(...)). If the underlying method throws synchronously, the wrapper throws before returning a Promise, breaking the declaredPromise<unknown>contract. Wrap the call inPromise.resolve().then(...)so sync throws become promise rejections consistently.
This issue also appears on line 411 of the same file.
Summary
redisand@redis/clientruntime implementations into the explicit@veryfront/ext-redisboundaryRedisRuntimeProvidercontracts and lifecycle-owned clients for cache, rate limiting, platform adapters, shared clients, proxy-routing Pub/Sub, and Claude Code Pub/SubCompatibility
@veryfront/ext-redisSafety
Validation
deno task verify:quickdeno task build:npmextensions/ext-redis: 5 test files / 28 stepsext-rediscomplete source and test type-checkgit diff --checkStack
mainafter refactor(core): extract runtime implementations behind extensions #3212 merges.