implement stronger input validation for tip metadata.#292
Merged
OlufunbiIK merged 3 commits intoOlufunbiIK:mainfrom Mar 26, 2026
Merged
implement stronger input validation for tip metadata.#292OlufunbiIK merged 3 commits intoOlufunbiIK:mainfrom
OlufunbiIK merged 3 commits intoOlufunbiIK:mainfrom
Conversation
|
@ohamamarachi474-del is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR adds metadata validation and persistence to tip creation via new DTO fields with constraints, introduces an in-memory event store for sequencing notifications, and enables client catch-up by replaying missed WebSocket events. The WebSocketModule is registered in the application's module graph, and integration tests verify end-to-end notification delivery and replay behavior. Changes
Sequence DiagramsequenceDiagram
participant Client as Client (WebSocket)
participant Gateway as WebSocket Gateway
participant EventStore as Event Store
participant TipsService as Tips Service
participant EventBus as Event Bus
Note over Client,EventBus: Tip Verification & Initial Notification
Client->>Gateway: join_artist_room {artistId, lastSequenceId?}
Gateway->>EventStore: getLatestSequenceId()
Gateway->>Client: connected.latestSequenceId
alt Client provides lastSequenceId
Gateway->>EventStore: getEventsAfter(lastSequenceId, [artistId])
EventStore-->>Gateway: missed events
Gateway->>Client: tip_notification (replayed events)
end
TipsService->>EventBus: emit tip.verified event
EventBus->>Gateway: `@OnEvent`('tip.verified')
Gateway->>Gateway: sendTipNotification()
rect rgba(100, 150, 200, 0.5)
Gateway->>EventStore: storeEvent('tip_received', payload, rooms)
EventStore-->>Gateway: StoredEvent {sequenceId, ...}
end
Gateway->>Client: tip_notification {type, sequenceId, data}
Client->>Gateway: ack {sequenceId}
Gateway-->>Gateway: log acknowledgement
Note over Client,EventStore: Client Reconnects for Catch-up
Client->>Gateway: join_artist_room {artistId, lastSequenceId: 5}
Gateway->>EventStore: getEventsAfter(5, [artistId])
EventStore-->>Gateway: events with sequenceId > 5
Gateway->>Client: tip_notification (missed event with sequenceId 6)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/tips/create-tips.dto.ts`:
- Around line 6-35: TipMetadataDto's nested fields (source, campaign, platform,
os, version) are not sanitized before persistence; add the same sanitization
used for message by applying the `@SanitiseAsPlainText`() decorator to each of
these properties in TipMetadataDto so they are cleaned during validation, or
alternatively ensure tips.service.ts serializes only after passing them through
the existing sanitiser function used for message (the code path around the
serialization call in tips.service.ts where metadata is JSON.stringified).
Reference TipMetadataDto and the serialization point in tips.service.ts and
apply the sanitiser consistently to prevent HTML/control characters from
reaching storage.
In `@backend/src/websocket/event-store.service.ts`:
- Line 16: The code silently returns a truncated tail when a client-supplied
sequenceId precedes the oldest retained event (maxWindowSize), so update the
logic in the event-store code (e.g., inside getEventsAfter and the
reconnection/replay path that consumes sequenceId) to detect stale cursors:
compare the requested sequenceId against the earliest retained sequence (compute
from current store head and maxWindowSize) and, when the request is older,
return an explicit error/indicator (or special response) instead of returning a
partial backlog; ensure the error includes the requested sequenceId and the
earliest available sequence to allow the client to decide whether to request a
full snapshot or resync.
In `@backend/src/websocket/websocket.gateway.ts`:
- Around line 167-179: The notification payload in websocket.gateway.ts
currently sets asset: tip.asset which is undefined for newly verified tips
because tips.service.ts populates assetCode; update the payload construction
(the payload object near where tip is read) to include the correct asset
identifier by using tip.assetCode (or set assetCode: tip.assetCode and/or
replace asset with assetCode) so clients receive the non-XLM asset code instead
of undefined.
- Around line 190-198: Currently the gateway emits 'tip_notification' twice when
a socket is in both rooms by calling this.server.to(artistRoom).emit(...) and
again this.server.to(trackRoom).emit(...); change this to emit once to the union
of rooms by chaining room targets on the same server broadcast (use
this.server.to(artistRoom).to(trackRoom).emit(...) when trackRoom exists) so
clients in both rooms receive a single event; also collapse the two logger calls
into one single log after the emit (include storedEvent.sequenceId and the list
of target rooms like artistRoom and trackRoom) and keep the fallback to emit
only to artistRoom when trackRoom is falsy.
In `@backend/test/websocket-delivery.spec.ts`:
- Around line 17-19: The test currently boots the full AppModule via
Test.createTestingModule({ imports: [AppModule] }), which pulls real
Redis/Postgres wiring and makes the suite fragile; instead build a narrow test
module that only includes the gateway/event-store pieces or stub the external
providers: replace imports: [AppModule] with a Test.createTestingModule that
imports the specific gateway module(s) (e.g., the websocket gateway or
EventStore module) or registers the gateway provider(s) directly, and override
external providers like the Redis client provider (e.g., RedisService or the
token used in your app) and the DB providers (DataSource/TypeOrm or
getRepositoryToken(...) repositories) via .overrideProvider(...).useValue(mock)
so the test uses lightweight mocks rather than real Redis/Postgres.
🪄 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: 1178aef2-f324-47db-a480-6d9f9d023fe2
📒 Files selected for processing (8)
backend/src/app.module.tsbackend/src/tips/create-tips.dto.tsbackend/src/tips/tips.service.tsbackend/src/websocket/event-store.service.tsbackend/src/websocket/websocket.gateway.tsbackend/src/websocket/websocket.module.tsbackend/test/tips-validation.unit.spec.tsbackend/test/websocket-delivery.spec.ts
Comment on lines
+6
to
+35
| export class TipMetadataDto { | ||
| @ApiPropertyOptional({ description: 'Source of the tip', example: 'mobile_app' }) | ||
| @IsOptional() | ||
| @IsString() | ||
| @MaxLength(50) | ||
| source?: string; | ||
|
|
||
| @ApiPropertyOptional({ description: 'Campaign ID', example: 'summer2023' }) | ||
| @IsOptional() | ||
| @IsString() | ||
| @MaxLength(100) | ||
| campaign?: string; | ||
|
|
||
| @ApiPropertyOptional({ description: 'Platform (web/ios/android)', example: 'android' }) | ||
| @IsOptional() | ||
| @IsString() | ||
| @MaxLength(20) | ||
| platform?: string; | ||
|
|
||
| @ApiPropertyOptional({ description: 'Operating System', example: 'Android 13' }) | ||
| @IsOptional() | ||
| @IsString() | ||
| @MaxLength(50) | ||
| os?: string; | ||
|
|
||
| @ApiPropertyOptional({ description: 'Client App Version', example: '2.1.0' }) | ||
| @IsOptional() | ||
| @IsString() | ||
| @MaxLength(20) | ||
| version?: string; |
There was a problem hiding this comment.
Sanitize nested metadata before persisting it.
These fields are length/type checked, but unlike message they never run through @SanitiseAsPlainText(). Line 182 of backend/src/tips/tips.service.ts then serializes them directly, so HTML/control characters still reach storage.
Suggested fix
`@IsOptional`()
`@IsString`()
+ `@SanitiseAsPlainText`()
`@MaxLength`(50)
source?: string;
@@
`@IsOptional`()
`@IsString`()
+ `@SanitiseAsPlainText`()
`@MaxLength`(100)
campaign?: string;
@@
`@IsOptional`()
`@IsString`()
+ `@SanitiseAsPlainText`()
`@MaxLength`(20)
platform?: string;
@@
`@IsOptional`()
`@IsString`()
+ `@SanitiseAsPlainText`()
`@MaxLength`(50)
os?: string;
@@
`@IsOptional`()
`@IsString`()
+ `@SanitiseAsPlainText`()
`@MaxLength`(20)
version?: string;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export class TipMetadataDto { | |
| @ApiPropertyOptional({ description: 'Source of the tip', example: 'mobile_app' }) | |
| @IsOptional() | |
| @IsString() | |
| @MaxLength(50) | |
| source?: string; | |
| @ApiPropertyOptional({ description: 'Campaign ID', example: 'summer2023' }) | |
| @IsOptional() | |
| @IsString() | |
| @MaxLength(100) | |
| campaign?: string; | |
| @ApiPropertyOptional({ description: 'Platform (web/ios/android)', example: 'android' }) | |
| @IsOptional() | |
| @IsString() | |
| @MaxLength(20) | |
| platform?: string; | |
| @ApiPropertyOptional({ description: 'Operating System', example: 'Android 13' }) | |
| @IsOptional() | |
| @IsString() | |
| @MaxLength(50) | |
| os?: string; | |
| @ApiPropertyOptional({ description: 'Client App Version', example: '2.1.0' }) | |
| @IsOptional() | |
| @IsString() | |
| @MaxLength(20) | |
| version?: string; | |
| export class TipMetadataDto { | |
| `@ApiPropertyOptional`({ description: 'Source of the tip', example: 'mobile_app' }) | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@SanitiseAsPlainText`() | |
| `@MaxLength`(50) | |
| source?: string; | |
| `@ApiPropertyOptional`({ description: 'Campaign ID', example: 'summer2023' }) | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@SanitiseAsPlainText`() | |
| `@MaxLength`(100) | |
| campaign?: string; | |
| `@ApiPropertyOptional`({ description: 'Platform (web/ios/android)', example: 'android' }) | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@SanitiseAsPlainText`() | |
| `@MaxLength`(20) | |
| platform?: string; | |
| `@ApiPropertyOptional`({ description: 'Operating System', example: 'Android 13' }) | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@SanitiseAsPlainText`() | |
| `@MaxLength`(50) | |
| os?: string; | |
| `@ApiPropertyOptional`({ description: 'Client App Version', example: '2.1.0' }) | |
| `@IsOptional`() | |
| `@IsString`() | |
| `@SanitiseAsPlainText`() | |
| `@MaxLength`(20) | |
| version?: string; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/tips/create-tips.dto.ts` around lines 6 - 35, TipMetadataDto's
nested fields (source, campaign, platform, os, version) are not sanitized before
persistence; add the same sanitization used for message by applying the
`@SanitiseAsPlainText`() decorator to each of these properties in TipMetadataDto
so they are cleaned during validation, or alternatively ensure tips.service.ts
serializes only after passing them through the existing sanitiser function used
for message (the code path around the serialization call in tips.service.ts
where metadata is JSON.stringified). Reference TipMetadataDto and the
serialization point in tips.service.ts and apply the sanitiser consistently to
prevent HTML/control characters from reaching storage.
| private readonly logger = new Logger(EventStoreService.name); | ||
| private events: StoredEvent[] = []; | ||
| private nextSequenceId = 1; | ||
| private readonly maxWindowSize = 1000; // Store up to 1000 recent events |
There was a problem hiding this comment.
Detect stale cursors instead of silently replaying a partial backlog.
Once the 1000-event window rolls over, older events are evicted. If a client reconnects with a sequenceId older than the first retained event, getEventsAfter() returns only the tail and gives no signal that earlier events were lost. Clients will think replay completed successfully when it did not.
Also applies to: 33-35, 44-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/websocket/event-store.service.ts` at line 16, The code silently
returns a truncated tail when a client-supplied sequenceId precedes the oldest
retained event (maxWindowSize), so update the logic in the event-store code
(e.g., inside getEventsAfter and the reconnection/replay path that consumes
sequenceId) to detect stale cursors: compare the requested sequenceId against
the earliest retained sequence (compute from current store head and
maxWindowSize) and, when the request is older, return an explicit
error/indicator (or special response) instead of returning a partial backlog;
ensure the error includes the requested sequenceId and the earliest available
sequence to allow the client to decide whether to request a full snapshot or
resync.
Comment on lines
+167
to
+179
| const payload = { | ||
| tipId: tip.id, | ||
| artistId: tip.artistId, | ||
| trackId: tip.trackId, | ||
| amount: tip.amount, | ||
| asset: tip.asset, | ||
| message: tip.message, | ||
| senderAddress: tip.isAnonymous ? undefined : tip.senderAddress, | ||
| isAnonymous: tip.isAnonymous, | ||
| createdAt: tip.createdAt, | ||
| artist: tip.artist, | ||
| track: tip.track, | ||
| }; |
There was a problem hiding this comment.
Send assetCode in the notification payload.
Lines 152-177 of backend/src/tips/tips.service.ts populate assetCode, not asset, so this emits asset: undefined for newly verified tips. Non-XLM tips will be mislabeled by clients that fall back to XLM.
Suggested fix
- asset: tip.asset,
+ asset: tip.assetCode ?? tip.asset,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/websocket/websocket.gateway.ts` around lines 167 - 179, The
notification payload in websocket.gateway.ts currently sets asset: tip.asset
which is undefined for newly verified tips because tips.service.ts populates
assetCode; update the payload construction (the payload object near where tip is
read) to include the correct asset identifier by using tip.assetCode (or set
assetCode: tip.assetCode and/or replace asset with assetCode) so clients receive
the non-XLM asset code instead of undefined.
Comment on lines
190
to
198
| // Send to artist room | ||
| const artistRoom = `artist_${tip.artistId}`; | ||
| this.server.to(artistRoom).emit('tip_notification', notificationData); | ||
| this.logger.log(`Sent tip notification to room: ${artistRoom}`); | ||
| this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${artistRoom}`); | ||
|
|
||
| // Send to track room if track is specified | ||
| if (tip.trackId) { | ||
| const trackRoom = `track_${tip.trackId}`; | ||
| if (trackRoom) { | ||
| this.server.to(trackRoom).emit('tip_notification', notificationData); | ||
| this.logger.log(`Sent tip notification to room: ${trackRoom}`); | ||
| this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${trackRoom}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "websocket.gateway.ts"Repository: OlufunbiIK/tip-tune
Length of output: 107
🏁 Script executed:
wc -l ./backend/src/websocket/websocket.gateway.tsRepository: OlufunbiIK/tip-tune
Length of output: 111
🏁 Script executed:
sed -n '150,220p' ./backend/src/websocket/websocket.gateway.tsRepository: OlufunbiIK/tip-tune
Length of output: 2678
🏁 Script executed:
grep -n "socket.join\|client.join" ./backend/src/websocket/websocket.gateway.tsRepository: OlufunbiIK/tip-tune
Length of output: 115
🏁 Script executed:
sed -n '75,125p' ./backend/src/websocket/websocket.gateway.tsRepository: OlufunbiIK/tip-tune
Length of output: 1651
🏁 Script executed:
head -30 ./backend/src/websocket/websocket.gateway.tsRepository: OlufunbiIK/tip-tune
Length of output: 879
🏁 Script executed:
cat > /tmp/test_socketio.js << 'EOF'
// Testing Socket.io room behavior
// Documenting Socket.io API
// According to Socket.io documentation:
// server.to(room1).to(room2).emit() - emits to the INTERSECTION (rooms that have both)
// To emit to UNION, you need to emit separately or use different approach
console.log("Socket.io room chaining behavior:");
console.log("server.to(room1).emit() - emits to all sockets in room1");
console.log("server.to(room1).to(room2).emit() - emits only to sockets in BOTH room1 AND room2 (intersection)");
console.log("");
console.log("If a socket is in artist_1 and track_5:");
console.log("- Current code: emits to artist_1, then emits to track_5 -> socket receives twice");
console.log("- Suggested fix with chaining: would only emit to sockets in BOTH rooms");
console.log("");
console.log("To solve the duplicate without losing messages:");
console.log("- Option 1: Track socket IDs and emit once per socket");
console.log("- Option 2: Use namespace or user-based approach");
console.log("- Option 3: Emit to union and deduplicate at client");
EOF
cat /tmp/test_socketio.jsRepository: OlufunbiIK/tip-tune
Length of output: 1110
🌐 Web query:
socket.io server.to(room1).to(room2).emit behavior intersection union
💡 Result:
The behavior of server.to(room1).to(room2).emit(...) in Socket.IO is a union of the rooms, not an intersection. Every socket that is in at least one of the specified rooms receives the event exactly once, even if it belongs to multiple rooms. This is explicitly stated in the official Socket.IO documentation across multiple versions: - "In that case, a union is performed: every socket that is at least in one of the rooms will get the event once (even if the socket is in two or more rooms)." (Rooms docs v3 and v4) - Emit cheatsheet example: io.to("room1").to(["room2", "room3"]).except("room4").emit("hello"); describes targeting clients "in at least one of the rooms". The to method chains by accumulating room names in the socket's _rooms array, and the broadcast targets the union via the adapter. There is no intersection behavior; except is used for exclusion instead.
Citations:
- 1: https://socket.io/docs/v4/emit-cheatsheet/
- 2: https://socket.io/docs/v3/rooms/
- 3: https://socket.io/docs/v4/rooms
- 4: https://socket.io/docs/v4/emit-cheatsheet
🏁 Script executed:
sed -n '188,220p' ./backend/src/websocket/websocket.gateway.tsRepository: OlufunbiIK/tip-tune
Length of output: 1415
Broadcast once to the room union.
A socket can be in both artist_* and track_* rooms. The current code emits separately to each room, sending duplicate tip_notification events to clients subscribed to both. Use Socket.io's room chaining to emit to the union of rooms in a single broadcast:
Suggested fix
- this.server.to(artistRoom).emit('tip_notification', notificationData);
- this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${artistRoom}`);
-
- // Send to track room if track is specified
- if (trackRoom) {
- this.server.to(trackRoom).emit('tip_notification', notificationData);
- this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${trackRoom}`);
- }
+ if (trackRoom) {
+ this.server
+ .to(artistRoom)
+ .to(trackRoom)
+ .emit('tip_notification', notificationData);
+ this.logger.log(
+ `Sent tip notification ${storedEvent.sequenceId} to rooms: ${artistRoom}, ${trackRoom}`,
+ );
+ } else {
+ this.server.to(artistRoom).emit('tip_notification', notificationData);
+ this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${artistRoom}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Send to artist room | |
| const artistRoom = `artist_${tip.artistId}`; | |
| this.server.to(artistRoom).emit('tip_notification', notificationData); | |
| this.logger.log(`Sent tip notification to room: ${artistRoom}`); | |
| this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${artistRoom}`); | |
| // Send to track room if track is specified | |
| if (tip.trackId) { | |
| const trackRoom = `track_${tip.trackId}`; | |
| if (trackRoom) { | |
| this.server.to(trackRoom).emit('tip_notification', notificationData); | |
| this.logger.log(`Sent tip notification to room: ${trackRoom}`); | |
| this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${trackRoom}`); | |
| } | |
| if (trackRoom) { | |
| this.server | |
| .to(artistRoom) | |
| .to(trackRoom) | |
| .emit('tip_notification', notificationData); | |
| this.logger.log( | |
| `Sent tip notification ${storedEvent.sequenceId} to rooms: ${artistRoom}, ${trackRoom}`, | |
| ); | |
| } else { | |
| this.server.to(artistRoom).emit('tip_notification', notificationData); | |
| this.logger.log(`Sent tip notification ${storedEvent.sequenceId} to room: ${artistRoom}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/websocket/websocket.gateway.ts` around lines 190 - 198, Currently
the gateway emits 'tip_notification' twice when a socket is in both rooms by
calling this.server.to(artistRoom).emit(...) and again
this.server.to(trackRoom).emit(...); change this to emit once to the union of
rooms by chaining room targets on the same server broadcast (use
this.server.to(artistRoom).to(trackRoom).emit(...) when trackRoom exists) so
clients in both rooms receive a single event; also collapse the two logger calls
into one single log after the emit (include storedEvent.sequenceId and the list
of target rooms like artistRoom and trackRoom) and keep the fallback to emit
only to artistRoom when trackRoom is falsy.
Comment on lines
+17
to
+19
| const moduleFixture: TestingModule = await Test.createTestingModule({ | ||
| imports: [AppModule], | ||
| }).compile(); |
There was a problem hiding this comment.
Avoid booting the full AppModule here.
This pulls in the real Redis and Postgres wiring from Lines 48-83 of backend/src/app.module.ts. The suite will fail whenever that infra is unavailable, which turns a websocket regression test into an environment smoke test. Build a narrow test module for the gateway/event store, or override those external providers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/test/websocket-delivery.spec.ts` around lines 17 - 19, The test
currently boots the full AppModule via Test.createTestingModule({ imports:
[AppModule] }), which pulls real Redis/Postgres wiring and makes the suite
fragile; instead build a narrow test module that only includes the
gateway/event-store pieces or stub the external providers: replace imports:
[AppModule] with a Test.createTestingModule that imports the specific gateway
module(s) (e.g., the websocket gateway or EventStore module) or registers the
gateway provider(s) directly, and override external providers like the Redis
client provider (e.g., RedisService or the token used in your app) and the DB
providers (DataSource/TypeOrm or getRepositoryToken(...) repositories) via
.overrideProvider(...).useValue(mock) so the test uses lightweight mocks rather
than real Redis/Postgres.
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
2 participants
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.
closes #258
Summary by CodeRabbit