refactor(moderation): hybrid pipeline with event-driven enforcement#234
Merged
Conversation
…d adr for hybrid pipeline refactor Establishes domain documentation for the moderation pipeline refactor: - CONTEXT-MAP.md at root with module dependency rules - Per-module CONTEXT.md with glossary, boundaries, and flows - ADR-0001 documenting the hybrid pipeline architecture decision
…ith typed connectors and requests Implements #229. Replaces all ad-hoc Http:: facade usage with Saloon-based typed connectors and request classes for the Discord REST API. - Add saloonphp/saloon dependency - Create DiscordConnector (Bot token auth, timeouts) - Create DiscordOAuthConnector (OAuth2 flows) - Create typed requests: Members (Get/Modify/Remove), Bans (Create), Messages (Create/Delete), Channels (CreateDm), OAuth (ExchangeCode/GetUser) - Extract DiscordRoleResolver (protection tier from member roles) - Migrate DiscordOAuthClient to use Saloon transport - Register connectors as singletons in ServiceProvider - Add unit tests for all requests and resolver (MockClient) - Add feature test for OAuth client migration
…sify-and-route, and platform registry Implements #230. Creates the complete moderation pipeline core that replaces the inline orchestration in MessageReceivedEvent. - Create PlatformRegistry with Platform enum-based O(1) lookup - Create SubmitForModeration action (single entry point, sync rule pre-screen) - Create ScreenContent job (async AI classification, self-contained) - Create ClassifyAndRoute job (enriches rule-matched cases with AI) - Extract RouteCaseAction from RouteDecision (testable in isolation) - Create CaseReadyForEnforcement event (auto-execution policy) - Update ExecuteAction to use PlatformRegistry instead of container tags - Register PlatformRegistry as singleton in ModerationServiceProvider
… find-external-identity tenant parameter Implements #231. Makes ModerationContentDTO queue-serializable by removing the Eloquent User model, and adjusts FindExternalIdentity for non-HTTP contexts. - Remove ?User $author from ModerationContentDTO (only authorExternalId remains) - Add optional ?string $tenantId to FindExternalIdentity::handle() (backward compatible) - Refactor IngestContent to use FindExternalIdentity action instead of raw query - Remove 'author' key from DiscordModerationAdapter ingest payload - Update all tests constructing ModerationContentDTO to remove author parameter - Add DTO serialization tests and FindExternalIdentity tenantId tests
Companion to 1cb2a36 — updates ExecuteAction tests to pass PlatformRegistry via method injection after the ExecuteAction refactor.
…main decisions Adds class-level docblocks and key inline comments to the pipeline classes to make the flow navigable without reading the ADR first.
…auto-execute listener, and embed builder Implements #232. Refactors bot-discord to use event-driven enforcement pattern where the moderation domain decides WHEN to act and the platform decides HOW. - Refactor DiscordModerationAdapter to use DiscordConnector (Saloon) + DiscordRoleResolver - Create AutoExecuteAction listener for CaseReadyForEnforcement event - Extract ModerationEmbedBuilder (pure class, testable without HTTP) - Refactor NotifyModerationChannel to use Saloon transport - Register adapter via PlatformRegistry instead of container tags - Migrate all 30+ adapter tests from Http::fake to Saloon MockClient - Add tests for AutoExecuteAction and ModerationEmbedBuilder
…e integration Implements #233. Strips MessageReceivedEvent down to a thin orchestrator that delegates entirely to SubmitForModeration. - Remove all inline classification, routing, and enforcement logic - Remove 10 unused imports (classifiers, jobs, rules, actions) - Handler now: resolve tenant → track activity → submit to moderation pipeline - Simplify error logging (remove full stack trace)
…rs, and event handler Adds class-level docblocks and key inline comments to bot-discord and enforcement classes explaining the flow, responsibilities, and design rationale.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR introduces a Saloon-based Discord transport layer (connectors, requests, role resolver) and refactors OAuth. It implements a hybrid moderation pipeline (submit, screen, classify-and-route), adds a PlatformRegistry, updates ExecuteAction, and emits CaseReadyForEnforcement. Bot-Discord now delegates to the pipeline, uses the Saloon adapter, adds AutoExecuteAction and an embed builder, and refactors notifications. Identity lookup gains tenant-aware caching. Admin panel gains ExternalIdentity resources. Extensive skills and guidelines docs are added, plus minor composer and config updates. Possibly related issues
Possibly related PRs
Suggested reviewers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app-modules/bot-discord/src/Events/MessageReceivedEvent.php (1)
57-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not drop message attachments before moderation submission.
attachmentsis always sent as an empty array, so attachment-only content is never represented in moderation input.Proposed fix
$content = DiscordModerationAdapter::make()->ingest([ 'message_id' => $message->id, 'author_id' => $message->user_id, 'content' => $message->content, 'channel_id' => $message->channel_id, 'guild_id' => (string) $message->guild_id, 'username' => $message->author->username, - 'attachments' => [], + 'attachments' => collect($message->attachments ?? []) + ->map(fn ($attachment) => $attachment->url ?? null) + ->filter() + ->values() + ->all(), 'tenant_id' => (string) $tenantProvider->tenant_id, ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/bot-discord/src/Events/MessageReceivedEvent.php` around lines 57 - 65, The moderation payload currently forces 'attachments' => [] in the DiscordModerationAdapter::make()->ingest call, which discards any attachment-only messages; update the payload construction in MessageReceivedEvent (around the $content assignment) to include $message->attachments (map them to a simple array of URLs/filenames/metadata suitable for moderation) instead of always sending an empty array, and ensure the adapter receives attachments even when message->content is empty so attachment-only messages are represented.app-modules/bot-discord/src/Moderation/DiscordModerationAdapter.php (1)
118-144:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun post-actions only after successful platform execution.
DM notification and message deletion currently run before checking whether the Discord action failed. If kick/ban/mute fails, users can still receive “action applied” DM and side effects may run inconsistently.
Proposed fix
$response = $this->executeAction($action, $guildId, $discordId); + if ($response instanceof Response && $response->failed()) { + return ExecutionResultDTO::failure( + Platform::Discord, + $response->body(), + ); + } + // Best-effort: DM notification and content deletion are non-fatal. try { $this->sendDmNotification($discordId, $action); } catch (Throwable) { } if ($this->shouldDeleteContent($action->action_type)) { try { $this->deleteOriginalMessage($action); } catch (Throwable) { } } if (!$response instanceof Response) { return ExecutionResultDTO::success(Platform::Discord, [ 'action' => $action->action_type->value, ]); } - - if ($response->failed()) { - return ExecutionResultDTO::failure( - Platform::Discord, - $response->body(), - ); - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/bot-discord/src/Moderation/DiscordModerationAdapter.php` around lines 118 - 144, The post-action DM and content-deletion should only run after confirming the platform action succeeded: call executeAction($action, $guildId, $discordId) as now, then check the result—if $response is an instance of Response and $response->failed() return ExecutionResultDTO::failure(Platform::Discord, $response->body()); if $response is not a Response treat it as success. Only after determining success, invoke sendDmNotification($discordId, $action) and, when shouldDeleteContent($action->action_type) is true, call deleteOriginalMessage($action), each wrapped in try/catch to keep them best-effort; finally return ExecutionResultDTO::success(Platform::Discord, ['action' => $action->action_type->value]).app-modules/moderation/src/Enforcement/ExecuteAction.php (1)
40-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent false “resolved” state when no platform execution happened.
At Line 40, unsupported platforms are silently skipped, but at Lines 55-58 the case is always marked resolved. This can resolve actions with zero actual executions (e.g., stale/invalid
target_platforms), which hides enforcement failures.Track whether at least one execution ran and only resolve the case then; otherwise fail the job (or persist an explicit failed execution result) so it stays observable/retriable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/moderation/src/Enforcement/ExecuteAction.php` around lines 40 - 58, The loop in ExecuteAction.php currently skips unsupported platforms and unconditionally marks the case resolved; update it to track whether any execution actually ran (e.g., set a flag or count when $registry->resolve($platform)->execute(...) is invoked and push a failed ExecutionResultDTO when no platforms were executed), then only call $this->action->case->update(['status' => CaseStatus::Resolved, 'resolved_at' => now()]) if at least one execution occurred; otherwise persist an explicit failed execution result or throw/return a job failure so the action remains observable/retriable. Ensure you adjust the $this->action->update([... 'execution_results' ...]) logic to reflect the new failed-entry behavior and reference $registry, $platform, $results, $this->action->update and $this->action->case->update when making changes.
🟡 Minor comments (13)
app-modules/integration-discord/src/Transport/Requests/Bans/CreateBan.php-18-22 (1)
18-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate ban message-deletion seconds at construction time.
On Line 21, negative values are currently accepted, which can generate invalid request payloads and avoidable API failures. Add a guard before storing the value.
✅ Proposed fix
final class CreateBan extends Request implements HasBody { use HasJsonBody; @@ - public function __construct( - private readonly string $guildId, - private readonly string $userId, - private readonly int $deleteMessageSeconds = 0, - ) {} + private readonly int $deleteMessageSeconds; + + public function __construct( + private readonly string $guildId, + private readonly string $userId, + int $deleteMessageSeconds = 0, + ) { + if ($deleteMessageSeconds < 0) { + throw new \InvalidArgumentException('deleteMessageSeconds must be >= 0.'); + } + + $this->deleteMessageSeconds = $deleteMessageSeconds; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/integration-discord/src/Transport/Requests/Bans/CreateBan.php` around lines 18 - 22, The constructor of CreateBan accepts a negative $deleteMessageSeconds which leads to invalid payloads; add a guard in the __construct of CreateBan that validates $deleteMessageSeconds (the incoming parameter) and rejects negative values (e.g., throw an \InvalidArgumentException with a clear message) before assigning to the private readonly property so only non-negative seconds are stored.app-modules/integration-discord/CONTEXT.md-17-37 (1)
17-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language identifier to the fenced code block.
The structure block starts without a fence language, which triggers markdownlint
MD040.Proposed fix
-``` +```text src/ ├── Transport/ │ ├── DiscordConnector.php ← Bot token auth, base URL, timeout │ ├── DiscordOAuthConnector.php ← OAuth client credentials │ ├── DiscordRoleResolver.php ← Resolves member protection tier │ ├── Requests/ │ │ ├── Members/ ← GetMember, ModifyMember, RemoveMember │ │ ├── Bans/ ← CreateBan │ │ ├── Messages/ ← CreateMessage, DeleteMessage │ │ ├── Channels/ ← CreateDmChannel │ │ └── OAuth/ ← ExchangeCodeForToken, GetCurrentUser │ └── DTOs/ │ └── DiscordMemberDTO.php ├── OAuth/ │ ├── DiscordOAuthClient.php ← Implements OAuthClientContract (uses Transport) │ ├── DiscordOAuthAccessDTO.php │ └── DiscordOAuthUser.php └── ETL/ ← Historical import (existing, unchanged)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@app-modules/integration-discord/CONTEXT.mdaround lines 17 - 37, The fenced
code block in CONTEXT.md (the directory tree listing that includes
DiscordConnector.php, DiscordOAuthConnector.php, DiscordRoleResolver.php,
DiscordOAuthClient.php, etc.) lacks a language identifier which triggers
markdownlint MD040; add a language tag (e.g., "text") immediately after the
opening triple backticks so the block becomes ```text and re-run linting to
verify the MD040 warning is resolved.</details> </blockquote></details> <details> <summary>app-modules/panel-admin/src/Filament/Resources/ExternalIdentities/Schemas/ExternalIdentityInfolist.php-25-27 (1)</summary><blockquote> `25-27`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix misleading label for `user.name`.** This field shows a user name but is labeled "Model Id", which is confusing in the admin UI. Rename it to something like "Owner" or "User". <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/src/Filament/Resources/ExternalIdentities/Schemas/ExternalIdentityInfolist.php` around lines 25 - 27, The TextEntry field TextEntry::make('user.name') currently uses an incorrect label('Model Id'); update its label to a clear user-facing name such as label('Owner') or label('User') so the UI correctly reflects that this displays the related user's name; locate the TextEntry::make('user.name') call in ExternalIdentityInfolist.php and replace the label argument accordingly. ``` </details> </blockquote></details> <details> <summary>.agents/skills/configure-nightwatch/SKILL.md-38-52 (1)</summary><blockquote> `38-52`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add a language tag to the ASCII flow fenced block.** Line 38 opens a fenced block without a language, which triggers MD040 and can fail markdown lint checks. <details> <summary>Suggested fix</summary> ```diff -``` +```text Request/Command/Scheduled Task | v [Sampling?] ----NO----> Drop entire trace | YES v Events generated | v [Filtering?] ----YES---> Drop specific event | NO v [Redaction] ----------> Store modified data ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/configure-nightwatch/SKILL.md around lines 38 - 52, The
fenced ASCII flow block in SKILL.md is missing a language tag (triggering
MD040); update the triple-backtick fence that opens the block (the ASCII diagram
starting "Request/Command/Scheduled Task ... [Redaction]") to include a language
identifier such astext (orflow) so the block becomes ```text and re-run
linting.</details> </blockquote></details> <details> <summary>.agents/skills/modular/SKILL.md-15-34 (1)</summary><blockquote> `15-34`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Specify a language for the module-structure fenced block.** Line 15 starts a fenced block without a language, which triggers MD040 in markdownlint. <details> <summary>Suggested fix</summary> ```diff -``` +```text app-modules/ {module-name}/ composer.json # PSR-4 autoload, Laravel provider discovery src/ Providers/ Models/ Http/ tests/ Feature/ Unit/ routes/ {module-name}-routes.php resources/ database/ migrations/ factories/ seeders/ ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/modular/SKILL.md around lines 15 - 34, Change the unlabeled
fenced code block that documents the module directory structure (the
triple-backtick block showing "app-modules/ {module-name}/ ...") to specify a
language to satisfy MD040; update the opening fence to include a language token
such as "text" (e.g., changetotext) so the markdown linter no longer
flags the block.</details> </blockquote></details> <details> <summary>.agents/skills/laravel-best-practices/rules/caching.md-31-35 (1)</summary><blockquote> `31-35`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add version qualifier for `Cache::memo()` method.** This feature requires Laravel 12.9.0 or higher. Add a note specifying the minimum version requirement to prevent compatibility issues with older Laravel versions. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/laravel-best-practices/rules/caching.md around lines 31 - 35, Add a version qualifier noting that Cache::memo() requires Laravel 12.9.0+; update the guidance for Cache::memo() (the line referencing Cache::memo()->get('settings')) to include a short note like "Requires Laravel 12.9.0 or later" so readers know the minimum framework version needed to use Cache::memo(). ``` </details> </blockquote></details> <details> <summary>.agents/skills/laravel-best-practices/rules/collections.md-38-45 (1)</summary><blockquote> `38-45`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Update documentation with correct Laravel version for `#[CollectedBy]` attribute.** The `#[CollectedBy]` attribute is a Laravel 11+ feature (introduced in Laravel 11, not 10.17+). Add version qualifiers if the project targets earlier Laravel versions. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/laravel-best-practices/rules/collections.md around lines 38 - 45, The docs incorrectly state the Laravel version for the #[CollectedBy] attribute; update the rule text around `#[CollectedBy]`, `UserCollection`, and the `newCollection()` mention to say that `#[CollectedBy]` is available in Laravel 11+ (not 10.17+), and add a short note advising projects on older Laravel versions to continue using the `newCollection()` override as the compatible alternative; ensure the version qualifier is added to the example and the explanatory sentence. ``` </details> </blockquote></details> <details> <summary>.agents/skills/laravel-best-practices/rules/caching.md-23-29 (1)</summary><blockquote> `23-29`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add version requirement note to `Cache::flexible()` example.** `Cache::flexible()` was introduced in Laravel 11, but the documentation doesn't explicitly state this version dependency. Add a note clarifying the minimum Laravel version requirement for this feature. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/laravel-best-practices/rules/caching.md around lines 23 - 29, Add a short version requirement note next to the Cache::flexible() example clarifying that Cache::flexible() was introduced in Laravel 11 and requires Laravel >= 11; update the markdown around the example (the section containing Cache::flexible('users', [300, 600], fn () => User::all())) to include a one-line note such as "Note: introduced in Laravel 11 — requires Laravel 11 or newer" so readers know the minimum framework version dependency. ``` </details> </blockquote></details> <details> <summary>.agents/skills/laravel-best-practices/rules/architecture.md-170-180 (1)</summary><blockquote> `170-180`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add version qualifier for `Concurrency::run()` method.** The `Concurrency::run()` method is only available in Laravel 11+. Add a note indicating the minimum required version, either as "(Laravel 11+)" inline with the section title or as a callout before the code example. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/laravel-best-practices/rules/architecture.md around lines 170 - 180, Add a Laravel version qualifier for Concurrency::run(): update the section title or add a short callout before the example to indicate this API requires Laravel 11+ (e.g., append "(Laravel 11+)" to the heading "Use `Concurrency::run()` for Parallel Execution" or add a one-line note above the code block), and ensure the mention references the Concurrency::run() method so readers know the minimum framework version needed. ``` </details> </blockquote></details> <details> <summary>.ai/guidelines/02-issue-tracker.blade.php-1-16 (1)</summary><blockquote> `1-16`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Markdown structure is flattened, reducing readability and parser reliability.** Lines 1–16 combine headings, lists, and command blocks into wrapped prose. Please split into proper Markdown blocks (`##`, bullet lists, fenced commands) so the guidance is consistently rendered and machine-parseable. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ai/guidelines/02-issue-tracker.blade.php around lines 1 - 16, The Markdown in the "Issue tracker: GitHub Issues..." section is flattened; reformat it into proper Markdown blocks by splitting the top-line heading into a level-2 header (## Issue tracker), turning the conventions list into a bullet list, and placing each CLI example (gh issue create, gh issue view, gh issue list, gh issue comment, gh issue edit, gh issue close, and the note about inferring the repo) into fenced code blocks or single-line code spans so commands are preserved and machine-parseable; ensure subsections like "Conventions" and "When a skill says..." are separate headers and each command example uses explicit flags and suggested heredoc usage where applicable. ``` </details> </blockquote></details> <details> <summary>.agents/skills/laravel-best-practices/rules/testing.md-3-6 (1)</summary><blockquote> `3-6`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Correct the description of `LazilyRefreshDatabase` behavior—it defers migrations until database access, not based on schema freshness.** The current description suggests `LazilyRefreshDatabase` skips the first migration if the schema is already up to date. This is inaccurate. `LazilyRefreshDatabase` actually defers all migrations until a test actively accesses the database; it doesn't perform schema freshness detection. The trait hooks into the database connection to defer the refresh operation, not to conditionally skip it based on schema state. Adjust the wording to clarify this distinction. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/laravel-best-practices/rules/testing.md around lines 3 - 6, Update the description to accurately state that the LazilyRefreshDatabase trait defers running migrations until a test actually accesses the database rather than skipping the first migration based on schema freshness; replace the sentence that claims it "skips the first migration if the schema is already up to date" with wording that LazilyRefreshDatabase hooks into the database connection and postpones the refresh operation until the first DB interaction, and contrast this behavior with RefreshDatabase which runs migrations up front. ``` </details> </blockquote></details> <details> <summary>app-modules/moderation/CONTEXT.md-24-71 (1)</summary><blockquote> `24-71`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix diagram encoding artifacts and add a fenced-block language.** Line 24 opens a fenced block without a language, and lines like 42/56/64 contain corrupted characters (`���`) in the diagram. This hurts readability and can fail markdown lint checks. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@app-modules/moderation/CONTEXT.mdaround lines 24 - 71, The fenced diagram
block in CONTEXT.md lacks a language tag and contains corrupted characters (���)
around "Create Case", "ClassifyAndRoute" and "CaseReadyForEnforcement"; update
the opening fence to include a language (e.g.,text ordiagram) and
replace each corrupted sequence with the intended symbols (e.g., arrows "→" or
box-drawing characters) so the diagram renders cleanly and passes linting;
ensure the fence is properly closed and verify labels like SubmitForModeration,
ScreenContent, ClassifyAndRoute, CaseReadyForEnforcement, AutoExecuteAction,
ModerationAction, and ExecuteAction remain accurate after the fix.</details> </blockquote></details> <details> <summary>app-modules/identity/tests/Unit/ExternalIdentity/FindExternalIdentityTest.php-64-89 (1)</summary><blockquote> `64-89`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Update fallback cache-key expectation to remain tenant-scoped.** This test currently enforces a tenant-less key when `tenantId` is null, which locks in cross-tenant cache collisions. It should assert a key that includes the resolved request tenant. <details> <summary>Proposed fix</summary> ```diff - $expectedKey = sprintf('provider-%s-%s', IdentityProvider::Discord->value, 'discord-no-tenant'); + $expectedKey = sprintf( + 'provider-%s-%s-%s', + IdentityProvider::Discord->value, + 'discord-no-tenant', + $tenant->id + ); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/identity/tests/Unit/ExternalIdentity/FindExternalIdentityTest.php` around lines 64 - 89, The test currently expects a tenant-less cache key but should assert the tenant-scoped key; update the expectedKey construction in the test (the variable named expectedKey) to include the resolved tenant id (use the $tenant->id or request()->get('tenant_id')) so it matches how FindExternalIdentity caches entries (e.g. prefix the key with the tenant id like 'tenant-{tenantId}-provider-{provider}-{providerId}' using IdentityProvider::Discord->value and 'discord-no-tenant'), then assert Cache::has($expectedKey) as before. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (7)</summary><blockquote> <details> <summary>app-modules/integration-discord/tests/Unit/Transport/DiscordRoleResolverTest.php (2)</summary><blockquote> `16-87`: _⚡ Quick win_ **Consider extracting connector setup to reduce repetition.** Each test case creates a new `DiscordConnector` and `DiscordRoleResolver` with identical setup. Consider extracting a helper function to create a resolver with a given mock client. <details> <summary>♻️ Proposed helper function</summary> ```diff +function makeResolver(MockClient $mockClient): DiscordRoleResolver +{ + $connector = new DiscordConnector('test-token'); + $connector->withMockClient($mockClient); + return new DiscordRoleResolver($connector); +} + it('returns admin when member has an admin role', function (): void { $mockClient = new MockClient([ GetMember::class => MockResponse::make([ 'roles' => ['some-role', 'admin-role-1', 'other-role'], ]), ]); - - $connector = new DiscordConnector('test-token'); - $connector->withMockClient($mockClient); - - $resolver = new DiscordRoleResolver($connector); + $resolver = makeResolver($mockClient); expect($resolver->resolveProtectionTier('guild-123', 'user-456'))->toBe('admin'); }); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/integration-discord/tests/Unit/Transport/DiscordRoleResolverTest.php` around lines 16 - 87, Extract a small test helper that builds a DiscordRoleResolver pre-wired with a MockClient to avoid repeating the same setup in every spec: create a function (e.g. makeResolverWithMockClient or createResolver) that accepts a MockClient (or the GetMember response payload) and instantiates DiscordConnector('test-token'), calls withMockClient($mockClient) and returns new DiscordRoleResolver($connector); then replace the repeated blocks in each test with a single call to that helper and pass the appropriate MockClient/response for GetMember. ``` </details> --- `11-87`: _⚡ Quick win_ **Consider adding edge case coverage for robustness.** The tests cover happy paths and basic failure (404), but additional edge cases would improve resilience verification: - Malformed API response (e.g., `roles` is not an array or missing) - Empty `roles` array explicitly tested - Other HTTP error codes (401 unauthorized, 403 forbidden, 500 server error) - Exception handling (network errors, timeouts) <details> <summary>🧪 Example edge case tests</summary> ```php it('returns null when roles field is missing from response', function (): void { $mockClient = new MockClient([ GetMember::class => MockResponse::make(['user' => ['id' => 'user-456']]), ]); $resolver = makeResolver($mockClient); expect($resolver->resolveProtectionTier('guild-123', 'user-456'))->toBeNull(); }); it('returns null when roles is not an array', function (): void { $mockClient = new MockClient([ GetMember::class => MockResponse::make(['roles' => 'invalid']), ]); $resolver = makeResolver($mockClient); expect($resolver->resolveProtectionTier('guild-123', 'user-456'))->toBeNull(); }); it('returns null for forbidden response', function (): void { $mockClient = new MockClient([ GetMember::class => MockResponse::make(['message' => 'Missing Permissions'], 403), ]); $resolver = makeResolver($mockClient); expect($resolver->resolveProtectionTier('guild-123', 'user-456'))->toBeNull(); }); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/integration-discord/tests/Unit/Transport/DiscordRoleResolverTest.php` around lines 11 - 87, Tests for DiscordRoleResolver currently exercise happy paths and a 404 but lack edge-case coverage; add unit tests that assert resolveProtectionTier returns null for (1) responses missing the roles field, (2) responses where roles is not an array (e.g., string), (3) an explicit empty roles array, and (4) other HTTP errors like 401/403/500 and simulated network exceptions. Use the same pattern as existing tests: create a MockClient with GetMember::class => MockResponse::make(..., status), attach it to a DiscordConnector via withMockClient, construct DiscordRoleResolver, and assert resolveProtectionTier('guild-123','user-456') yields null; consider extracting a makeResolver helper to reduce duplication and reference the DiscordRoleResolver::resolveProtectionTier and DiscordConnector::withMockClient symbols when adding these cases. ``` </details> </blockquote></details> <details> <summary>app-modules/bot-discord/tests/Feature/Moderation/DiscordModerationAdapterTest.php (2)</summary><blockquote> `362-374`: _⚡ Quick win_ **Assert no outbound calls for configuration precondition failures.** Both failure-path tests should verify early exit semantics (`assertNothingSent`) so they fail if Discord calls are attempted with invalid config. <details> <summary>Proposed assertions update</summary> ```diff test('returns failure when guild id is not configured', function (): void { $mockClient = mockConnector([]); @@ $result = DiscordModerationAdapter::make()->execute($action, $user); expect($result->success)->toBeFalse() ->and($result->error)->toContain('not configured'); + $mockClient->assertNothingSent(); }); test('returns failure when bot token is not configured', function (): void { @@ - $mockClient = mockConnector([ - GetMember::class => MockResponse::make([], 500), - ]); + $mockClient = mockConnector([]); @@ - // Since guild_id is still set, the adapter will try to call the role resolver - // which will fail. The adapter catches the Throwable and returns failure. $result = DiscordModerationAdapter::make()->execute($action, $user); - expect($result->success)->toBeFalse(); + expect($result->success)->toBeFalse(); + $mockClient->assertNothingSent(); }); ``` </details> Also applies to: 376-395 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/bot-discord/tests/Feature/Moderation/DiscordModerationAdapterTest.php` around lines 362 - 374, The failure-path tests for configuration preconditions (the test that sets config('he4rt.discord.guild_id','') and its sibling) must also assert no outbound Discord calls were made; after creating the mockConnector([]) and before/after calling DiscordModerationAdapter::make()->execute($action, $user) add an assertion to verify the mock HTTP client sent nothing (use assertNothingSent or the test helper that checks no requests were dispatched) so the tests fail if the adapter attempts any external calls when config is invalid; update both tests that cover missing guild id and the other config precondition to include this assertNothingSent check referencing the same mockConnector/mock client used in each test. ``` </details> --- `94-110`: _⚡ Quick win_ **Stabilize timeout-duration assertions by freezing time.** These assertions depend on wall-clock time, which can make them flaky at day boundaries. Freeze test time once and assert against that fixed baseline. <details> <summary>Proposed test hardening</summary> ```diff +use Illuminate\Support\Facades\Date; use Illuminate\Foundation\Testing\RefreshDatabase; use Saloon\Http\Faking\MockClient; use Saloon\Http\Faking\MockResponse; @@ beforeEach(function (): void { config()->set('he4rt.discord.guild_id', '123456789'); config()->set('discord.token', 'bot-token'); + Date::setTestNow('2026-01-15 12:00:00'); }); + +afterEach(function (): void { + Date::setTestNow(); +}); @@ - $mockClient->assertSent(fn ($request) => $request instanceof ModifyMember - && str_contains($request->body()->all()['communication_disabled_until'] ?? '', now()->addDays(7)->format('Y-m-d'))); + $mockClient->assertSent(fn ($request) => $request instanceof ModifyMember + && str_contains($request->body()->all()['communication_disabled_until'] ?? '', Date::now()->addDays(7)->format('Y-m-d'))); @@ - $mockClient->assertSent(fn ($request) => $request instanceof ModifyMember - && str_contains($request->body()->all()['communication_disabled_until'] ?? '', now()->addDays(28)->format('Y-m-d'))); + $mockClient->assertSent(fn ($request) => $request instanceof ModifyMember + && str_contains($request->body()->all()['communication_disabled_until'] ?? '', Date::now()->addDays(28)->format('Y-m-d'))); ``` </details> Also applies to: 112-130 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/bot-discord/tests/Feature/Moderation/DiscordModerationAdapterTest.php` around lines 94 - 110, Freeze the test clock at the start of the test and assert against that frozen instant: call Carbon::setTestNow($frozen = now()) (or equivalent) before creating $action and executing DiscordModerationAdapter::make()->execute($action, $user), then use $frozen->addDays(7)->format('Y-m-d') in your ModifyMember request assertion (and clear the test now after the test). Apply the same change to the other test block referenced (lines 112-130) so assertions no longer depend on wall-clock boundaries. ``` </details> </blockquote></details> <details> <summary>.agents/skills/tailwindcss-development/SKILL.md (1)</summary><blockquote> `28-29`: _⚡ Quick win_ **Soften absolute guidance on `tailwind.config.js` usage in v4.** The current wording presents configuration as binary. While Tailwind v4 emphasizes CSS-first configuration with `@theme`, the official documentation confirms that `tailwind.config.js` is still usable via the `@config` directive for backward compatibility and plugin scenarios. Revise to acknowledge both approaches: <details> <summary>Suggested change</summary> ```diff -In Tailwind v4, configuration is CSS-first using the `@theme` directive — no separate `tailwind.config.js` file is needed: +In Tailwind v4, prefer CSS-first configuration using the `@theme` directive. Use a `tailwind.config.js` file only when backward compatibility or a plugin scenario requires it: ``` </details> Also applies to: 121-122 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/tailwindcss-development/SKILL.md around lines 28 - 29, Change the absolute statement that Tailwind v4 requires CSS-first `@theme` and removes `tailwind.config.js` to a softer note that `@theme` is the preferred CSS-first approach but `tailwind.config.js` remains supported via the `@config` directive for backward compatibility and plugin use; update the sentence containing "In Tailwind v4, configuration is CSS-first using the `@theme` directive — no separate `tailwind.config.js` file is needed:" to acknowledge both approaches, and apply the same wording adjustment to the other occurrence that references Tailwind v4 configuration (the lines mentioning `@theme` and `tailwind.config.js`). ``` </details> </blockquote></details> <details> <summary>app-modules/moderation/src/Cases/Events/CaseReadyForEnforcement.php (1)</summary><blockquote> `8-21`: _⚡ Quick win_ **Use `SerializesModels` for model-carrying event payloads.** This event transports an Eloquent model (Line 20). Add `SerializesModels` to avoid serializing full model snapshots when listeners are queued. <details> <summary>Proposed change</summary> ```diff use He4rt\Moderation\Cases\Models\ModerationCase; use Illuminate\Foundation\Events\Dispatchable; +use Illuminate\Queue\SerializesModels; ... final readonly class CaseReadyForEnforcement { use Dispatchable; + use SerializesModels; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/moderation/src/Cases/Events/CaseReadyForEnforcement.php` around lines 8 - 21, The event class CaseReadyForEnforcement currently uses Dispatchable but transports an Eloquent model directly, so add the SerializesModels trait to avoid serializing full model snapshots when listeners are queued; import Illuminate\Queue\SerializesModels and include the trait alongside Dispatchable inside the final readonly class CaseReadyForEnforcement (so the class uses Dispatchable and SerializesModels) ensuring the public ModerationCase $case constructor remains unchanged. ``` </details> </blockquote></details> <details> <summary>app-modules/moderation/src/Platform/PlatformRegistry.php (1)</summary><blockquote> `23-37`: _⚡ Quick win_ **Validate adapter class contract at registration time.** `register()` currently accepts any class-string, so a bad registration fails only later at runtime. Add an `is_subclass_of(..., ModerationPlatformContract::class)` guard to fail fast. <details> <summary>Proposed change</summary> ```diff public function register(Platform $platform, string $adapterClass): void { + if (! is_subclass_of($adapterClass, ModerationPlatformContract::class)) { + throw new RuntimeException("Adapter [$adapterClass] must implement ModerationPlatformContract."); + } $this->adapters[$platform->value] = $adapterClass; } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/moderation/src/Platform/PlatformRegistry.php` around lines 23 - 37, The register() method in PlatformRegistry currently accepts any class-string and defers contract checks to resolve(); update register(Platform $platform, string $adapterClass) to validate that $adapterClass implements ModerationPlatformContract by using is_subclass_of($adapterClass, ModerationPlatformContract::class, true) (or similar) and throw a RuntimeException (or InvalidArgumentException) with a clear message if the check fails, so bad registrations fail fast; ensure the adapters[$platform->value] assignment only happens after this guard. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In @.agents/skills/laravel-best-practices/rules/scheduling.md:
- Around line 25-27: Replace the incorrect reference to the non-existent
Event::takeUntilTimeout() with concrete, supported alternatives: explain
dispatching the work as a queued job and setting the job timeout on the job
class, show using the shell timeout utility around exec/command (e.g.,
$schedule->exec('timeout 60s php artisan your:command')), and mention
implementing manual elapsed-time checks (microtime) inside command logic; also
clarify that withoutOverlapping() prevents concurrent starts but does not kill
long runs and that you can use ->withoutOverlapping(30) to release the lock
after 30 minutes.In @.agents/skills/laravel-best-practices/rules/security.md:
- Line 97: Update the guidance text so it no longer claims
@csrfis
automatically applied in Inertia apps; explicitly state that@csrfmust be
included in all Blade form elements and that Inertia only applies CSRF
automatically for requests made via its JavaScript API (e.g.,useForm),
whereas traditional HTML forms still require the@csrfdirective to ensure
protection.In
@app-modules/bot-discord/src/Events/MessageReceivedEvent.php:
- Around line 38-41: The tenant lookup using ExternalIdentity::query() is
missing a provider constraint so an ExternalIdentity with the same
external_account_id from a different provider could be returned; update the
query in MessageReceivedEvent to include a where('provider', '')
(or where('provider', ExternalIdentity::PROVIDER_DISCORD) if a constant exists)
alongside the existing where('model_type', (new Tenant)->getMorphClass()) and
where('external_account_id', (string) $message->guild_id) before calling
firstOrFail() so only the Discord provider records are considered.In
@app-modules/bot-discord/src/Listeners/NotifyModerationChannel.php:
- Around line 34-44: Wrap the call to $this->connector->send(new
CreateMessage(...)) in a try/catch so transport exceptions don’t bubble and
break the moderation flow: catch the transport/connection exception (or a broad
\Throwable) around the send in NotifyModerationChannel (the block that uses
CreateMessage and
$this->embedBuilder->buildRoleMentions()/buildCaseEmbed($case)), log the
exception details with context (case id, exception message/stack) and then
continue; keep the existing $response->failed() check for HTTP failures when a
response is returned.In
@app-modules/identity/src/ExternalIdentity/Actions/FindExternalIdentity.php:
- Around line 15-23: The cache key generation for FindExternalIdentity omits
tenant when $tenantId is null, allowing cross-tenant collisions; update the
$cacheKey logic used by Cache::remember so it always encodes the tenant scope
(e.g. use an explicit placeholder like "tenant-null" or serialize the tenantId)
so the key consistently reflects the tenant used by the subsequent call to
$this->find($provider, $providerId, $tenantId); ensure the change touches the
$cacheKey variable creation that feeds Cache::remember so cached entries cannot
be shared across tenants.In
@app-modules/integration-discord/src/Transport/DiscordOAuthConnector.php:
- Around line 18-22: The constructor currently exposes the OAuth secret as
public readonly (clientSecret) on DiscordOAuthConnector; change its visibility
to private readonly and update any call sites to use an explicit accessor method
(e.g., getClientSecret() or a method that performs the specific operation
needing the secret) so the secret is no longer part of the public object state;
modify the constructor signature to accept private readonly string $clientSecret
and add the minimal accessor in the DiscordOAuthConnector class, then update
usages to call that accessor instead of reading the property directly.In
@app-modules/moderation/src/Classification/Jobs/IngestContent.php:
- Around line 34-43: The try/catch around
Resolve(FindExternalIdentity::class)->handle(...) is currently catching
Throwable and swallowing real errors; change it to only tolerate the "identity
not found" case by catching the specific exception thrown when an external
identity is absent (e.g., IdentityNotFoundException or ModelNotFoundException
from FindExternalIdentity::handle) and set $authorId = null only in that branch,
while letting other exceptions (runtime/infrastructure) bubble up or rethrow;
locate the lookup call using FindExternalIdentity::class,
IdentityProvider::Discord->value, $this->content->authorExternalId and
$this->content->tenantId and replace the broad catch(Throwable) with a targeted
catch for the not-found exception (or a conditional that rethrows non-not-found
exceptions).In
@app-modules/moderation/src/Classification/Jobs/ScreenContent.php:
- Around line 115-118: The current ExternalIdentity lookup hardcodes
IdentityProvider::Discord and omits tenant, causing cross-tenant/provider
mis-association when resolving $this->content->authorExternalId to an author_id;
change the query in the resolution logic (the ExternalIdentity::query() call) to
filter by the actual provider and tenant instead of IdentityProvider::Discord —
i.e., include the tenant identifier (e.g., tenant_id) and use the content's
provider field rather than the hardcoded Discord constant, mirroring the
tenant-aware/provider-aware lookup used on the sync entry path so the returned
value('model_id') is scoped correctly.In
@app-modules/moderation/src/Pipeline/SubmitForModeration.php:
- Around line 89-92: The current lookup hardcodes IdentityProvider::Discord and
skips tenant scoping (ExternalIdentity::query() using IdentityProvider::Discord
and $content->authorExternalId), causing cross-tenant and non-Discord
mis-resolution; replace this direct query with the tenant-aware helper (e.g.,
call FindExternalIdentity or equivalent) and derive the provider from
$content->sourcePlatform instead of IdentityProvider::Discord so the resolved
author_id is tenant-scoped and uses the correct platform.In
@app-modules/panel-admin/src/Filament/Resources/ExternalIdentities/RelationManagers/MessagesRelationManager.php:
- Around line 269-287: The code constructs a ModerationContentDTO via
ModerationContentDTO::fromPlatform(Platform::Discord, ...) but then submits the
report with resolve(SubmitReport::class)->handle(..., platform: Platform::Web),
causing an inconsistent platform; update the SubmitReport call to use the same
platform as the DTO (i.e., Platform::Discord) or derive the platform from the
created $contentDTO so both the DTO creation and the
resolve(SubmitReport::class)->handle(...) platform argument match and pass a
single, consistent Platform value.In
@app-modules/panel-admin/src/Filament/Resources/ExternalIdentities/Schemas/ExternalIdentityForm.php:
- Around line 24-33: The form allows mismatched polymorphic pairs because
TextInput::make('model_type') is editable while
Select::make('model_id')->relationship('user','name') forces model_id to a user;
fix by deriving/locking model_type from the chosen relation in
ExternalIdentityForm: either make the relation truly polymorphic (replace
relationship('user','name') with a morphTo/morph relationship so the select
drives both type and id) or keep the current user relation and change model_type
to a hidden/disabled field that is automatically set to the User class (populate
model_type when model_id changes or set a default and mark
TextInput::make('model_type')->disabled()/hidden()); ensure updates happen in
the form lifecycle so saved records always have consistent model_type/model_id
pairs.
Outside diff comments:
In@app-modules/bot-discord/src/Events/MessageReceivedEvent.php:
- Around line 57-65: The moderation payload currently forces 'attachments' => []
in the DiscordModerationAdapter::make()->ingest call, which discards any
attachment-only messages; update the payload construction in
MessageReceivedEvent (around the $content assignment) to include
$message->attachments (map them to a simple array of URLs/filenames/metadata
suitable for moderation) instead of always sending an empty array, and ensure
the adapter receives attachments even when message->content is empty so
attachment-only messages are represented.In
@app-modules/bot-discord/src/Moderation/DiscordModerationAdapter.php:
- Around line 118-144: The post-action DM and content-deletion should only run
after confirming the platform action succeeded: call executeAction($action,
$guildId, $discordId) as now, then check the result—if $response is an instance
of Response and $response->failed() return
ExecutionResultDTO::failure(Platform::Discord, $response->body()); if $response
is not a Response treat it as success. Only after determining success, invoke
sendDmNotification($discordId, $action) and, when
shouldDeleteContent($action->action_type) is true, call
deleteOriginalMessage($action), each wrapped in try/catch to keep them
best-effort; finally return ExecutionResultDTO::success(Platform::Discord,
['action' => $action->action_type->value]).In
@app-modules/moderation/src/Enforcement/ExecuteAction.php:
- Around line 40-58: The loop in ExecuteAction.php currently skips unsupported
platforms and unconditionally marks the case resolved; update it to track
whether any execution actually ran (e.g., set a flag or count when
$registry->resolve($platform)->execute(...) is invoked and push a failed
ExecutionResultDTO when no platforms were executed), then only call
$this->action->case->update(['status' => CaseStatus::Resolved, 'resolved_at' =>
now()]) if at least one execution occurred; otherwise persist an explicit failed
execution result or throw/return a job failure so the action remains
observable/retriable. Ensure you adjust the $this->action->update([...
'execution_results' ...]) logic to reflect the new failed-entry behavior and
reference $registry, $platform, $results, $this->action->update and
$this->action->case->update when making changes.
Minor comments:
In @.agents/skills/configure-nightwatch/SKILL.md:
- Around line 38-52: The fenced ASCII flow block in SKILL.md is missing a
language tag (triggering MD040); update the triple-backtick fence that opens the
block (the ASCII diagram starting "Request/Command/Scheduled Task ...
[Redaction]") to include a language identifier such astext (orflow) so
the block becomes ```text and re-run linting.In @.agents/skills/laravel-best-practices/rules/architecture.md:
- Around line 170-180: Add a Laravel version qualifier for Concurrency::run():
update the section title or add a short callout before the example to indicate
this API requires Laravel 11+ (e.g., append "(Laravel 11+)" to the heading "Use
Concurrency::run()for Parallel Execution" or add a one-line note above the
code block), and ensure the mention references the Concurrency::run() method so
readers know the minimum framework version needed.In @.agents/skills/laravel-best-practices/rules/caching.md:
- Around line 31-35: Add a version qualifier noting that Cache::memo() requires
Laravel 12.9.0+; update the guidance for Cache::memo() (the line referencing
Cache::memo()->get('settings')) to include a short note like "Requires Laravel
12.9.0 or later" so readers know the minimum framework version needed to use
Cache::memo().- Around line 23-29: Add a short version requirement note next to the
Cache::flexible() example clarifying that Cache::flexible() was introduced in
Laravel 11 and requires Laravel >= 11; update the markdown around the example
(the section containing Cache::flexible('users', [300, 600], fn () =>
User::all())) to include a one-line note such as "Note: introduced in Laravel 11
— requires Laravel 11 or newer" so readers know the minimum framework version
dependency.In @.agents/skills/laravel-best-practices/rules/collections.md:
- Around line 38-45: The docs incorrectly state the Laravel version for the
#[CollectedBy] attribute; update the rule text around#[CollectedBy],
UserCollection, and thenewCollection()mention to say that#[CollectedBy]
is available in Laravel 11+ (not 10.17+), and add a short note advising projects
on older Laravel versions to continue using thenewCollection()override as
the compatible alternative; ensure the version qualifier is added to the example
and the explanatory sentence.In @.agents/skills/laravel-best-practices/rules/testing.md:
- Around line 3-6: Update the description to accurately state that the
LazilyRefreshDatabase trait defers running migrations until a test actually
accesses the database rather than skipping the first migration based on schema
freshness; replace the sentence that claims it "skips the first migration if the
schema is already up to date" with wording that LazilyRefreshDatabase hooks into
the database connection and postpones the refresh operation until the first DB
interaction, and contrast this behavior with RefreshDatabase which runs
migrations up front.In @.agents/skills/modular/SKILL.md:
- Around line 15-34: Change the unlabeled fenced code block that documents the
module directory structure (the triple-backtick block showing "app-modules/
{module-name}/ ...") to specify a language to satisfy MD040; update the opening
fence to include a language token such as "text" (e.g., changetotext)
so the markdown linter no longer flags the block.In @.ai/guidelines/02-issue-tracker.blade.php:
- Around line 1-16: The Markdown in the "Issue tracker: GitHub Issues..."
section is flattened; reformat it into proper Markdown blocks by splitting the
top-line heading into a level-2 header (## Issue tracker), turning the
conventions list into a bullet list, and placing each CLI example (gh issue
create, gh issue view, gh issue list, gh issue comment, gh issue edit, gh issue
close, and the note about inferring the repo) into fenced code blocks or
single-line code spans so commands are preserved and machine-parseable; ensure
subsections like "Conventions" and "When a skill says..." are separate headers
and each command example uses explicit flags and suggested heredoc usage where
applicable.In
@app-modules/identity/tests/Unit/ExternalIdentity/FindExternalIdentityTest.php:
- Around line 64-89: The test currently expects a tenant-less cache key but
should assert the tenant-scoped key; update the expectedKey construction in the
test (the variable named expectedKey) to include the resolved tenant id (use the
$tenant->id or request()->get('tenant_id')) so it matches how
FindExternalIdentity caches entries (e.g. prefix the key with the tenant id like
'tenant-{tenantId}-provider-{provider}-{providerId}' using
IdentityProvider::Discord->value and 'discord-no-tenant'), then assert
Cache::has($expectedKey) as before.In
@app-modules/integration-discord/CONTEXT.md:
- Around line 17-37: The fenced code block in CONTEXT.md (the directory tree
listing that includes DiscordConnector.php, DiscordOAuthConnector.php,
DiscordRoleResolver.php, DiscordOAuthClient.php, etc.) lacks a language
identifier which triggers markdownlint MD040; add a language tag (e.g., "text")
immediately after the opening triple backticks so the block becomes ```text and
re-run linting to verify the MD040 warning is resolved.In
@app-modules/integration-discord/src/Transport/Requests/Bans/CreateBan.php:
- Around line 18-22: The constructor of CreateBan accepts a negative
$deleteMessageSeconds which leads to invalid payloads; add a guard in the
__construct of CreateBan that validates $deleteMessageSeconds (the incoming
parameter) and rejects negative values (e.g., throw an \InvalidArgumentException
with a clear message) before assigning to the private readonly property so only
non-negative seconds are stored.In
@app-modules/moderation/CONTEXT.md:
- Around line 24-71: The fenced diagram block in CONTEXT.md lacks a language tag
and contains corrupted characters (���) around "Create Case", "ClassifyAndRoute"
and "CaseReadyForEnforcement"; update the opening fence to include a language
(e.g.,text ordiagram) and replace each corrupted sequence with the
intended symbols (e.g., arrows "→" or box-drawing characters) so the diagram
renders cleanly and passes linting; ensure the fence is properly closed and
verify labels like SubmitForModeration, ScreenContent, ClassifyAndRoute,
CaseReadyForEnforcement, AutoExecuteAction, ModerationAction, and ExecuteAction
remain accurate after the fix.In
@app-modules/panel-admin/src/Filament/Resources/ExternalIdentities/Schemas/ExternalIdentityInfolist.php:
- Around line 25-27: The TextEntry field TextEntry::make('user.name') currently
uses an incorrect label('Model Id'); update its label to a clear user-facing
name such as label('Owner') or label('User') so the UI correctly reflects that
this displays the related user's name; locate the TextEntry::make('user.name')
call in ExternalIdentityInfolist.php and replace the label argument accordingly.
Nitpick comments:
In @.agents/skills/tailwindcss-development/SKILL.md:
- Around line 28-29: Change the absolute statement that Tailwind v4 requires
CSS-first@themeand removestailwind.config.jsto a softer note that
@themeis the preferred CSS-first approach buttailwind.config.jsremains
supported via the@configdirective for backward compatibility and plugin use;
update the sentence containing "In Tailwind v4, configuration is CSS-first using
the@themedirective — no separatetailwind.config.jsfile is needed:" to
acknowledge both approaches, and apply the same wording adjustment to the other
occurrence that references Tailwind v4 configuration (the lines mentioning
@themeandtailwind.config.js).In
@app-modules/bot-discord/tests/Feature/Moderation/DiscordModerationAdapterTest.php:
- Around line 362-374: The failure-path tests for configuration preconditions
(the test that sets config('he4rt.discord.guild_id','') and its sibling) must
also assert no outbound Discord calls were made; after creating the
mockConnector([]) and before/after calling
DiscordModerationAdapter::make()->execute($action, $user) add an assertion to
verify the mock HTTP client sent nothing (use assertNothingSent or the test
helper that checks no requests were dispatched) so the tests fail if the adapter
attempts any external calls when config is invalid; update both tests that cover
missing guild id and the other config precondition to include this
assertNothingSent check referencing the same mockConnector/mock client used in
each test.- Around line 94-110: Freeze the test clock at the start of the test and assert
against that frozen instant: call Carbon::setTestNow($frozen = now()) (or
equivalent) before creating $action and executing
DiscordModerationAdapter::make()->execute($action, $user), then use
$frozen->addDays(7)->format('Y-m-d') in your ModifyMember request assertion (and
clear the test now after the test). Apply the same change to the other test
block referenced (lines 112-130) so assertions no longer depend on wall-clock
boundaries.In
@app-modules/integration-discord/tests/Unit/Transport/DiscordRoleResolverTest.php:
- Around line 16-87: Extract a small test helper that builds a
DiscordRoleResolver pre-wired with a MockClient to avoid repeating the same
setup in every spec: create a function (e.g. makeResolverWithMockClient or
createResolver) that accepts a MockClient (or the GetMember response payload)
and instantiates DiscordConnector('test-token'), calls
withMockClient($mockClient) and returns new DiscordRoleResolver($connector);
then replace the repeated blocks in each test with a single call to that helper
and pass the appropriate MockClient/response for GetMember.- Around line 11-87: Tests for DiscordRoleResolver currently exercise happy
paths and a 404 but lack edge-case coverage; add unit tests that assert
resolveProtectionTier returns null for (1) responses missing the roles field,
(2) responses where roles is not an array (e.g., string), (3) an explicit empty
roles array, and (4) other HTTP errors like 401/403/500 and simulated network
exceptions. Use the same pattern as existing tests: create a MockClient with
GetMember::class => MockResponse::make(..., status), attach it to a
DiscordConnector via withMockClient, construct DiscordRoleResolver, and assert
resolveProtectionTier('guild-123','user-456') yields null; consider extracting a
makeResolver helper to reduce duplication and reference the
DiscordRoleResolver::resolveProtectionTier and DiscordConnector::withMockClient
symbols when adding these cases.In
@app-modules/moderation/src/Cases/Events/CaseReadyForEnforcement.php:
- Around line 8-21: The event class CaseReadyForEnforcement currently uses
Dispatchable but transports an Eloquent model directly, so add the
SerializesModels trait to avoid serializing full model snapshots when listeners
are queued; import Illuminate\Queue\SerializesModels and include the trait
alongside Dispatchable inside the final readonly class CaseReadyForEnforcement
(so the class uses Dispatchable and SerializesModels) ensuring the public
ModerationCase $case constructor remains unchanged.In
@app-modules/moderation/src/Platform/PlatformRegistry.php:
- Around line 23-37: The register() method in PlatformRegistry currently accepts
any class-string and defers contract checks to resolve(); update
register(Platform $platform, string $adapterClass) to validate that
$adapterClass implements ModerationPlatformContract by using
is_subclass_of($adapterClass, ModerationPlatformContract::class, true) (or
similar) and throw a RuntimeException (or InvalidArgumentException) with a clear
message if the check fails, so bad registrations fail fast; ensure the
adapters[$platform->value] assignment only happens after this guard.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
…er and external identity admin resource - Make ClientAccessManager implement Wireable (fromLivewire/toLivewire) - Add tenant() relationship to ExternalIdentity model - Register ExternalIdentityResource in admin panel
gvieira18
previously approved these changes
May 16, 2026
Prettier had collapsed all markdown into single lines. Restores proper heading hierarchy, list formatting, code blocks, and table alignment.
danielhe4rt
dismissed stale reviews from gvieira18 and Clintonrocha98
via
50a1238
Clintonrocha98
approved these changes
May 16, 2026
gvieira18
approved these changes
May 16, 2026
3 tasks
danielhe4rt
added a commit
that referenced
this pull request
May 16, 2026
## Summary - Remove the legacy events module (models, enums, actions, migrations, and pivot tables) - Add a migration to drop all legacy event-related tables (`events`, `events_talks`, `events_attendees`, `sponsors`, `events_sponsors`, `events_agenda`, `event_submission_speakers`) - Clean up references to the old events system that was replaced by the new timeline/social feed ## Context The events module was superseded by the new timeline social feed (#223) and moderation system. The old models (`EventModel`, `EventAgenda`, `EventSegment`, `EventSubmission`, etc.) and their associated enums, actions, and pivot tables are no longer used. ## Test plan - [ ] Run `php artisan migrate` to confirm the drop migration executes cleanly - [ ] Run `php artisan test --compact` to verify no remaining references to removed classes - [ ] Confirm no Filament resources reference the deleted models <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Description This PR removes the legacy events system (models, enums, actions, and related database structures) that was superseded by the new timeline social feed (`#223`) and moderation system. The changes include deletion of 26 files—database migrations, Eloquent models, pivot tables, enums, and action classes—replaced by a single migration that drops all event-related tables in a single transaction. ## References - [`#223` Timeline Social Feed](#223) — feat(timeline): social feed with posts, replies, and moderation events - [`#234` Moderation Pipeline](#234) — refactor(moderation): hybrid pipeline with event-driven enforcement - [`#215` Discord Bot Moderation](#215) — feat(moderation): discord bot moderation system ## Contributor Summary | Contributor | Lines Added | Lines Removed | Files Changed | |---|---|---|---| | danielhe4rt | 20 | 1,271 | 27 | ## Changes Summary | File Path | Change Description | |---|---| | `app-modules/events/database/migrations/2025_11_05_191403_create_events_table.php` | Removed migration creating `events` table | | `app-modules/events/database/migrations/2025_11_05_192008_create_events_talks_table.php` | Removed migration creating `events_talks` table | | `app-modules/events/database/migrations/2025_11_05_192756_create_events_attendees_table.php` | Removed migration creating `events_attendees` pivot table | | `app-modules/events/database/migrations/2025_11_05_193042_create_sponsors_table.php` | Removed migration creating `sponsors` table | | `app-modules/events/database/migrations/2025_11_05_193141_create_events_sponsors_table.php` | Removed migration creating `events_sponsors` pivot table | | `app-modules/events/database/migrations/2025_11_27_132714_add_attend_order_to_events_attendees_table.php` | Removed migration adding `attend_order` column | | `app-modules/events/database/migrations/2025_11_27_145728_create_events_agenda_table.php` | Removed migration creating `events_agenda` table | | `app-modules/events/database/migrations/2025_11_27_153537_remove_starts_at_ends_at_columns_to_events_talks_table.php` | Removed migration removing timestamp columns | | `app-modules/events/database/migrations/2025_11_27_171411_create_event_submission_speakers_table.php` | Removed migration creating `event_submission_speakers` table | | `app-modules/events/database/migrations/2026_05_16_195205_drop_events_module_tables.php` | Added new migration dropping all legacy event tables | | `app-modules/events/src/Actions/AttendEventAction.php` | Removed action class for event attendance | | `app-modules/events/src/Actions/LeaveEventAction.php` | Removed action class for leaving events | | `app-modules/events/src/Enums/AttendingStatusEnum.php` | Removed enum for attendance statuses | | `app-modules/events/src/Enums/EventTypeEnum.php` | Removed enum for event types | | `app-modules/events/src/Enums/SchedulableTypeEnum.php` | Removed enum for schedulable types | | `app-modules/events/src/Enums/SponsoringLevelEnum.php` | Removed enum for sponsoring levels | | `app-modules/events/src/Enums/Talks/TalkStatusEnum.php` | Removed enum for talk statuses | | `app-modules/events/src/Models/EventAgenda.php` | Removed `EventAgenda` model | | `app-modules/events/src/Models/EventModel.php` | Removed main `EventModel` with relationships and operations | | `app-modules/events/src/Models/EventSegment.php` | Removed `EventSegment` model | | `app-modules/events/src/Models/EventSubmission.php` | Removed `EventSubmission` model | | `app-modules/events/src/Models/Pivot/EventAttend.php` | Removed `EventAttend` pivot model | | `app-modules/events/src/Models/Pivot/EventSubmissionSpeaker.php` | Removed `EventSubmissionSpeaker` pivot model | | `app-modules/events/src/Models/Pivot/SponsorAttend.php` | Removed `SponsorAttend` pivot model | | `app-modules/events/src/Models/Sponsor.php` | Removed `Sponsor` model | <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/he4rt/heartdevs.com/pull/236?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/he4rt/heartdevs.com/pull/236?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
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
4 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.
Resumo
Refatoração completa do sistema de moderação para uma arquitetura híbrida com enforcement event-driven. Quebra as god classes, respeita os limites de cada domínio, e prepara o terreno para múltiplas plataformas (Twitch, Telegram, etc).
Antes
MessageReceivedEvent(150+ linhas) fazia tudo inline: classificação, criação de case, roteamento, execuçãobot-discordimportava e instanciava diretamente classifiers, rules, jobs e models do módulomoderationHttp::facade, sem tipagem, retry ou rate limitingDepois
MessageReceivedEventtem 35 linhas: resolve tenant → track activity →SubmitForModerationmoderation(agnóstico de plataforma)CaseReadyForEnforcement→ listener por plataforma)PlatformRegistrycom lookup O(1) por enumArquitetura
Módulos afetados
moderationintegration-discordbot-discordidentitytenantIdopcionalDocumentação criada
CONTEXT-MAP.md— mapa de contextos e regras de dependênciaapp-modules/moderation/CONTEXT.md— glossário, pipeline flow, eventos, boundariesapp-modules/moderation/docs/adr/0001-hybrid-pipeline-with-event-driven-enforcement.md— ADR completoapp-modules/integration-discord/CONTEXT.md— estrutura Transport/Saloonapp-modules/bot-discord/CONTEXT.md— papel do módulo e dependênciasTestes
Http::fake()para SaloonMockClientIssues fechadas
Closes #229
Closes #230
Closes #231
Closes #232
Closes #233