-
Notifications
You must be signed in to change notification settings - Fork 133
Created a log file for all commands sent by users #713 #717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new command logging middleware is introduced to track user commands and callback queries via Winston, writing to a timestamped log file. The middleware is registered in the bot's initialization sequence, and the resulting log file is added to gitignore to prevent accidental commits. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
bot/middleware/commandlogging.ts (2)
43-51: Consider reusing existing command parsing logic.This simple whitespace-based parsing duplicates work already done by
commandArgsMiddlewareinbot/middleware/commands.ts, which handles quoted arguments, escape sequences, and other edge cases. The existing middleware populatesctx.state.commandwith parsed command details.💡 Suggested approach
Option 1: Access the already-parsed command from
ctx.state.command(requires moving commandLogger after commandArgsMiddleware):// After commandArgsMiddleware runs: if (ctx.state?.command) { const { command, args } = ctx.state.command; logger.info(`User @${userName} [${userId}] executed command: ${command} with args: [${args.join(', ')}]`); }Option 2: If logging must happen before commandArgsMiddleware, document that this is intentionally simplified parsing for early-stage logging.
70-71: Consider adding more context to generic log message.The fallback log "Received non-command message or update from user" provides minimal debugging value. Consider including the update type or other contextual information.
💡 Example enhancement
-logger.info(`Received non-command message or update from user.`); +logger.info(`Received non-command update: ${ctx.updateType ?? 'unknown'}`);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignorebot/middleware/commandlogging.tsbot/start.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to **/*.ts : Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/middleware/**/*.ts : Maintain middleware chain order: User validation → Admin checking → Context enhancement → Command routing
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/start.ts : Implement bot initialization, command registration, and scheduled job setup in bot/start.ts
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to bot/** : Place commands, scenes, and middleware modules under bot/
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to **/*.ts : Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Applied to files:
bot/middleware/commandlogging.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/**/*.ts : Use custom Telegraf context types (MainContext, CommunityContext) in handlers and modules
Applied to files:
bot/middleware/commandlogging.tsbot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/middleware/**/*.ts : Maintain middleware chain order: User validation → Admin checking → Context enhancement → Command routing
Applied to files:
bot/middleware/commandlogging.tsbot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/start.ts : Implement bot initialization, command registration, and scheduled job setup in bot/start.ts
Applied to files:
bot/middleware/commandlogging.tsbot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to bot/modules/*/{commands,actions,messages,scenes,index}.ts : Structure each feature module with commands.ts, actions.ts, messages.ts, scenes.ts, and index.ts
Applied to files:
bot/middleware/commandlogging.tsbot/start.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to app.ts : Keep startup logic centralized in the root app.ts that bootstraps the Telegram bot and Mongo connection
Applied to files:
bot/start.ts
📚 Learning: 2025-08-14T18:53:14.640Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-14T18:53:14.640Z
Learning: Applies to app.ts : Keep app.ts as the single entry point to connect to MongoDB and initialize the bot
Applied to files:
bot/start.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Applies to bot/** : Place commands, scenes, and middleware modules under bot/
Applied to files:
bot/start.ts
📚 Learning: 2025-10-22T14:03:55.114Z
Learnt from: CR
Repo: lnp2pBot/bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-22T14:03:55.114Z
Learning: Use npm start for the production path (prestart → tsc) to launch the compiled bot
Applied to files:
bot/start.ts
🧬 Code graph analysis (2)
bot/middleware/commandlogging.ts (4)
bot/modules/community/communityContext.ts (1)
CommunityContext(18-22)bot/modules/block/index.ts (1)
bot(8-31)bot/middleware/commands.ts (1)
ctx(3-52)app.ts (1)
logger(28-57)
bot/start.ts (1)
bot/middleware/commandlogging.ts (1)
commandLogger(31-78)
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: ci_to_main
🔇 Additional comments (5)
.gitignore (1)
6-7: LGTM - Log file appropriately excluded.Correctly prevents the new command log file from being committed to version control.
bot/start.ts (2)
77-77: LGTM - Import added correctly.
196-196: Early middleware placement for command logging is correct and intentional.The
commandLogger()middleware runs first to capture all incoming updates comprehensively before any filtering or session handling occurs. This placement aligns with standard logging patterns and serves as an audit trail for all requests. There are no conflicting logging middleware in the codebase, and the middleware does not manipulate context, making early placement appropriate.bot/middleware/commandlogging.ts (2)
56-69: LGTM - Safe callback query handling.The try-catch around
ctx.callbackQuery.message?.textcorrectly handles cases where the message field might be unavailable or have unexpected structure.
72-77: LGTM - Proper error handling and middleware continuation.The error handling ensures logging failures don't break the middleware chain, and
next()is always called to continue processing.
| @@ -0,0 +1,78 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove @ts-nocheck and fix type errors.
The @ts-nocheck directive suppresses all TypeScript checking for this file, hiding potential type errors. TypeScript exists to catch bugs at compile time—disabling it defeats that purpose.
🔎 Recommended fix
Remove the directive and address any type errors that surface:
-// @ts-nocheck
import { MiddlewareFn } from 'telegraf';
import { CommunityContext } from '../modules/community/communityContext';
import winston from 'winston';📝 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.
| // @ts-nocheck |
🤖 Prompt for AI Agents
In bot/middleware/commandlogging.ts at line 1, remove the top-line "@ts-nocheck"
and restore TypeScript checking; then run the TypeScript compiler to surface
errors and fix them by adding precise type annotations for any middleware
function signature, parameters (context, next, request/response), imported
helpers, and returned values; replace unsafe any/unknown casts with correct
types or narrowings, export/import correct types from your framework (e.g.,
Koa/Express/Telegraf types) or define local interfaces for the expected objects,
and if a third-party value truly lacks types, add a minimal module declaration
or a narrow type alias instead of disabling checks globally. Ensure the file
compiles without errors and keep only minimal targeted ts-expect-error comments
if an unavoidable single-line exception remains.
| const logger = winston.createLogger({ | ||
| format: winston.format.combine( | ||
| winston.format.timestamp({ | ||
| format: 'YYYY-MM-DDTHH:mm:ss.SSSZ', | ||
| }), | ||
| winston.format.colorize(), | ||
| winston.format.printf(info => { | ||
| return `[${info.timestamp}] ${info.level}: ${info.message} ${ | ||
| info.stack ? info.stack : '' | ||
| }`; | ||
| }), | ||
| ), | ||
| levels: winston.config.syslog.levels, | ||
| level: 'debug', | ||
| transports: [ | ||
| new winston.transports.File({ | ||
| filename: logFile, | ||
| maxsize: 5 * 1024 * 1024 * 1000, // 5GB | ||
| }), | ||
| ], | ||
| exitOnError: false, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the existing logger and remove colorize from file transport.
Two issues here:
-
Duplicate logger instance: The codebase already has a logger imported from
'../logger'. Creating a separate Winston instance leads to inconsistent configuration and maintenance burden. -
colorize() in file transport: The
winston.format.colorize()formatter adds ANSI escape codes that render properly in terminals but produce unreadable garbage in log files (e.g.,[90m[37m).
🔎 Proposed fix
Import and use the existing logger, or if a separate file is required, remove colorize:
-import winston from 'winston';
+import { logger } from '../../logger';
-const logFile = process.env.COMMAND_LOG_FILE || 'commands.log';
-
-const logger = winston.createLogger({
- format: winston.format.combine(
- winston.format.timestamp({
- format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
- }),
- winston.format.colorize(),
- winston.format.printf(info => {
- return `[${info.timestamp}] ${info.level}: ${info.message} ${
- info.stack ? info.stack : ''
- }`;
- }),
- ),
- levels: winston.config.syslog.levels,
- level: 'debug',
- transports: [
- new winston.transports.File({
- filename: logFile,
- maxsize: 5 * 1024 * 1024 * 1000, // 5GB
- }),
- ],
- exitOnError: false,
-});
+// If a separate file transport is truly needed, add it to the existing logger configurationIf you must use a separate file, remove winston.format.colorize():
const logger = winston.createLogger({
format: winston.format.combine(
winston.format.timestamp({
format: 'YYYY-MM-DDTHH:mm:ss.SSSZ',
}),
- winston.format.colorize(),
winston.format.printf(info => {
return `[${info.timestamp}] ${info.level}: ${info.message} ${
info.stack ? info.stack : ''
}`;
}),
),Based on learnings, "Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant."
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In bot/middleware/commandlogging.ts around lines 8 to 29, the file creates a new
Winston logger and applies winston.format.colorize() which both duplicates the
project's existing logger (imported from '../logger') and injects ANSI color
codes into file output; replace usage with the shared logger from ../logger
(remove the separate createLogger block) or, if a distinct file-only logger is
required, create it without winston.format.colorize() and match the project's
existing format/levels/transports (use same timestamp/printf format and a File
transport without colorize and with a sensible maxsize) so file logs remain
readable and configuration is consistent.
| const userName = msg.from?.username ?? ''; | ||
|
|
||
| logger.info(`User @${userName} [${userId}] ${isCommand? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n bot/middleware/commandlogging.ts | sed -n '50,60p'Repository: lnp2pBot/bot
Length of output: 544
🏁 Script executed:
find . -type f \( -name "*privacy*" -o -name "*PRIVACY*" -o -name "*policy*" -o -name "*POLICY*" -o -name "*.md" \) | head -20Repository: lnp2pBot/bot
Length of output: 305
🏁 Script executed:
fd -t f '\.(log|conf|config|json)$' | grep -E '(log|config)' | head -20Repository: lnp2pBot/bot
Length of output: 88
🏁 Script executed:
rg -i 'logger|console|log' bot/middleware/commandlogging.ts -A 2 -B 2Repository: lnp2pBot/bot
Length of output: 1242
🏁 Script executed:
cat README.md | head -100Repository: lnp2pBot/bot
Length of output: 7179
🏁 Script executed:
rg -i 'privacy|personal data|user data|gdpr|retention|pii|sensitive' README.mdRepository: lnp2pBot/bot
Length of output: 38
🏁 Script executed:
cat CONTRIBUTING.mdRepository: lnp2pBot/bot
Length of output: 4212
🏁 Script executed:
rg -i 'log.*rotat|maxfiles|maxdays|retention|delete.*log' --type ts --type jsRepository: lnp2pBot/bot
Length of output: 38
Fix spacing and establish log rotation and privacy policy for PII logging.
-
Spacing: Line 55 has a formatting issue—missing space after
?in the ternary operator (violates Airbnb style guide). -
PII logging & data retention: The middleware logs Telegram usernames and user IDs (also at line 61-62 for callback queries). The log file is configured to 5GB with no rotation policy or deletion mechanism. Once the limit is reached, logging stops but the file persists indefinitely with unrotated PII data. No privacy policy documentation addresses this logging practice. You need to:
- Implement log rotation (e.g.,
maxFiles,maxDays, ortailablein Winston config) - Document PII logging practices and retention policy
- Ensure compliance with your privacy commitments
- Implement log rotation (e.g.,
🔎 Fix for spacing issue
-logger.info(`User @${userName} [${userId}] ${isCommand? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`);
+logger.info(`User @${userName} [${userId}] ${isCommand ? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`);📝 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.
| const userName = msg.from?.username ?? ''; | |
| logger.info(`User @${userName} [${userId}] ${isCommand? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`); | |
| const userName = msg.from?.username ?? ''; | |
| logger.info(`User @${userName} [${userId}] ${isCommand ? 'executed command:' : 'sent message:'} ${command} with args: [${args.join(', ')}]`); |
🤖 Prompt for AI Agents
In bot/middleware/commandlogging.ts around lines 53 to 55, fix the missing space
after the ternary operator so the log message matches style guides, and address
PII logging/retention: update the application's Winston (or chosen logger)
configuration to enable log rotation and retention (e.g., set maxFiles or
maxDays, enable tailable or rotation options, and ensure archived logs are
purged), avoid long-term plaintext storage of Telegram PII where possible (mask
or hash usernames/IDs in logs if full values are not required), and add/update
project documentation describing what PII is logged, retention period,
rotation/purge policy, and how this complies with the privacy policy and any
legal requirements.
Added a commandlogging middleware that intercepts all commands sent by users and add them to a log file
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.