Conversation
WalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
participant WA as web-app<br/>weekly-report
participant Queue as Queue System
participant CT as core-telegram<br/>handler
participant TG as Telegram
WA->>WA: Create flow item
WA->>Queue: Publish FlowItemCreated event
Queue->>Queue: Event enqueued
Queue->>CT: Dispatch to handler
CT->>CT: Format item summary<br/>& description
CT->>TG: Send message with<br/>Atrium deep link
TG-->>CT: Message sent
CT-->>Queue: Handler returns true
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span multiple interconnected files across the queue package and consuming applications, introducing a new event type with corresponding entity, publisher, and handler. While individual changes follow consistent patterns, the distributed nature and mix of type definitions, entity structure, event publishing logic, and Telegram integration require attention across several domains. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/queue/src/repository/index.ts (1)
5-8: Make repository members readonly.Prevents accidental reassignment and clarifies intent.
Apply:
-class QueueRepository extends Repository { - flow: Flow = new Flow(this) - notification: Notification = new Notification(this) - telegram: Telegram = new Telegram(this) - ticket: Ticket = new Ticket(this) +class QueueRepository extends Repository { + readonly flow: Flow = new Flow(this) + readonly notification: Notification = new Notification(this) + readonly telegram: Telegram = new Telegram(this) + readonly ticket: Ticket = new Ticket(this)apps/web-app/server/tasks/ai/weekly-report.ts (1)
80-86: Add idempotency to prevent duplicate weekly items/events.Retries or overlapping schedules can create multiple “weekly_task_report” items for the same week. Use a deterministic key (e.g., year-week) and upsert/guard before create.
Example approach:
- Compute key =
${format(new Date(), 'yyyy')}-${format(new Date(), 'II')}(ISO week).- Add a unique DB constraint on (type, key) or use create/update with where unique.
apps/core-telegram/server/services/queue/index.ts (1)
64-90: Consider extracting a helper for Telegram startapp links.Both
handleFlowItemCreatedandhandleUserBeaconOnEpicCommentCreatedfollow a similar pattern:
- Create a
separatorconstant with value'zzzzz'- Build
startAppDatausing the same separator pattern- Send messages with inline keyboards containing startapp URLs
Consider extracting a helper function to reduce duplication:
function buildStartAppLink(entityType: string, entityId: string): string { const separator = 'zzzzz' const startAppData = `${entityType}${separator}${entityId}` return `https://t.me/sushi_atrium_bot/app?startapp=${startAppData}` }Then use it in both handlers:
const url = buildStartAppLink('flow', data.itemId) // ... reply_markup: { inline_keyboard: [[{ text: '👉 Открыть Атриум', url, }]] }packages/queue/src/repository/types.ts (1)
40-48: Consider refining thetypefield for better type safety.The
typefield inFlowItemCreatedDatais currently typed asstring, which allows any string value. If thetypefield has a known set of possible values (e.g., "task", "milestone", "note"), consider using a string literal union or enum for better type safety and IDE autocomplete.Example:
type FlowItemType = 'task' | 'milestone' | 'note' | 'other' type FlowItemCreatedData = { itemId: string type: FlowItemType title: string description: string }This would prevent typos and make the API more discoverable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/core-telegram/server/services/queue/index.ts(3 hunks)apps/web-app/server/tasks/ai/daily-report.ts(0 hunks)apps/web-app/server/tasks/ai/weekly-report.ts(2 hunks)packages/queue/src/repository/entities/flow.ts(1 hunks)packages/queue/src/repository/entities/index.ts(1 hunks)packages/queue/src/repository/entities/telegram.ts(1 hunks)packages/queue/src/repository/index.ts(1 hunks)packages/queue/src/repository/types.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/web-app/server/tasks/ai/daily-report.ts
🧰 Additional context used
🧬 Code graph analysis (4)
apps/core-telegram/server/services/queue/index.ts (2)
packages/queue/src/repository/types.ts (1)
FlowItemCreated(46-48)apps/core-telegram/server/services/telegram/atrium-bot.ts (1)
useAtriumBot(184-190)
packages/queue/src/repository/entities/flow.ts (1)
packages/queue/src/repository/index.ts (1)
repository(11-11)
apps/web-app/server/tasks/ai/weekly-report.ts (1)
packages/queue/src/repository/types.ts (1)
FlowItemCreated(46-48)
packages/queue/src/repository/index.ts (1)
packages/queue/src/repository/entities/flow.ts (1)
Flow(4-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
packages/queue/src/repository/entities/index.ts (1)
1-1: LGTM: barrel now exposes Flow.Simplifies downstream imports.
packages/queue/src/repository/entities/flow.ts (1)
4-12: LGTM: Flow entity scaffolded correctly.Matches repository wiring; safe to extend with producers/consumers later.
apps/web-app/server/tasks/ai/weekly-report.ts (1)
1-1: ****The concern about an alias mismatch is incorrect. The codebase shows a normal layered dependency structure:
@roll-stack/queueis the actual package name (defined inpackages/queue/package.json), and internally it depends on@nextorders/queue(an external upstream package). All imports from consuming apps, including the reviewed file, correctly use@roll-stack/queue. No build or runtime issues from this structure.Likely an incorrect or invalid review comment.
apps/core-telegram/server/services/queue/index.ts (2)
1-1: LGTM!The import statement correctly includes the new
FlowItemCreatedtype.
13-13: LGTM!Handler registration follows the existing pattern and is correctly integrated.
packages/queue/src/repository/types.ts (2)
6-6: LGTM!The new event type follows the existing naming convention and is properly integrated into the enum.
9-9: LGTM!The union type is correctly updated to include the new
FlowItemCreatedevent.
| async function handleFlowItemCreated(data: FlowItemCreated['data']): Promise<boolean> { | ||
| try { | ||
| const separator = 'zzzzz' | ||
| const startAppData = `flow${separator}${data.itemId}` | ||
|
|
||
| // Get first words | ||
| const messageIntro = data.description.split(' ').slice(0, 45).join(' ') | ||
| const preparedMessage = `${messageIntro}...\n\nОстальное в Атриуме 🙃` | ||
|
|
||
| await useAtriumBot().api.sendMessage(telegram.teamGroupId, preparedMessage, { | ||
| link_preview_options: { | ||
| is_disabled: true, | ||
| }, | ||
| reply_markup: { | ||
| inline_keyboard: [[{ | ||
| text: '👉 Открыть Атриум', | ||
| url: `https://t.me/sushi_atrium_bot/app?startapp=${startAppData}`, | ||
| }]], | ||
| }, | ||
| }) | ||
|
|
||
| return true | ||
| } catch (error) { | ||
| console.error(error) | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
Add input validation and fix truncation logic.
The function has several issues:
- Missing input validation: No checks that
data.descriptionexists or is a string before calling.split(), which could cause runtime errors. - Incorrect truncation logic: The ellipsis
...is always appended, even when the description is shorter than 45 words. - Fragile text splitting: Using
split(' ')doesn't handle multiple consecutive spaces, newlines, or other whitespace characters well.
Apply this diff to add validation and improve truncation:
async function handleFlowItemCreated(data: FlowItemCreated['data']): Promise<boolean> {
try {
+ if (!data.description || typeof data.description !== 'string') {
+ console.error('Invalid description in FlowItemCreated event')
+ return false
+ }
+
const separator = 'zzzzz'
const startAppData = `flow${separator}${data.itemId}`
- // Get first words
- const messageIntro = data.description.split(' ').slice(0, 45).join(' ')
- const preparedMessage = `${messageIntro}...\n\nОстальное в Атриуме 🙃`
+ // Get first 45 words, handling whitespace properly
+ const words = data.description.trim().split(/\s+/)
+ const messageIntro = words.slice(0, 45).join(' ')
+ const preparedMessage = words.length > 45
+ ? `${messageIntro}...\n\nОстальное в Атриуме 🙃`
+ : `${messageIntro}\n\nОстальное в Атриуме 🙃`
await useAtriumBot().api.sendMessage(telegram.teamGroupId, preparedMessage, {📝 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.
| async function handleFlowItemCreated(data: FlowItemCreated['data']): Promise<boolean> { | |
| try { | |
| const separator = 'zzzzz' | |
| const startAppData = `flow${separator}${data.itemId}` | |
| // Get first words | |
| const messageIntro = data.description.split(' ').slice(0, 45).join(' ') | |
| const preparedMessage = `${messageIntro}...\n\nОстальное в Атриуме 🙃` | |
| await useAtriumBot().api.sendMessage(telegram.teamGroupId, preparedMessage, { | |
| link_preview_options: { | |
| is_disabled: true, | |
| }, | |
| reply_markup: { | |
| inline_keyboard: [[{ | |
| text: '👉 Открыть Атриум', | |
| url: `https://t.me/sushi_atrium_bot/app?startapp=${startAppData}`, | |
| }]], | |
| }, | |
| }) | |
| return true | |
| } catch (error) { | |
| console.error(error) | |
| return false | |
| } | |
| } | |
| async function handleFlowItemCreated(data: FlowItemCreated['data']): Promise<boolean> { | |
| try { | |
| if (!data.description || typeof data.description !== 'string') { | |
| console.error('Invalid description in FlowItemCreated event') | |
| return false | |
| } | |
| const separator = 'zzzzz' | |
| const startAppData = `flow${separator}${data.itemId}` | |
| // Get first 45 words, handling whitespace properly | |
| const words = data.description.trim().split(/\s+/) | |
| const messageIntro = words.slice(0, 45).join(' ') | |
| const preparedMessage = words.length > 45 | |
| ? `${messageIntro}...\n\nОстальное в Атриуме 🙃` | |
| : `${messageIntro}\n\nОстальное в Атриуме 🙃` | |
| await useAtriumBot().api.sendMessage(telegram.teamGroupId, preparedMessage, { | |
| link_preview_options: { | |
| is_disabled: true, | |
| }, | |
| reply_markup: { | |
| inline_keyboard: [[{ | |
| text: '👉 Открыть Атриум', | |
| url: `https://t.me/sushi_atrium_bot/app?startapp=${startAppData}`, | |
| }]], | |
| }, | |
| }) | |
| return true | |
| } catch (error) { | |
| console.error(error) | |
| return false | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/core-telegram/server/services/queue/index.ts around lines 64 to 90,
validate that data.description exists and is a string (fallback to empty string
or return false) before processing; split words using a robust whitespace
pattern (e.g. description.trim().split(/\s+/) or match word tokens) instead of
split(' ') to handle multiple spaces/newlines; only append the ellipsis when the
description was actually truncated (i.e., when word count > 45) and otherwise
use the full description without adding '...'; ensure preparedMessage is built
from the joined first 45 words (no extra spaces) and add the trailing hint text
unchanged.
| // Push Event | ||
| await queue.publish<FlowItemCreated>(Events.flowItemCreated, { | ||
| itemId: item.id, | ||
| type: item.type, | ||
| title: item.title, | ||
| description: item.description ?? '', | ||
| }) |
There was a problem hiding this comment.
Event payload may carry long PII-heavy content; clamp or slim it.
Weekly report text can be large and contain staff/task details. Sending full description over the bus risks privacy exposure and broker/message-size issues. Prefer publishing only identifiers, or at least truncate description.
Apply minimal change to clamp payload:
- await queue.publish<FlowItemCreated>(Events.flowItemCreated, {
+ await queue.publish<FlowItemCreated>(Events.flowItemCreated, {
itemId: item.id,
type: item.type,
title: item.title,
- description: item.description ?? '',
+ description: (item.description ?? '').slice(0, 2000),
})Longer-term: publish only itemId and let consumers load details from DB.
📝 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.
| // Push Event | |
| await queue.publish<FlowItemCreated>(Events.flowItemCreated, { | |
| itemId: item.id, | |
| type: item.type, | |
| title: item.title, | |
| description: item.description ?? '', | |
| }) | |
| // Push Event | |
| await queue.publish<FlowItemCreated>(Events.flowItemCreated, { | |
| itemId: item.id, | |
| type: item.type, | |
| title: item.title, | |
| description: (item.description ?? '').slice(0, 2000), | |
| }) |
🤖 Prompt for AI Agents
In apps/web-app/server/tasks/ai/weekly-report.ts around lines 88 to 94, the
event payload includes the full item.description which may be large and contain
PII; replace that with a clamped/trimmed value (e.g., limit to a small fixed
length like 200 characters and optionally strip newlines) or omit it entirely so
the message only carries identifiers (itemId, type, title) and minimal text;
implement this minimal change now (truncate or set to empty string/null), and
note longer-term to publish only itemId and let consumers fetch full details
from the DB.
| Events.ticketMessageCreated, | ||
| Events.notificationUserBeaconOnEpicCommentCreated, | ||
| Events.flowItemCreated, | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
Subscription looks good; verify consumer handler and idempotency.
Telegram is now subscribed to flowItemCreated. Ensure a concrete handler exists and deduplicates to avoid duplicate Telegram/Atrium sends on retries/at‑least‑once delivery.
Run:
🏁 Script executed:
#!/bin/bash
# 1) Confirm a handler exists
rg -nP -C2 '\bflowItemCreated\b' apps/core-telegram/server/services/queue -S
# 2) Check for dedup/idempotency in the handler
rg -nP -C2 'dedup|idempot|once|messageId|processed' apps/core-telegram/server/services/queue -SLength of output: 455
🏁 Script executed:
#!/bin/bash
# Find and inspect handleFlowItemCreated implementation
rg -nA20 'handleFlowItemCreated\s*=' apps/core-telegram/server/services/queue -SLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check for any dedup/idempotency patterns in broader scope
rg -nP 'dedup|idempot|once|messageId|processed' apps/core-telegram/server/services -SLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for event processing patterns or message tracking in queue handlers
rg -nP 'cache|Map|Set|Store|tracking|duplicate' apps/core-telegram/server/services/queue -SLength of output: 307
🏁 Script executed:
#!/bin/bash
# Find where handleFlowItemCreated is imported or defined
rg -nP 'handleFlowItemCreated' apps/core-telegram --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check the index.ts file to see the full imports and structure
cat apps/core-telegram/server/services/queue/index.tsLength of output: 2915
🏁 Script executed:
#!/bin/bash
# Look for queue handlers directory or handlers file
fd -type f -name '*.ts' -path '*queue*' apps/core-telegram/server | head -20Length of output: 170
Handler exists but lacks idempotency protection; duplicates will be sent on retries.
Verification confirms handleFlowItemCreated sends a message to telegram.teamGroupId without any dedup mechanism. Under at-least-once delivery, event reprocessing causes duplicate Telegram messages in the group. All three handlers (ticketMessageCreated, userBeaconOnEpicCommentCreated, flowItemCreated) share this vulnerability.
Add idempotency: track processed event IDs (e.g., in a cache or database) or make the handler idempotent before consuming this event.
1 participant
Summary by CodeRabbit
New Features
Chores