initial commit of cagematch - #311
Open
jasonmorais wants to merge 6 commits into
Open
Conversation
Contributor
Reviewer's GuideIntroduce a backend-neutral feature rate limiting core (Mongo + Redis contenders), wire it into the OCOM API/community creation flow, and extend local dev + E2E tooling to run a Redis memory server with deterministic worktree ports and REDIS_URL conversion. Sequence diagram for community.create rate limiting flowsequenceDiagram
actor User
participant ApiBootstrap as ApiBootstrap
participant ApplicationServicesFactory as buildApplicationServicesFactory
participant CommunityService as Community.create
participant RateLimiter as ServiceRateLimiting
User->>ApiBootstrap: HTTP request (community.create)
ApiBootstrap->>ApiBootstrap: startUp ServiceRedisRateLimiting
ApiBootstrap->>ApplicationServicesFactory: buildApplicationServicesFactory(context)
ApplicationServicesFactory->>ApplicationServicesFactory: derive RateLimitSubject
ApplicationServicesFactory->>CommunityService: Community.create(command, rateLimitingService, rateLimitSubject)
CommunityService->>RateLimiter: consume({ feature: "community.create", subject: rateLimitSubject })
alt [decision.allowed]
RateLimiter-->>CommunityService: RateLimitDecision { allowed: true, remaining }
CommunityService->>CommunityService: perform community creation
else [!decision.allowed]
RateLimiter-->>CommunityService: RateLimitDecision { allowed: false, retryAfterMs }
CommunityService-->>User: Error("Rate limit exceeded for feature community.create")
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 6 issues, and left some high level feedback:
Fixed security issues:
-
js-yaml (link)
-
nanoid (link)
-
In
apps/api/src/index.tstheisRedisflag is hard-coded totrue; consider wiring this to an environment-driven toggle (e.g.,CAGEMATCH_USE_REDISor presence ofREDIS_URL) so you can switch between Mongo and Redis backends without code changes. -
The rate-limit enforcement in
Community.createcurrently throws a genericErrorwith a string message when limits are exceeded; introducing a specific error type (e.g.,RateLimitExceededErrorcarrying feature and timing metadata) would make it easier for callers and middleware to distinguish and handle these failures. -
ServiceMongoRateLimiting reads
COSMOSDB_CONNECTION_STRINGand manages its own Mongo client, which duplicates the existing Mongoose/Mongo infrastructure; you may want to align this with the shared connection or make the separation explicit in configuration/comments to avoid future confusion around multiple Mongo clients.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `apps/api/src/index.ts` the `isRedis` flag is hard-coded to `true`; consider wiring this to an environment-driven toggle (e.g., `CAGEMATCH_USE_REDIS` or presence of `REDIS_URL`) so you can switch between Mongo and Redis backends without code changes.
- The rate-limit enforcement in `Community.create` currently throws a generic `Error` with a string message when limits are exceeded; introducing a specific error type (e.g., `RateLimitExceededError` carrying feature and timing metadata) would make it easier for callers and middleware to distinguish and handle these failures.
- ServiceMongoRateLimiting reads `COSMOSDB_CONNECTION_STRING` and manages its own Mongo client, which duplicates the existing Mongoose/Mongo infrastructure; you may want to align this with the shared connection or make the separation explicit in configuration/comments to avoid future confusion around multiple Mongo clients.
## Individual Comments
### Comment 1
<location path="apps/docs/docs/decisions/0034-redis-backed-application-service-rate-limiting.md" line_range="19" />
<code_context>
+
+Cellix needs rate limiting for application operations such as `community.create`. The limit must be selected by the feature being executed and may vary by stable caller characteristics such as actor type, role, account, or community scope. It must also work when multiple application instances process requests concurrently. It also requires role/user type based distinctions as well.
+
+The implementation needs a storage service that can perform a high volume of small counter operations efficiently, expire counters automatically, work during local development, and remain replaceable. The architectural question is which storage sercvice to use and where the rate-limit decision should be made.
+
+## Decision Drivers
</code_context>
<issue_to_address>
**issue (typo):** Fix the typo in "sercvice".
Please correct "sercvice" to "service" in this sentence.
```suggestion
The implementation needs a storage service that can perform a high volume of small counter operations efficiently, expire counters automatically, work during local development, and remain replaceable. The architectural question is which storage service to use and where the rate-limit decision should be made.
```
</issue_to_address>
### Comment 2
<location path="apps/docs/docs/decisions/0034-redis-backed-application-service-rate-limiting.md" line_range="23" />
<code_context>
+
+## Decision Drivers
+
+1. **Fast operations**: Rate-limit checks need to be efficient - theres a lot of possible volume with AI driven requests.
+2. **Feature and role awareness**: Policies must be selected from application context, not only from an HTTP route or client address.
+3. **Distributed consistency**: All instances of the application must observe the same counters. This means a local to machine hosted version is not enough.
</code_context>
<issue_to_address>
**issue (typo):** Use "there's" instead of "theres".
Update this documentation line to use the correct contraction: change "theres" to "there's".
```suggestion
1. **Fast operations**: Rate-limit checks need to be efficient - there's a lot of possible volume with AI driven requests.
```
</issue_to_address>
### Comment 3
<location path="apps/docs/docs/decisions/0034-redis-backed-application-service-rate-limiting.md" line_range="43" />
<code_context>
+
+Chosen option: **Redis-backed rate limiting through a generic application service**, because Redis best meets the requirements for speed, counter operations, automatic expiration, local compatability, and future implementation flexibility.
+
+Redis is the recommended primary service for rate limiting in Cellix. MongoDB remains a compatabile alternative if Redis for whatever reason does not work for the organization, but the speed difference will be very noticable.
+
+The application selects the concrete service when it composes infrastructure services. Service selection is therefore very flexible, and solutions could be swapped out with relative ease.
</code_context>
<issue_to_address>
**issue (typo):** Fix spelling of "compatabile" and "noticable".
Please update the spellings to "compatible" and "noticeable" in this sentence.
Suggested implementation:
```
Chosen option: **Redis-backed rate limiting through a generic application service**, because Redis best meets the requirements for speed, counter operations, automatic expiration, local compatibility, and future implementation flexibility.
```
```
Redis is the recommended primary service for rate limiting in Cellix. MongoDB remains a compatible alternative if Redis for whatever reason does not work for the organization, but the speed difference will be very noticeable.
```
</issue_to_address>
### Comment 4
<location path="apps/docs/docs/decisions/0034-redis-backed-application-service-rate-limiting.md" line_range="49" />
<code_context>
+
+### Service Boundary
+
+Rate limiting is integrated at the **application-service level** rather than at a the register function level for endpoints, or deeper at the domain level.
+
+The application service is the first boundary that has all of the information needed to make the business operation decision without letting the request drive too deep into the application, which also follows our typical infrastructure service usage model:
</code_context>
<issue_to_address>
**issue (typo):** Remove the duplicated article in "at a the register function level".
Change "at a the register function level" to "at the register function level" or "at a register function level" to remove the extra article.
```suggestion
Rate limiting is integrated at the **application-service level** rather than at the register function level for endpoints, or deeper at the domain level.
```
</issue_to_address>
### Comment 5
<location path="apps/docs/docs/decisions/0034-redis-backed-application-service-rate-limiting.md" line_range="59" />
<code_context>
+- the user type or role used by policy criteria
+- the point at which the scenario is about to perform its application work
+
+For example, the application service can pass `feature: 'community.create'` with information about the actual user. The application policy can then give the userusing that feature a limit of five operations per fifteen minutes and staff actors a limit of twenty operations per fifteen minutes.
+
+This placement also means rate limiting applies to an application use case, not just over a single endpoint. The same application service can be called from GraphQL, REST, or some other layer without any difference.
</code_context>
<issue_to_address>
**issue (typo):** Add a space in "userusing".
In this sentence, change "userusing" to "user using".
```suggestion
For example, the application service can pass `feature: 'community.create'` with information about the actual user. The application policy can then give the user using that feature a limit of five operations per fifteen minutes and staff actors a limit of twenty operations per fifteen minutes.
```
</issue_to_address>
### Comment 6
<location path="apps/docs/docs/decisions/0034-redis-backed-application-service-rate-limiting.md" line_range="128" />
<code_context>
+- Bad, because it provides no particular enforcement benefit over the Mongo solution.
+- Bad, because the speed of this solution provides no benefit over the existing Mongo infrastructure.
+
+The time-series option is has been written off a possible solution due to the one benefit it provides not outweighing the negative impact on local development and performance, along with overhead.
+
+## More Information
</code_context>
<issue_to_address>
**suggestion (typo):** Tighten the phrasing in "is has been written off a possible solution".
Rephrase this clause to "has been written off as a possible solution" (remove "is" and add "as") to fix the grammar.
```suggestion
The time-series option has been written off as a possible solution due to the one benefit it provides not outweighing the negative impact on local development and performance, along with overhead.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a backend-neutral application rate-limiting layer with Redis and MongoDB implementations, wire it into the API/community creation flow, and add Redis local-dev and test infrastructure.
New Features:
Enhancements:
Build:
Tests: