Update 2025.12.1#70
Conversation
|
- Expand AGENTS.md with Cortex mediator, config merge order, and agent guidance - Add .serena/memories and docs/code-wiki/agents.md - Remove .windsurfrules
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Automatic note-finding post-processor was removed without replacement
The old FindNoteRequestHandler (backend/ZiziBot.Application/Handlers/Telegram/Note/FindNoteRequest.cs, deleted) was a IRequestPostProcessor that ran after every Telegram bot request. It automatically detected notes in messages — either via hashtag (#tag) or by matching the full message text against saved notes — and sent the matching note content back to the chat. This was a core bot feature.
In the new pipeline registration at backend/ZiziBot.Application/Infrastructure/Extensions/CortexExtension.cs:24-28, there is no equivalent ITelegramPostProcessPipeline registered for note finding. The feature appears to have been dropped during the migration from MediatR to Cortex.Mediator. Users who relied on hashtag-triggered notes will no longer receive automatic note responses.
Was this helpful? React with 👍 or 👎 to provide feedback.
| try | ||
| { | ||
| await ApplyActionsAsync(botRequest, actions, htmlMessage); | ||
|
|
||
| return PreProcessResult<TResponse>.Stop(); | ||
| } | ||
| catch (Exception exception) | ||
| { | ||
| logger.LogError(exception, "Error on Action Result pipeline"); | ||
| return PreProcessResult<TResponse>.Stop(); | ||
| } |
There was a problem hiding this comment.
🔴 ActionResultPipeline unconditionally short-circuits pipeline, preventing handler execution
The new ActionResultPipeline always returns PreProcessResult<TResponse>.Stop() (lines 27 and 32), which prevents the actual request handler from ever executing when pipeline actions are present. The old ActionResultPipelineBehavior (deleted in this PR) called return await next(cancellationToken) after applying actions, meaning the handler still processed the request. This is a significant behavioral regression: commands from users who trigger enforcement actions (e.g., users without a username receiving Mute+Warn from CheckUsernamePipeline) will now silently be dropped instead of being processed with a warning applied.
Prompt for agents
The ActionResultPipeline.ProcessAsync method always returns PreProcessResult<TResponse>.Stop() in both the try and catch blocks. The old equivalent code (ActionResultPipelineBehavior in backend/ZiziBot.Application/Handlers/Telegram/Middleware/ActionResultPipelineBehavior.cs) called 'return await next(cancellationToken)' after applying enforcement actions, meaning the handler still executed. The new code should return PreProcessResult<TResponse>.Continue in the normal (try) path after applying actions, and only Stop() on exception. This would preserve the original behavior where actions (delete, mute, kick, warn) are applied but the request still proceeds to the handler.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "ShortCircuited" | ||
| ); | ||
|
|
||
| return result.Response!; |
There was a problem hiding this comment.
🔴 PreProcessPipeline returns null when Stop() is called without a response value
At PreProcessPipeline.cs:68, when a pre-processor short-circuits with !result.ShouldContinue, the pipeline returns result.Response!. However, PreProcessResult<TResponse>.Stop() (the parameterless overload at IPreProcessPipeline.cs:30-33) sets Response = default(TResponse), which is null for reference types like BotResponseBase. The ! operator suppresses the compiler warning but does not prevent a null return at runtime. Both ActionResultPipeline (lines 27, 32) and FeatureControlPipeline (line 35) use the parameterless Stop() variant, meaning the pipeline will return null to callers that expect a non-null TResponse.
Prompt for agents
In PreProcessPipeline.cs line 68, 'return result.Response!' can return null because PreProcessResult<TResponse>.Stop() sets Response to default(TResponse). Callers that use the parameterless Stop() (ActionResultPipeline lines 27/32, FeatureControlPipeline line 35) will cause null to be returned as the pipeline result. Fix options: (1) Add a null check and return a default-constructed TResponse (e.g. via Activator.CreateInstance or a 'new()' constraint) when result.Response is null, or (2) require all Stop() calls to provide a valid response instance, removing the parameterless Stop() overload.
Was this helpful? React with 👍 or 👎 to provide feedback.
| { | ||
| logger.LogError(e, "An error occured when executing task {Task}", task); | ||
| } | ||
| }, stoppingToken); | ||
|
|
||
| logger.LogInformation("Task triggered: {Task}", task); | ||
| } | ||
| logger.LogInformation("Task triggered: {Task}", task); | ||
| } | ||
|
|
||
| await Task.Delay(0, stoppingToken); | ||
| await Task.Delay(0, stoppingToken); | ||
|
|
||
| logger.LogInformation("All tasks are triggered. Count: {Count}", services.Count); | ||
| logger.LogInformation("All tasks are triggered. Count: {Count}", services.Count); | ||
| }); |
There was a problem hiding this comment.
📝 Info: TaskRunnerHostedService changed from parallel to sequential task execution
The old TaskRunnerHostedService used Task.Run to fire-and-forget each startup task in parallel. The new implementation at backend/ZiziBot.Application/Services/HostedServices/TaskRunnerHostedService.cs:16-43 executes tasks sequentially within a single ExecuteAsync call via scopedUtil.ExecuteAsync. Each task runs to completion before the next one starts. This changes startup behavior: if any task is slow (e.g., RSS job registration, shalat time registration), it blocks subsequent tasks. The old parallel approach had its own issues (unobserved exceptions), but the sequential approach may significantly increase startup time.
Was this helpful? React with 👍 or 👎 to provide feedback.
| await dataFacade.MongoDb.SaveChangesAsync(cancellationToken); | ||
| } | ||
|
|
||
| var engineConfig = await dataFacade.AppSetting.GetRequiredConfigSectionAsync<EngineConfig>(); | ||
| if (engineConfig.ConsoleUrl.IsNullOrWhiteSpace()) | ||
| await serviceFacade.TelegramService.SendMessageText("Maaf fitur ini belum dipersiapkan"); | ||
|
|
||
| var sessionId = Guid.NewGuid().ToString(); | ||
| var webUrl = engineConfig.ConsoleUrl.SetQueryParam("session_id", sessionId).ToString(); | ||
|
|
||
| var replyMarkup = InlineKeyboardMarkup.Empty(); | ||
| var htmlMessage = HtmlMessage.Empty | ||
| .BoldBr("🎛 ZiziBot Console") | ||
| .TextBr("Buka Console untuk mengelola pengaturan, catatan dan lain-lain.") | ||
| .TextBr($"OTP: {otp}") |
There was a problem hiding this comment.
🚩 PrepareConsoleRequest: OTP generated and displayed regardless of chat type, but only persisted for private chats
The OTP is generated unconditionally on line 21 and displayed in the message on line 49 (TextBr($"OTP: {otp}")), but it is only saved to the database inside the if (request.IsPrivateChat) block (lines 23-36). In group chats, users will see a random OTP number that cannot be redeemed since it was never persisted. The old code only generated the OTP inside the private chat block. This could confuse group chat users who see an OTP they can't use.
(Refers to lines 21-49)
Was this helpful? React with 👍 or 👎 to provide feedback.
…/startup safeguards
| services.AddTransient(typeof(ITelegramPostProcessPipeline<,>), typeof(CheckAfkSessionPipeline<,>)); | ||
| services.AddTransient(typeof(ITelegramPostProcessPipeline<,>), typeof(EnsureChatAdminPipeline<,>)); | ||
| services.AddTransient(typeof(ITelegramPostProcessPipeline<,>), typeof(EnsureChatSettingPipeline<,>)); | ||
| services.AddTransient(typeof(ITelegramPostProcessPipeline<,>), typeof(UpsertBotUserPipeline<,>)); | ||
| services.AddTransient(typeof(ITelegramPostProcessPipeline<,>), typeof(InsertChatActivityPipeline<,>)); |
There was a problem hiding this comment.
🔴 Automatic note detection and response feature completely removed during mediator migration
The automatic note-matching post-processor is deleted (FindNoteRequestHandler at the old backend/ZiziBot.Application/Handlers/Telegram/Note/FindNoteRequest.cs) but no equivalent pipeline is registered in CortexExtension.cs:10-28, so the bot no longer auto-responds to hashtag-triggered notes or text-matching notes.
Impact: Users sending messages containing #note_name or text matching saved notes will no longer receive automatic note responses from the bot.
Migration omission: FindNote post-processor not ported to new pipeline system
In the old MediatR-based pipeline, FindNoteRequestHandler was registered as an IRequestPostProcessor<TRequest, TResponse> via:
.AddOpenRequestPostProcessor(typeof(FindNoteRequestHandler<,>))
in the deleted backend/ZiziBot.Infrastructure/MediatRExtension.cs.
This handler ran after every BotRequestBase handler and would:
- Look up all notes for the chat (
dataFacade.ChatSetting.GetAllByChat) - Match hashtag words (e.g.,
#mytag) against note queries - Match the full message text against note queries (case-insensitive)
- Send matching notes (text or media) via
TelegramService
The new Cortex pipeline in backend/ZiziBot.Application/Extensions/CortexExtension.cs:24-28 registers five ITelegramPostProcessPipeline implementations (CheckAfkSession, EnsureChatAdmin, EnsureChatSetting, UpsertBotUser, InsertChatActivity) but does NOT include any FindNote equivalent. The note CRUD handlers (CreateNoteBotRequest, GetNoteRequest, DeleteNoteRequest) still exist, but the automatic note detection/response pipeline step is entirely missing.
Prompt for agents
The old FindNoteRequestHandler (deleted from backend/ZiziBot.Application/Handlers/Telegram/Note/FindNoteRequest.cs) was a MediatR IRequestPostProcessor that automatically detected notes matching hashtags or message text after every BotRequestBase handler. During migration to Cortex.Mediator, this post-processor was not ported to the new ITelegramPostProcessPipeline system.
To fix: Create a new FindNotePipeline<TRequest, TResponse> class implementing ITelegramPostProcessPipeline<TRequest, TResponse> with the same logic as the deleted FindNoteRequestHandler (checking message text against saved notes, matching hashtags, and sending matching notes). Then register it in CortexExtension.AddApplicationCortexMediator() alongside the other ITelegramPostProcessPipeline registrations, e.g.:
services.AddTransient(typeof(ITelegramPostProcessPipeline<,>), typeof(FindNotePipeline<,>));
The original logic is available in git history at the base commit for reference.
Was this helpful? React with 👍 or 👎 to provide feedback.
| var htmlMessage = HtmlMessage.Empty | ||
| .BoldBr("🎛 ZiziBot Console") | ||
| .TextBr("Buka Console untuk mengelola pengaturan, catatan dan lain-lain.") | ||
| .TextBr($"OTP: {otp}") |
There was a problem hiding this comment.
🔴 One-time password is displayed in group chat messages where it cannot be used
The one-time password is always included in the reply message (TextBr($"OTP: {otp}") at backend/ZiziBot.Application/Features/Handlers/Telegram/Basic/PrepareConsoleRequest.cs:49) regardless of whether the chat is private, so it is visible to all group members even though it is only saved to the database in private chats.
Impact: Users in group chats see a useless OTP that cannot be redeemed, and the code generates an OTP unnecessarily for non-private contexts.
Mechanism: OTP generation moved outside the private-chat guard
In the old code, the OTP was generated inside the if (request.IsPrivateChat) block, so it was never computed or displayed in group chats. In the new code, ValueUtil.GenerateOtp() is called unconditionally at line 21, and the HTML message at line 49 always includes the OTP text. However, the OTP is only persisted to the database inside the if (request.IsPrivateChat) block (lines 23-36), so in group chats the OTP is shown but never stored — making it impossible to redeem.
The OTP line and the database save should both be gated behind the private-chat check, or the OTP should only be appended to the message when request.IsPrivateChat is true.
Was this helpful? React with 👍 or 👎 to provide feedback.
| try | ||
| { | ||
| await ApplyActionsAsync(botRequest, actions, htmlMessage); | ||
|
|
||
| return PreProcessResult<TResponse>.Stop(); | ||
| } | ||
| catch (Exception exception) | ||
| { | ||
| logger.LogError(exception, "Error on Action Result pipeline"); | ||
| return PreProcessResult<TResponse>.Stop(); | ||
| } |
There was a problem hiding this comment.
🚩 ActionResultPipeline now blocks handler execution — behavioral change from old code
The old ActionResultPipelineBehavior (deleted) would apply moderation actions (warn/mute/kick/delete) and then call return await next(cancellationToken), meaning the actual command handler would STILL execute after moderation. The new ActionResultPipeline at backend/ZiziBot.Application/Pipelines/PrePipeline/ActionResultPipeline.cs:27 returns PreProcessResult<TResponse>.Stop(), which completely blocks handler execution.
This means: in the old system, a user who triggered a word filter warning could still have their bot command processed. In the new system, the command is silently dropped. This is a significant behavioral change. If the old pass-through behavior was intentional (e.g., warn but still process the user's request), this change could confuse users whose commands stop working. If the intent was always to block, this is a fix.
Was this helpful? React with 👍 or 👎 to provide feedback.
| catch (Exception ex) | ||
| { | ||
| stopwatch.Stop(); | ||
| logger.LogError( | ||
| ex, | ||
| "PipelineMetrics Stage={Stage} SessionId={SessionId} RequestType={RequestType} ResponseType={ResponseType} Duration={DurationMs} Status={Status} Message={Message}", | ||
| "Request", | ||
| sessionId, | ||
| requestType, | ||
| responseType, | ||
| stopwatch.Elapsed.ToString("mm\\:ss\\.fff"), | ||
| "Failed", | ||
| ex.Message | ||
| ); | ||
| throw; |
There was a problem hiding this comment.
🚩 LoggingPipeline now re-throws exceptions instead of swallowing them
The old LoggingPipelineBehaviour caught all exceptions and returned new TResponse(), effectively swallowing errors silently. The new LoggingPipeline at backend/ZiziBot.Application/Pipelines/PrePipeline/LoggingPipeline.cs:59 re-throws exceptions after logging. This is a significant behavioral change: previously, any handler exception would result in a silent empty response; now exceptions propagate up to the caller and potentially to the global exception handler middleware. This is generally the correct behavior (errors should not be silently swallowed), but callers that previously relied on the swallowing behavior may now see unhandled exceptions.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🚩 Outbox messages are enqueued but no consumer processes them
The new OutboxService (backend/ZiziBot.Application/Services/OutboxService.cs) and OutboxRepository (backend/ZiziBot.Application/Infrastructure/Database/Repository/OutboxRepository.cs) provide infrastructure to enqueue outbox messages, and several handlers now call outboxService.EnqueueAsync(...) (e.g., PostMirrorUserRequest, DeleteMirrorUserRequest, SubmitDonationRequest, WebHookTrakteerDonationRequest). However, there is no background worker, hosted service, or Hangfire job that reads pending outbox messages from the database and processes/dispatches them. The OutboxRepository.GetPendingAsync method exists but is never called. This means all enqueued outbox messages accumulate in MongoDB without being processed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (engineConfig.ConsoleUrl.IsNullOrWhiteSpace()) | ||
| await serviceFacade.TelegramService.SendMessageText("Maaf fitur ini belum dipersiapkan"); |
There was a problem hiding this comment.
🔴 Console setup continues after detecting missing configuration, causing a crash
The handler sends an error message when the console URL is unconfigured (engineConfig.ConsoleUrl.IsNullOrWhiteSpace() at backend/ZiziBot.Application/Features/Handlers/Telegram/Basic/PrepareConsoleRequest.cs:39) but does not return early, so execution falls through to use the null/empty URL on line 43, which will throw or produce a broken link.
Impact: When the console URL is not configured, the bot crashes or sends a broken response instead of gracefully stopping.
Mechanism: missing early return after null-check
At lines 39-40, if ConsoleUrl is null or whitespace, the code sends an error message to the user but does not return. Execution continues to line 43 where engineConfig.ConsoleUrl.SetQueryParam("session_id", sessionId) is called on the null/empty string. Depending on how Flurl handles null inputs, this will either throw a NullReferenceException or produce an invalid URL that gets sent to the user.
The fix is to add return serviceFacade.TelegramService.Complete(); (or similar early return) after line 40.
Prompt for agents
In PrepareConsoleHandler.Handle (PrepareConsoleRequest.cs), after the check at line 39-40 that detects ConsoleUrl is null or whitespace, add an early return so execution does not fall through to line 43 where ConsoleUrl is used. For example, change lines 39-40 to: if (engineConfig.ConsoleUrl.IsNullOrWhiteSpace()) return await serviceFacade.TelegramService.SendMessageText("Maaf fitur ini belum dipersiapkan"); This was also a pre-existing issue in the old code, but the code was reorganized in this PR.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "ShortCircuited" | ||
| ); | ||
|
|
||
| return result.Response!; |
There was a problem hiding this comment.
🚩 Pre-process pipeline short-circuit may return null response
When a pre-processor calls PreProcessResult<TResponse>.Stop() (the parameterless overload at backend/ZiziBot.Application/Infrastructure/Pipelines/PrePipeline/IPreProcessPipeline.cs:30-33), the Response property is set to default(TResponse), which is null for reference types like BotResponseBase. The pipeline then returns result.Response! at backend/ZiziBot.Application/Infrastructure/Pipelines/PrePipeline/PreProcessPipeline.cs:68 with a null-forgiving operator, which suppresses the compiler warning but will propagate null to the caller.
Currently, FeatureControlPipeline at backend/ZiziBot.Application/Infrastructure/Pipelines/PrePipeline/FeatureControlPipeline.cs:30-35 uses the parameterless Stop() when the response type is not assignable to BotResponseBase. If the response type is a reference type, this will return null to the mediator caller, which may cause a NullReferenceException downstream.
Was this helpful? React with 👍 or 👎 to provide feedback.
| { | ||
| var jobId = $"{CronJobKey.Rss_Prefix}:{rss.Id}"; | ||
| var rssUrl = await rss.RssUrl.DetectRss(throwIfError: true); | ||
| var jobId = await StringUtil.GenerateRssKeyAsync(); |
There was a problem hiding this comment.
🚩 RSS job IDs changed from deterministic to random, preventing deduplication
In the old code, RSS job IDs were deterministic: $"{CronJobKey.Rss_Prefix}:{rss.Id}" (based on the MongoDB entity ID). In the new code at backend/ZiziBot.Application/Features/UseCases/Rss/RegisterRssJobAllUseCase.cs:52 and backend/ZiziBot.Application/Features/Handlers/Telegram/Rss/RegisterRssJobAllRequest.cs:43, job IDs are generated via StringUtil.GenerateRssKeyAsync() which produces a random NanoId. This means:
- Each time RSS jobs are re-registered (e.g., on restart via
RssRefresh), old Hangfire recurring jobs are removed byHangfireUtil.RemoveRssJobs()but new ones get completely new IDs. - If
RegisterRssJobAllUseCase.Handleis called concurrently or multiple times without cleanup, duplicate jobs could be created since the IDs are no longer tied to the entity. - The
GenerateRssKeyAsyncalso calls.ToUpper()on the NanoId, which reduces the entropy of the ID.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
| @@ -9,8 +9,8 @@ public class RssSettingEntity : EntityBase | |||
| public int? ThreadId { get; set; } | |||
| public long UserId { get; set; } | |||
| public string RssUrl { get; set; } | |||
| public string? OriginalUrl { get; set; } | |||
| public string CronJobId { get; set; } | |||
| public string? OriginalRssUrl { get; set; } | |||
There was a problem hiding this comment.
🚩 RSS entity field renamed from OriginalUrl to OriginalRssUrl — existing MongoDB documents may have stale field name
The RssSettingEntity renamed OriginalUrl to OriginalRssUrl at backend/ZiziBot.Application/Infrastructure/Database/MongoDb/Entities/RssSettingEntity.cs:12. A migration (RssRefreshOriginalRssUrlMigration at backend/ZiziBot.Application/Infrastructure/Database/MongoDb/Migrations/RssRefreshOriginalRssUrlMigration.cs) populates the new field from RssUrl where OriginalRssUrl is empty. However, since MongoDB stores field names as-is and the EF Core MongoDB provider maps by property name, existing documents with the old OriginalUrl field will not be read into the new OriginalRssUrl property — they'll appear as null/empty. The migration handles this by copying from RssUrl, but this means the original URL value stored in OriginalUrl is lost. The AddRssUseCase at backend/ZiziBot.Application/Features/UseCases/Rss/AddRssUseCase.cs:43 now correctly sets OriginalRssUrl, and RegisterRssJobAllUseCase at line 48 reads from it.
Was this helpful? React with 👍 or 👎 to provide feedback.
| rss.Status = EventStatus.Inactive; | ||
| rss.LastErrorMessage = e.Message; | ||
|
|
||
| logger.LogError("Error registering RSS Job. RssUrl: {RssUrl}, ChatId: {ChatId}, ThreadId: {ThreadId}. Message: {Message}", | ||
| rss.RssUrl, rss.ChatId, rss.ThreadId, e.Message); | ||
| logger.LogWarning("Error registering RSS Job. RssUrl: {RssUrl}, ChatId: {ChatId}, ThreadId: {ThreadId}. Message: {Message}", | ||
| rssUrl, rss.ChatId, rss.ThreadId, e.Message); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
🚩 RegisterRssJobAllUseCase uses ParallelForEachAsync with shared DbContext mutations
At backend/ZiziBot.Application/Features/UseCases/Rss/RegisterRssJobAllUseCase.cs:46-74, ParallelForEachAsync is used to process RSS settings concurrently, and each iteration mutates entity properties (rss.RssUrl, rss.CronJobId, rss.Status, etc.) on entities tracked by a shared MongoDbContext. EF Core's DbContext is not thread-safe — concurrent modifications to tracked entities can cause data corruption or exceptions. The SaveChangesAsync() at line 76 then attempts to save all changes. This was present in the new code but not the old handler version at RegisterRssJobAllRequest.cs:41-54 which uses a sequential foreach loop. The use-case version introduces a concurrency risk.
(Refers to lines 46-76)
Was this helpful? React with 👍 or 👎 to provide feedback.
| var locksToRemove = new List<string>(); | ||
|
|
||
| foreach (var (key, lockSlim) in FileLocks) | ||
| { | ||
| if (!lockSlim.IsReadLockHeld && !lockSlim.IsWriteLockHeld && !lockSlim.IsUpgradeableReadLockHeld) | ||
| locksToRemove.Add(key); | ||
| } | ||
|
|
||
| foreach (var key in locksToRemove) | ||
| { | ||
| if (FileLocks.TryRemove(key, out var lockToDispose)) | ||
| { | ||
| lockToDispose.Dispose(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🚩 FileUtil cleanup can dispose locks that are concurrently acquired by other threads
In FileUtil.CleanupOldLocksIfNeeded() at backend/ZiziBot.Application/Common/Utils/FileUtil.cs:43-55, the cleanup iterates over FileLocks, identifies locks that appear idle (no read/write/upgradeable locks held), removes them, and disposes them. However, between checking IsReadLockHeld/IsWriteLockHeld at line 45 and calling TryRemove + Dispose at lines 51-53, another thread could call GetFileLock → GetOrAdd which returns the existing lock instance before it's removed. That thread would then hold a reference to a lock that gets disposed. Subsequent TryEnterWriteLock/TryEnterReadLock calls on the disposed lock would throw ObjectDisposedException. The window is small but exists under concurrent file operations.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Uh oh!
There was an error while loading. Please reload this page.