feat(integration-twitch): Twitch integration with admin panel, tenant-scoped EventSub, and subscription management#272
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a new Integration Twitch module with Saloon-based connectors and request classes, an OAuth client and app-token caching service, EventSub webhook ingestion with signature verification and idempotent persistence to twitch_event_logs, subscription registration/sync (console commands + admin UI), tenant-channel linking command, Filament admin pages/widgets/resources for event logs and subscriptions, Livewire connection hub updates, configuration additions, migrations/models, tests, and documentation updates. Possibly related issues
Possibly related PRs
Suggested reviewers
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
app-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php (2)
91-109: 💤 Low valueModerator condition test uses same ID for broadcaster and moderator.
Line 104-108 tests
ChannelModeratorAdd->condition('12345')and expectsmoderator_user_idto equal thebroadcaster_user_id. While this correctly validates the enum's default behavior, in realistic scenarios these would be different users. Consider adding a comment explaining this tests the fallback when no moderator ID is provided, or add a second assertion showing explicit moderator ID usage.🤖 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-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php` around lines 91 - 109, The test for ChannelModeratorAdd currently passes the same ID for broadcaster and moderator by calling TwitchEventSubType::ChannelModeratorAdd->condition('12345'); update the test to clarify intent: either add a short inline comment stating this call verifies the fallback behavior when no moderator ID is provided, or add an additional assertion that calls ChannelModeratorAdd->condition('12345', '67890') and expects 'moderator_user_id' => '67890' to demonstrate explicit moderator ID handling; reference the TwitchEventSubType::ChannelModeratorAdd->condition usage to locate where to add the comment or new assertion.
38-60: 💤 Low valueUnused variable
$totalTypessuggests incomplete assertion.Line 51 calculates
$totalTypesbut never uses it in assertions. If the intent is to verify all event types were processed, consider adding an assertion like$mock->assertSentCount(1 + $totalTypes)to validate that oneListSubscriptionsplus oneCreateSubscriptionper type were sent.💡 Strengthen assertion to verify all types processed
test('subscribes to all event types', function (): void { $mock = mockEventSubResponses(); $totalTypes = count(TwitchEventSubType::cases()); $this->artisan('twitch:subscribe', [ 'broadcaster_user_id' => '12345', '--all' => true, ])->assertSuccessful(); $mock->assertSent(ListSubscriptions::class); - $mock->assertSent(CreateSubscription::class); + // 1 ListSubscriptions + 1 CreateSubscription per type + $mock->assertSentCount(1 + $totalTypes); });🤖 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-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php` around lines 38 - 60, The test computes $totalTypes using TwitchEventSubType::cases() but never uses it; update the "subscribes to all event types" test (the test that calls mockEventSubResponses() and runs the artisan 'twitch:subscribe' with '--all') to assert that the expected number of HTTP calls were made—e.g. assert that one ListSubscriptions plus one CreateSubscription per event type were sent by using $mock->assertSentCount(1 + $totalTypes) or alternately $mock->assertSent(CreateSubscription::class, $totalTypes) in addition to $mock->assertSent(ListSubscriptions::class) so the test actually validates all event types were processed.app-modules/integration-twitch/tests/Feature/LinkTwitchChannelCommandTest.php (1)
57-68: ⚡ Quick winTest name implies warning verification but doesn't check output.
The test name "warns when channel is already linked" suggests a warning message should be verified, but the test only checks that no duplicate
ExternalIdentityis created. Consider either renaming the test to better reflect what it validates (e.g., "prevents duplicate channel links") or adding an output assertion if the command actually warns.💡 Option: Add warning output verification
test('warns when channel is already linked', function (): void { mockHelixUsersResponse(); $tenant = Tenant::factory()->create(['slug' => 'he4rt-developers']); $this->artisan('twitch:link-channel', ['login' => 'danielhe4rt', '--tenant' => 'he4rt-developers']) ->assertSuccessful(); $this->artisan('twitch:link-channel', ['login' => 'danielhe4rt', '--tenant' => 'he4rt-developers']) - ->assertSuccessful(); + ->assertSuccessful() + ->expectsOutputToContain('already linked'); expect(ExternalIdentity::query()->where('provider', IdentityProvider::Twitch)->count())->toBe(1); });🤖 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-twitch/tests/Feature/LinkTwitchChannelCommandTest.php` around lines 57 - 68, The test name says it "warns when channel is already linked" but it never asserts any output; update LinkTwitchChannelCommandTest to either rename the test to reflect that it "prevents duplicate channel links" or add an assertion that the command emits the expected warning message when run the second time (chain an output assertion on the second $this->artisan(...) call to check for the warning text produced by the LinkTwitchChannelCommand, e.g., a message containing "already linked" or the exact string the command logs), while keeping the existing ExternalIdentity count assertion.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@app-modules/integration-twitch/CONTEXT.md`:
- Line 19: The fenced code block in CONTEXT.md currently uses plain ``` which
triggers MD040; update the opening fence to include a language identifier (e.g.,
change the opening "```" before the "src/" tree to "```text") so the block
becomes ```text ... ``` while leaving the closing "```" as-is — locate the
triple-backtick surrounding the "src/" tree and add the language tag to the
opening fence.
In
`@app-modules/integration-twitch/database/migrations/2026_05_20_000001_create_twitch_event_logs_table.php`:
- Line 18: The column definition for twitch_message_id in the
CreateTwitchEventLogsTable migration currently allows nulls which breaks
DB-level idempotency; remove the ->nullable() call so the column is defined as
non-nullable (keep ->unique()) in the migration (look for the
$table->string('twitch_message_id')->... line in the
create_twitch_event_logs_table migration). If this migration has already been
run in environments, create a new migration that ALTERs the twitch_event_logs
table: first backfill or reject existing NULL twitch_message_id rows, then ALTER
the twitch_message_id column to NOT NULL and ensure the UNIQUE constraint
remains; update any seeders or code that writes this column to guarantee a value
is provided.
In `@app-modules/integration-twitch/src/Enums/TwitchEventSubType.php`:
- Around line 45-80: Update the condition mappings in the TwitchEventSubType
enum: in condition() keep version() as-is but change cases so channel.follow and
channel.shoutout.create/receive return ['broadcaster_user_id' => $broadcasterId,
'moderator_user_id' => $userId] (use moderator_user_id instead of user_id), make
channel.shield_mode.begin and channel.shield_mode.end return only
['broadcaster_user_id' => $broadcasterId] (remove user_id), and update
channel.moderator.add/channel.moderator.remove to use ['broadcaster_user_id' =>
$broadcasterId, 'moderator_user_id' => $userId] without falling back to
$broadcasterId (validate or throw if $userId is null so a real moderator id is
required); adjust the match arms in condition() for the symbols ChannelFollow,
ChannelShieldModeBegin, ChannelShieldModeEnd, ChannelShoutoutCreate,
ChannelShoutoutReceive, ChannelModeratorAdd, and ChannelModeratorRemove
accordingly.
In
`@app-modules/integration-twitch/src/Http/Controllers/TwitchWebhookController.php`:
- Around line 24-40: The check-then-insert using
TwitchEventLog::query()->where(...)->exists() followed by create() is racy;
change to an atomic insert (e.g. TwitchEventLog::query()->insertOrIgnore([...])
or upsert([...], ['twitch_message_id'], [...])) using the same payload fields
(event_type, broadcaster_user_id, user_id, twitch_message_id, payload) with
messageId, then inspect the returned affected-rows value and treat a 0 (ignored)
result as a duplicate by returning response('', 204); keep the rest of the
handler logic unchanged and remove the separate exists() check to avoid the
unique-key race.
In `@app-modules/integration-twitch/src/Transport/TwitchOAuthConnector.php`:
- Around line 19-21: The $clientSecret property on TwitchOAuthConnector is
public and exposes a sensitive secret; change its declaration from public
readonly string $clientSecret to private readonly string $clientSecret inside
the TwitchOAuthConnector class, update all internal method references in that
class to use $this->clientSecret, and if external code currently reads the
property, replace those call sites with a minimal accessor (e.g., a protected
getClientSecret(): string method) or preferably refactor callers to avoid
needing the raw secret; ensure no public getter is added unless strictly
necessary.
---
Nitpick comments:
In
`@app-modules/integration-twitch/tests/Feature/LinkTwitchChannelCommandTest.php`:
- Around line 57-68: The test name says it "warns when channel is already
linked" but it never asserts any output; update LinkTwitchChannelCommandTest to
either rename the test to reflect that it "prevents duplicate channel links" or
add an assertion that the command emits the expected warning message when run
the second time (chain an output assertion on the second $this->artisan(...)
call to check for the warning text produced by the LinkTwitchChannelCommand,
e.g., a message containing "already linked" or the exact string the command
logs), while keeping the existing ExternalIdentity count assertion.
In
`@app-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php`:
- Around line 91-109: The test for ChannelModeratorAdd currently passes the same
ID for broadcaster and moderator by calling
TwitchEventSubType::ChannelModeratorAdd->condition('12345'); update the test to
clarify intent: either add a short inline comment stating this call verifies the
fallback behavior when no moderator ID is provided, or add an additional
assertion that calls ChannelModeratorAdd->condition('12345', '67890') and
expects 'moderator_user_id' => '67890' to demonstrate explicit moderator ID
handling; reference the TwitchEventSubType::ChannelModeratorAdd->condition usage
to locate where to add the comment or new assertion.
- Around line 38-60: The test computes $totalTypes using
TwitchEventSubType::cases() but never uses it; update the "subscribes to all
event types" test (the test that calls mockEventSubResponses() and runs the
artisan 'twitch:subscribe' with '--all') to assert that the expected number of
HTTP calls were made—e.g. assert that one ListSubscriptions plus one
CreateSubscription per event type were sent by using $mock->assertSentCount(1 +
$totalTypes) or alternately $mock->assertSent(CreateSubscription::class,
$totalTypes) in addition to $mock->assertSent(ListSubscriptions::class) so the
test actually validates all event types were processed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7dfb4da3-aa1e-4318-a8f3-a818a7b78c0e
📒 Files selected for processing (39)
CONTEXT-MAP.mdapp-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.phpapp-modules/integration-discord/CONTEXT.mdapp-modules/integration-twitch/CONTEXT.mdapp-modules/integration-twitch/database/migrations/2026_05_20_000001_create_twitch_event_logs_table.phpapp-modules/integration-twitch/routes/twitch-webhook-routes.phpapp-modules/integration-twitch/src/Client/TwitchBaseClient.phpapp-modules/integration-twitch/src/Console/LinkTwitchChannelCommand.phpapp-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.phpapp-modules/integration-twitch/src/Contracts/TwitchService.phpapp-modules/integration-twitch/src/ETL/Actions/.gitkeepapp-modules/integration-twitch/src/ETL/Console/.gitkeepapp-modules/integration-twitch/src/ETL/DTOs/.gitkeepapp-modules/integration-twitch/src/Enums/TwitchEventSubType.phpapp-modules/integration-twitch/src/Http/Controllers/TwitchWebhookController.phpapp-modules/integration-twitch/src/Http/Middleware/VerifyTwitchSignature.phpapp-modules/integration-twitch/src/IntegrationTwitchServiceProvider.phpapp-modules/integration-twitch/src/Models/TwitchEventLog.phpapp-modules/integration-twitch/src/OAuth/Client/TwitchOAuthClient.phpapp-modules/integration-twitch/src/OAuth/Contracts/TwitchOAuthService.phpapp-modules/integration-twitch/src/OAuth/TwitchAppTokenService.phpapp-modules/integration-twitch/src/OAuth/TwitchOAuthClient.phpapp-modules/integration-twitch/src/Subscriber/Client/TwitchSubscribersClient.phpapp-modules/integration-twitch/src/Subscriber/Contracts/TwitchSubscribersService.phpapp-modules/integration-twitch/src/Subscriber/DTO/TwitchSubscriberDTO.phpapp-modules/integration-twitch/src/Subscriber/Enum/SubscriptionTiersEnum.phpapp-modules/integration-twitch/src/Transport/Requests/EventSub/CreateSubscription.phpapp-modules/integration-twitch/src/Transport/Requests/EventSub/DeleteSubscription.phpapp-modules/integration-twitch/src/Transport/Requests/EventSub/ListSubscriptions.phpapp-modules/integration-twitch/src/Transport/Requests/OAuth/ExchangeCodeForToken.phpapp-modules/integration-twitch/src/Transport/Requests/OAuth/GetAppAccessToken.phpapp-modules/integration-twitch/src/Transport/Requests/Users/GetCurrentUser.phpapp-modules/integration-twitch/src/Transport/Requests/Users/GetUsers.phpapp-modules/integration-twitch/src/Transport/TwitchHelixConnector.phpapp-modules/integration-twitch/src/Transport/TwitchOAuthConnector.phpapp-modules/integration-twitch/tests/Feature/LinkTwitchChannelCommandTest.phpapp-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.phpapp-modules/integration-twitch/tests/Feature/TwitchWebhookTest.phpconfig/services.php
💤 Files with no reviewable changes (8)
- app-modules/integration-twitch/src/Contracts/TwitchService.php
- app-modules/integration-twitch/src/Subscriber/Client/TwitchSubscribersClient.php
- app-modules/integration-twitch/src/Subscriber/DTO/TwitchSubscriberDTO.php
- app-modules/integration-twitch/src/OAuth/Client/TwitchOAuthClient.php
- app-modules/integration-twitch/src/OAuth/Contracts/TwitchOAuthService.php
- app-modules/integration-twitch/src/Client/TwitchBaseClient.php
- app-modules/integration-twitch/src/Subscriber/Enum/SubscriptionTiersEnum.php
- app-modules/integration-twitch/src/Subscriber/Contracts/TwitchSubscribersService.php
- Fix EventSub condition fields: channel.follow/shield_mode/shoutout now use moderator_user_id; channel.moderator.add/remove use broadcaster_user_id only - Rename version()/condition() to getVersion()/getCondition() with PHPDoc - Replace check-then-insert with atomic insertOrIgnore for dedup (race-safe) - Dispatch TwitchEventReceived event after successful ingestion - Make clientSecret private on TwitchOAuthConnector with getter method - Make twitch_message_id non-nullable in migration - Fix markdown lint (add language to fenced code block) - Remove explicit RefreshDatabase (LazilyRefreshDatabase applied globally)
…d ETL dirs (#271) - Create integration-twitch CONTEXT.md with glossary, structure, and module boundaries - Add Integration Twitch to CONTEXT-MAP.md with dependency rules - Broaden ETL definition in integration-discord CONTEXT.md (batch + real-time) - Scaffold empty ETL directories (Actions, Console, DTOs) for future processing
…Guzzle (#267) - Add TwitchOAuthConnector and TwitchHelixConnector (Saloon ^4.0) - Add 7 Saloon Requests: OAuth (ExchangeCodeForToken, GetAppAccessToken), Users (GetCurrentUser, GetUsers), EventSub (Create/List/DeleteSubscription) - Add TwitchAppTokenService for cached client_credentials app token - Refactor TwitchOAuthClient to use Saloon connectors (keeps OAuthClientContract) - Update IdentityProvider::Twitch to resolve TwitchOAuthClient directly - Add eventsub_secret and eventsub_callback to config/services.php - Delete legacy: TwitchBaseClient, TwitchService, TwitchOAuthService, Subscriber/
- Create twitch_event_logs migration (event_type indexed, broadcaster_user_id indexed, twitch_message_id unique for dedup, payload jsonb) - Add TwitchEventLog model mirroring DiscordEventLog pattern - Add VerifyTwitchSignature middleware (HMAC-SHA256, replay protection 10min) - Add TwitchWebhookController handling verification, notification, revocation - Add webhook route at POST /api/webhooks/twitch/eventsub - Add 7 Pest tests covering security, ingestion, and deduplication
- Add LinkTwitchChannelCommand to link Twitch channel to tenant via ExternalIdentity - Resolves broadcaster ID via Helix API GetUsers request - Creates ExternalIdentity with IdentityProvider::Twitch on the target tenant - Handles duplicate linking, missing user, and missing tenant gracefully - Register command in IntegrationTwitchServiceProvider - Add 4 Pest tests with Saloon MockClient
- Add TwitchEventSubType enum with 36 event types, version() and condition() - Add SubscribeTwitchEventsCommand (twitch:subscribe --all/--type) - Skips already-existing subscriptions, handles 403 gracefully - Register command in IntegrationTwitchServiceProvider - Add 6 Pest tests covering enum, command, dedup, and error handling
- Fix EventSub condition fields: channel.follow/shield_mode/shoutout now use moderator_user_id; channel.moderator.add/remove use broadcaster_user_id only - Rename version()/condition() to getVersion()/getCondition() with PHPDoc - Replace check-then-insert with atomic insertOrIgnore for dedup (race-safe) - Dispatch TwitchEventReceived event after successful ingestion - Make clientSecret private on TwitchOAuthConnector with getter method - Make twitch_message_id non-nullable in migration - Fix markdown lint (add language to fenced code block) - Remove explicit RefreshDatabase (LazilyRefreshDatabase applied globally)
812840e to
36de27d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
app-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php (2)
49-60: ⚡ Quick winStrengthen
--alltest by asserting total subscription calls.This test currently passes even if only a subset of types is subscribed. Use
$totalTypesto assert the expected total outbound calls.✅ Proposed test assertion update
test('subscribes to all event types', function (): void { $mock = mockEventSubResponses(); $totalTypes = count(TwitchEventSubType::cases()); $this->artisan('twitch:subscribe', [ 'broadcaster_user_id' => '12345', '--all' => true, ])->assertSuccessful(); - $mock->assertSent(ListSubscriptions::class); - $mock->assertSent(CreateSubscription::class); + $mock->assertSent(ListSubscriptions::class); + $mock->assertSent(CreateSubscription::class); + $mock->assertSentCount(1 + $totalTypes); // 1 list + N create calls });🤖 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-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php` around lines 49 - 60, The test "subscribes to all event types" currently doesn't assert the number of CreateSubscription calls; update the test that uses mockEventSubResponses() and $totalTypes (from TwitchEventSubType::cases()) to assert that CreateSubscription was sent exactly $totalTypes times after running $this->artisan('twitch:subscribe', [..., '--all' => true]) — keep the existing asserts for ListSubscriptions::class and ensure you add an assertion that the mock recorded CreateSubscription::class was sent $totalTypes times.
84-89: ⚡ Quick winAdd Hype Train version assertions to prevent API-version regressions.
getVersion()coverage misseschannel.hype_train.*, so version drift can slip through.✅ Proposed coverage extension
test('enum getVersion returns correct values', function (): void { expect(TwitchEventSubType::StreamOnline->getVersion())->toBe('1') ->and(TwitchEventSubType::ChannelFollow->getVersion())->toBe('2') ->and(TwitchEventSubType::ChannelUpdate->getVersion())->toBe('2') - ->and(TwitchEventSubType::ChannelSubscribe->getVersion())->toBe('1'); + ->and(TwitchEventSubType::ChannelHypeTrainBegin->getVersion())->toBe('2') + ->and(TwitchEventSubType::ChannelHypeTrainProgress->getVersion())->toBe('2') + ->and(TwitchEventSubType::ChannelHypeTrainEnd->getVersion())->toBe('2') + ->and(TwitchEventSubType::ChannelSubscribe->getVersion())->toBe('1'); });🤖 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-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php` around lines 84 - 89, The test is missing assertions for the channel.hype_train.* enum variants; update the test 'enum getVersion returns correct values' to include assertions for all TwitchEventSubType members that map to channel.hype_train.* (e.g., ChannelHypeTrainStart, ChannelHypeTrainProgress, ChannelHypeTrainEnd or whatever exact enum names exist) by chaining additional ->and(...->getVersion())->toBe('<expected version>') checks using the current expected version string from the TwitchEventSubType enum so these Hype Train versions are covered and protected from regressions.app-modules/integration-twitch/tests/Feature/TwitchWebhookTest.php (1)
9-9: ⚡ Quick winRemove the unused
$messageTypeparameter fromtwitchWebhookPayload.This parameter is never read and is already flagged by static analysis; dropping it keeps the test helper clean.
🤖 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-twitch/tests/Feature/TwitchWebhookTest.php` at line 9, twitchWebhookPayload currently declares an unused parameter $messageType; remove that parameter from the function signature (change function twitchWebhookPayload(string $eventType = 'stream.online'): array) and update any test helpers or calls that pass a second argument to only pass the $eventType, ensuring all references to twitchWebhookPayload throughout the tests are adjusted accordingly (search for twitchWebhookPayload(...) usages) so static analysis no longer flags the unused parameter.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@app-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.php`:
- Around line 82-91: The current ListSubscriptions call only reads the first
page of EventSub subscriptions and misses later pages, causing duplicate
subscribe attempts; update getExistingSubscriptions() to paginate: repeatedly
call $helix->send(new ListSubscriptions($afterCursor)) (or include the after
cursor via the request or connector), read each response's json('data') and
json('pagination.cursor' or 'pagination.after') and accumulate entries until no
cursor is returned, then run the existing filter (checking
condition.broadcaster_user_id / to_broadcaster_user_id), pluck types and return
the full set; reference ListSubscriptions, $helix->send, json('data') and
pagination.cursor/after when implementing.
- Around line 36-38: Replace the direct TwitchEventSubType::from($specificType)
call with explicit validation so an invalid --type yields a clean CLI error: use
TwitchEventSubType::tryFrom($specificType) (or check against
array_map(fn($c)=>$c->name, TwitchEventSubType::cases())) and if it returns
null/invalid, write a user-friendly error (throw InvalidArgumentException or
$this->io->error and return non-zero) instead of allowing a ValueError to
bubble. Also update getExistingSubscriptions() to handle Twitch pagination:
iterate ListSubscriptions responses following the returned cursor(s) and
accumulate all json('data') pages rather than making a single call, ensuring all
existing subscriptions are detected.
In
`@app-modules/integration-twitch/src/Http/Middleware/VerifyTwitchSignature.php`:
- Line 22: The timestamp parsing and secret retrieval can throw or return empty
and cause a 500; update VerifyTwitchSignature to fail closed by wrapping
Date::parse($timestamp) in a try/catch (or validate with
Date::make/Date::hasFormat) and abort(403, 'Message timestamp too old') when
parsing fails or when diffInMinutes(now(), absolute: true) > 10, and retrieve
the EventSub secret using a non-throwing accessor (e.g.
config('services.twitch.eventsub_secret') or config()->get(...)) and abort(403,
'Missing EventSub secret') if the secret is null/empty before computing the HMAC
— adjust the code paths around Date::parse and
config()->string('services.twitch.eventsub_secret') accordingly.
- Around line 24-26: Before computing $expectedSignature in the
VerifyTwitchSignature middleware, guard against an empty EventSub secret: check
the $secret (from config()->string('services.twitch.eventsub_secret')) for empty
or whitespace and fail fast (log an error and abort/throw a 5xx response)
instead of using an empty key to compute $hmacMessage and $expectedSignature;
implement this check at the start of the signature logic in the
VerifyTwitchSignature class so the request is rejected immediately if the secret
is missing.
In `@app-modules/integration-twitch/src/OAuth/TwitchAppTokenService.php`:
- Around line 19-31: The getToken() flow in TwitchAppTokenService currently
calls Cache::remember(..., $this->getCacheTtl()) which reads the old
twitch_app_token_expires_in before requesting a fresh GetAppAccessToken; change
it so the TTL is derived from the fresh response: call
$this->connector->send(new GetAppAccessToken(...)) inside the method, extract
and validate access_token and expires_in (ensure access_token is a non-empty
string and expires_in is an integer > 0), compute ttl = max(0, (int)$expires_in
- 300), store the expires_in and the token with Cache::put using that computed
ttl, and return the access token; update/getCacheTtl() usage accordingly so it
no longer drives the new token's TTL. Ensure you reference the GetAppAccessToken
response handling and throw or gracefully handle missing/invalid values instead
of returning null.
---
Nitpick comments:
In
`@app-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php`:
- Around line 49-60: The test "subscribes to all event types" currently doesn't
assert the number of CreateSubscription calls; update the test that uses
mockEventSubResponses() and $totalTypes (from TwitchEventSubType::cases()) to
assert that CreateSubscription was sent exactly $totalTypes times after running
$this->artisan('twitch:subscribe', [..., '--all' => true]) — keep the existing
asserts for ListSubscriptions::class and ensure you add an assertion that the
mock recorded CreateSubscription::class was sent $totalTypes times.
- Around line 84-89: The test is missing assertions for the channel.hype_train.*
enum variants; update the test 'enum getVersion returns correct values' to
include assertions for all TwitchEventSubType members that map to
channel.hype_train.* (e.g., ChannelHypeTrainStart, ChannelHypeTrainProgress,
ChannelHypeTrainEnd or whatever exact enum names exist) by chaining additional
->and(...->getVersion())->toBe('<expected version>') checks using the current
expected version string from the TwitchEventSubType enum so these Hype Train
versions are covered and protected from regressions.
In `@app-modules/integration-twitch/tests/Feature/TwitchWebhookTest.php`:
- Line 9: twitchWebhookPayload currently declares an unused parameter
$messageType; remove that parameter from the function signature (change function
twitchWebhookPayload(string $eventType = 'stream.online'): array) and update any
test helpers or calls that pass a second argument to only pass the $eventType,
ensuring all references to twitchWebhookPayload throughout the tests are
adjusted accordingly (search for twitchWebhookPayload(...) usages) so static
analysis no longer flags the unused parameter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7abf5a01-97a4-4b88-a61e-aeeff9e91434
📒 Files selected for processing (40)
CONTEXT-MAP.mdapp-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.phpapp-modules/integration-discord/CONTEXT.mdapp-modules/integration-twitch/CONTEXT.mdapp-modules/integration-twitch/database/migrations/2026_05_20_000001_create_twitch_event_logs_table.phpapp-modules/integration-twitch/routes/twitch-webhook-routes.phpapp-modules/integration-twitch/src/Client/TwitchBaseClient.phpapp-modules/integration-twitch/src/Console/LinkTwitchChannelCommand.phpapp-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.phpapp-modules/integration-twitch/src/Contracts/TwitchService.phpapp-modules/integration-twitch/src/ETL/Actions/.gitkeepapp-modules/integration-twitch/src/ETL/Console/.gitkeepapp-modules/integration-twitch/src/ETL/DTOs/.gitkeepapp-modules/integration-twitch/src/Enums/TwitchEventSubType.phpapp-modules/integration-twitch/src/Events/TwitchEventReceived.phpapp-modules/integration-twitch/src/Http/Controllers/TwitchWebhookController.phpapp-modules/integration-twitch/src/Http/Middleware/VerifyTwitchSignature.phpapp-modules/integration-twitch/src/IntegrationTwitchServiceProvider.phpapp-modules/integration-twitch/src/Models/TwitchEventLog.phpapp-modules/integration-twitch/src/OAuth/Client/TwitchOAuthClient.phpapp-modules/integration-twitch/src/OAuth/Contracts/TwitchOAuthService.phpapp-modules/integration-twitch/src/OAuth/TwitchAppTokenService.phpapp-modules/integration-twitch/src/OAuth/TwitchOAuthClient.phpapp-modules/integration-twitch/src/Subscriber/Client/TwitchSubscribersClient.phpapp-modules/integration-twitch/src/Subscriber/Contracts/TwitchSubscribersService.phpapp-modules/integration-twitch/src/Subscriber/DTO/TwitchSubscriberDTO.phpapp-modules/integration-twitch/src/Subscriber/Enum/SubscriptionTiersEnum.phpapp-modules/integration-twitch/src/Transport/Requests/EventSub/CreateSubscription.phpapp-modules/integration-twitch/src/Transport/Requests/EventSub/DeleteSubscription.phpapp-modules/integration-twitch/src/Transport/Requests/EventSub/ListSubscriptions.phpapp-modules/integration-twitch/src/Transport/Requests/OAuth/ExchangeCodeForToken.phpapp-modules/integration-twitch/src/Transport/Requests/OAuth/GetAppAccessToken.phpapp-modules/integration-twitch/src/Transport/Requests/Users/GetCurrentUser.phpapp-modules/integration-twitch/src/Transport/Requests/Users/GetUsers.phpapp-modules/integration-twitch/src/Transport/TwitchHelixConnector.phpapp-modules/integration-twitch/src/Transport/TwitchOAuthConnector.phpapp-modules/integration-twitch/tests/Feature/LinkTwitchChannelCommandTest.phpapp-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.phpapp-modules/integration-twitch/tests/Feature/TwitchWebhookTest.phpconfig/services.php
💤 Files with no reviewable changes (8)
- app-modules/integration-twitch/src/OAuth/Client/TwitchOAuthClient.php
- app-modules/integration-twitch/src/Subscriber/Contracts/TwitchSubscribersService.php
- app-modules/integration-twitch/src/Subscriber/DTO/TwitchSubscriberDTO.php
- app-modules/integration-twitch/src/Client/TwitchBaseClient.php
- app-modules/integration-twitch/src/Subscriber/Enum/SubscriptionTiersEnum.php
- app-modules/integration-twitch/src/Contracts/TwitchService.php
- app-modules/integration-twitch/src/OAuth/Contracts/TwitchOAuthService.php
- app-modules/integration-twitch/src/Subscriber/Client/TwitchSubscribersClient.php
✅ Files skipped from review due to trivial changes (5)
- app-modules/integration-twitch/src/Events/TwitchEventReceived.php
- app-modules/integration-twitch/routes/twitch-webhook-routes.php
- CONTEXT-MAP.md
- app-modules/integration-discord/CONTEXT.md
- app-modules/integration-twitch/CONTEXT.md
Register EditTenantProfilePage as the tenant profile in AdminPanelProvider, with form fields for name, slug, domain, and active toggle, plus an embedded ConnectionHub Livewire component for managing integrations.
…ter providers - Fix connect() return type: use $this->redirect() instead of redirect()->away() which returns a Livewire Redirector, not RedirectResponse - Use dynamic panel ID instead of hardcoded 'user' for OAuth state - Implement disconnect() method with ExternalIdentity soft-disconnect - Switch from IdentityProvider::cases() to supportedProviders() filter - Pass current panel to view for panel-aware scope display
Admin panel (streamer/tenant) requests full EventSub scopes for channel management, while the app panel (regular user) only requests user:read:email. TwitchOAuthClient resolves scopes from the OAuthState panel, and IdentityProvider::getScopes() accepts an optional panel param so the ConnectionHub UI displays the correct scope list per context.
The OAuth callback route runs outside Filament's panel middleware, so filament()->auth() fails with NoDefaultPanelSetException. Set the current panel from the OAuthState before calling setUser().
Admin view shows all tenant connections in a table layout grouped by provider, with full detail (avatar, username, platform name, email, date). User view keeps single-instance connection per provider. - Persist panel and tenantId as Livewire properties during mount() to avoid unreliable filament() calls on subsequent requests - Add disconnectById() for admin to target specific identities - Fix enum-to-string comparison (broken in PHP 8.5) to enum-to-enum - Replace #[Computed] with private methods to avoid stale context - Redesign both views with brand accents and clear visual hierarchy
Document local EventSub testing with twitch-cli: setup, firing events, key flags, supported events, verification, and known raid field quirk.
Add @return array<int, self> to supportedProviders() and remove unnecessary nullsafe operator on non-nullable OAuthStateDTO parameter.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In `@app-modules/identity/src/Auth/Actions/AuthenticateAction.php`:
- Line 53: AuthenticateAction currently calls
filament()->setCurrentPanel(filament()->getPanel($state->panel)) without
validating OAuthStateDTO::$panel; update AuthenticateAction to first call
filament()->getPanel($state->panel), check if the result is null, and if so fail
gracefully (e.g., log the invalid panel, return an error/redirect or throw a
handled exception) instead of passing null into filament()->setCurrentPanel();
only call setCurrentPanel() when the resolved panel is non-null to avoid
downstream 500s.
In `@app/Livewire/ConnectionHub.php`:
- Around line 18-25: ConnectionHub currently trusts public Livewire properties
$panel and $tenantId and dereferences Tenant without null checks and exposes
disconnectById without authorization; fix by treating $panel/$tenantId as
untrusted: always read the authoritative values from
filament()->getCurrentPanel() and filament()->getTenant() inside render(),
connect() and disconnectById() instead of using the public properties, add a
null check after Tenant::query()->find(...) (or better, use
filament()->getTenant()) before accessing ->slug in connect(), and enforce an
authorization/ownership check in disconnectById() so it only updates
ExternalIdentity records that belong to the resolved tenant (compare resolved
tenant id to ExternalIdentity->tenant_id or use a policy/gate) and return/throw
when tenant is missing or unauthorized.
- Around line 85-103: disconnectById currently lacks an authorization check and
can be invoked by non-admins; add an explicit admin/permission guard at the
start of ConnectionHub::disconnectById (before querying ExternalIdentity) that
verifies the current user is authenticated and has the admin permission (e.g.
auth()->check() && auth()->user()->can('admin') or Gate::allows('admin')), and
if the check fails return early (send the existing
Notification::make()->title('Unauthorized')->danger()->send() or abort(403)) so
only authorized admins can proceed to update ExternalIdentity->disconnected_at.
In `@resources/views/livewire/connection-hub-admin.blade.php`:
- Line 19: The current filter that builds $connections from $tenantProviders
includes all identities for a provider; update the predicate used in the filter
(the closure that references ExternalIdentity) to also call and require
$c->isConnected() so only active/connected identities are included when
constructing $connections; keep the same variables ($tenantProviders,
$connections) and ExternalIdentity type reference so the change mirrors the
active filter used elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 08204041-7e9d-4d35-bbda-5b2ffac44280
📒 Files selected for processing (10)
app-modules/identity/src/Auth/Actions/AuthenticateAction.phpapp-modules/identity/src/ExternalIdentity/Enums/IdentityProvider.phpapp-modules/integration-twitch/CONTEXT.mdapp-modules/integration-twitch/src/OAuth/TwitchOAuthClient.phpapp-modules/panel-admin/src/Tenant/EditTenantProfilePage.phpapp/Livewire/ConnectionHub.phpapp/Providers/Filament/AdminPanelProvider.phpconfig/services.phpresources/views/livewire/connection-hub-admin.blade.phpresources/views/livewire/connection-hub.blade.php
✅ Files skipped from review due to trivial changes (1)
- app-modules/integration-twitch/CONTEXT.md
…mmand Deletes all existing EventSub subscriptions for a broadcaster. Lists each subscription, sends DELETE via Helix API, reports results.
…b secret - Display callback URL, masked secret, and broadcaster ID before operations - Add default eventsub_secret in config/services.php for dev convenience
…er_user_id extraction Twitch EventSub payloads use different field names per event type: - chat.message: chatter_user_id (not user_id) - channel.raid: from_broadcaster_user_id / to_broadcaster_user_id Add fallback chain so user_id and broadcaster_user_id columns are populated regardless of which field name the event type uses.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.php (2)
63-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle invalid
--typewithout uncaught enum exceptions.Line 64 uses
TwitchEventSubType::from(...); invalid input throwsValueErrorinstead of returning a clean command error.Suggested fix
- $types = $specificType - ? [TwitchEventSubType::from($specificType)] - : TwitchEventSubType::cases(); + if ($specificType !== null) { + $type = TwitchEventSubType::tryFrom($specificType); + if ($type === null) { + $this->error(sprintf('Invalid event type: %s', $specificType)); + return self::FAILURE; + } + $types = [$type]; + } else { + $types = TwitchEventSubType::cases(); + }#!/bin/bash set -euo pipefail FILE="app-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.php" # Verify direct enum parsing is used without guard. rg -n "TwitchEventSubType::from|TwitchEventSubType::tryFrom|ValueError|Invalid event type" "$FILE" -n -C2🤖 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-twitch/src/Console/SubscribeTwitchEventsCommand.php` around lines 63 - 65, The current SubscribeTwitchEventsCommand uses TwitchEventSubType::from($specificType) which will throw a ValueError on invalid input; update the parsing to safely handle bad values by using TwitchEventSubType::tryFrom($specificType) (or explicitly validating $specificType before converting), check for null and call $this->error(...) / return a non-zero exit code from the execute method to emit a clean command error; adjust the block that builds $types (and any usages of $specificType) so invalid types produce a user-friendly error instead of an uncaught exception.
148-159:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPaginate EventSub listing when discovering existing subscriptions.
Lines 150-159 process only one
ListSubscriptionsresponse page. If Twitch returns additional pages, existing subscriptions may be missed, causing duplicate create/delete behavior.Suggested direction
- $response = $helix->send(new ListSubscriptions()); - - /** `@var` array<int, array{id: string, type: string, condition: array<string, string>}> $subscriptions */ - $subscriptions = $response->json('data', []); + $subscriptions = []; + $after = null; + do { + $response = $helix->send(new ListSubscriptions(after: $after)); + /** `@var` array<int, array{id: string, type: string, condition: array<string, string>}> $page */ + $page = $response->json('data', []); + $subscriptions = [...$subscriptions, ...$page]; + $after = $response->json('pagination.cursor'); + } while ($after !== null); return collect($subscriptions) ->filter(fn (array $sub): bool => ($sub['condition']['broadcaster_user_id'] ?? null) === $broadcasterId || ($sub['condition']['to_broadcaster_user_id'] ?? null) === $broadcasterId) ->values() ->all();#!/bin/bash set -euo pipefail # 1) Inspect command pagination handling CMD="app-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.php" awk 'NR>=130 && NR<=200 {printf "%4d:%s\n", NR, $0}' "$CMD" # 2) Inspect ListSubscriptions request shape (supports `after` or not) LIST_FILE="$(fd -a 'ListSubscriptions.php' app-modules/integration-twitch/src | head -n1)" echo "List request file: ${LIST_FILE:-not-found}" if [ -n "${LIST_FILE:-}" ]; then awk 'NR>=1 && NR<=220 {printf "%4d:%s\n", NR, $0}' "$LIST_FILE" fi🤖 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-twitch/src/Console/SubscribeTwitchEventsCommand.php` around lines 148 - 159, getExistingSubscriptions currently calls $helix->send(new ListSubscriptions()) once and only inspects the first page; change it to paginate by repeatedly calling $helix->send(new ListSubscriptions(after: $cursor)) (or set the `after` param on ListSubscriptions) until the response has no pagination cursor, collecting and merging all `data` arrays before filtering. Ensure you still apply the same broadcaster filter (the condition keys 'broadcaster_user_id' and 'to_broadcaster_user_id') and return the combined values; update references to ListSubscriptions and $helix->send to perform the loop/merge and use the response pagination cursor field to advance pages.
🧹 Nitpick comments (1)
app-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php (1)
51-62: ⚡ Quick winStrengthen
--allassertions to catch partial subscription runs.Line 53 computes
$totalTypes, but the test only checks thatCreateSubscriptionwas sent at least once. This can miss regressions where only a subset is created.Suggested test tightening
test('subscribes to all event types', function (): void { $mock = mockEventSubResponses(); $totalTypes = count(TwitchEventSubType::cases()); $this->artisan('twitch:subscribe', [ 'broadcaster_user_id' => '12345', '--all' => true, ])->assertSuccessful(); $mock->assertSent(ListSubscriptions::class); $mock->assertSent(CreateSubscription::class); + $mock->assertSentCount($totalTypes + 1); // 1 list + N creates });🤖 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-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php` around lines 51 - 62, The test computes $totalTypes but only asserts CreateSubscription was sent at least once; update the assertion so the mocked HTTP client verifies a CreateSubscription request was sent exactly for every event type: after calling artisan('twitch:subscribe'...) use the mock returned by mockEventSubResponses() to assert CreateSubscription was sent $totalTypes times (or iterate TwitchEventSubType::cases() and assert each expected payload was sent), keeping the ListSubscriptions assertion as well to ensure both listing and full subscription creation occur.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@config/services.php`:
- Line 53: The 'eventsub_secret' entry currently uses a hardcoded default value
via env('TWITCH_EVENTSUB_SECRET', 'h34rt-...'), which makes webhook signatures
predictable; update the 'eventsub_secret' configuration to remove the hardcoded
fallback by calling env('TWITCH_EVENTSUB_SECRET') with no default (or set it to
null) so the secret is only taken from the environment, and ensure any
startup/validation logic that reads config('services.eventsub_secret') fails
fast or logs a clear error if the value is missing.
---
Duplicate comments:
In `@app-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.php`:
- Around line 63-65: The current SubscribeTwitchEventsCommand uses
TwitchEventSubType::from($specificType) which will throw a ValueError on invalid
input; update the parsing to safely handle bad values by using
TwitchEventSubType::tryFrom($specificType) (or explicitly validating
$specificType before converting), check for null and call $this->error(...) /
return a non-zero exit code from the execute method to emit a clean command
error; adjust the block that builds $types (and any usages of $specificType) so
invalid types produce a user-friendly error instead of an uncaught exception.
- Around line 148-159: getExistingSubscriptions currently calls $helix->send(new
ListSubscriptions()) once and only inspects the first page; change it to
paginate by repeatedly calling $helix->send(new ListSubscriptions(after:
$cursor)) (or set the `after` param on ListSubscriptions) until the response has
no pagination cursor, collecting and merging all `data` arrays before filtering.
Ensure you still apply the same broadcaster filter (the condition keys
'broadcaster_user_id' and 'to_broadcaster_user_id') and return the combined
values; update references to ListSubscriptions and $helix->send to perform the
loop/merge and use the response pagination cursor field to advance pages.
---
Nitpick comments:
In
`@app-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.php`:
- Around line 51-62: The test computes $totalTypes but only asserts
CreateSubscription was sent at least once; update the assertion so the mocked
HTTP client verifies a CreateSubscription request was sent exactly for every
event type: after calling artisan('twitch:subscribe'...) use the mock returned
by mockEventSubResponses() to assert CreateSubscription was sent $totalTypes
times (or iterate TwitchEventSubType::cases() and assert each expected payload
was sent), keeping the ListSubscriptions assertion as well to ensure both
listing and full subscription creation occur.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6a267ee4-e833-4494-9b34-6ed0e80ba07e
📒 Files selected for processing (5)
app-modules/integration-twitch/src/Console/SubscribeTwitchEventsCommand.phpapp-modules/integration-twitch/src/Http/Controllers/TwitchWebhookController.phpapp-modules/integration-twitch/tests/Feature/SubscribeTwitchEventsCommandTest.phpapp-modules/integration-twitch/tests/Feature/TwitchWebhookTest.phpconfig/services.php
…re event logs Add TwitchSubscription model with full Twitch API fields (subscription_id, type, status, condition, transport, callback_url, cost, version) plus tenant_id FK. Add TwitchSubscriptionStatus enum with Filament HasColor/HasLabel. Add tenant_id nullable FK to twitch_event_logs with tenant() relationship.
Route now includes {tenant:slug} parameter. Controller receives Tenant
via implicit binding and sets tenant_id on event logs directly.
Add SubstituteBindings middleware to webhook route group.
Update all webhook tests to create and use a test tenant.
… subscriptions Add TwitchCluster to admin panel with dedicated sub-navigation: - Dashboard page with stats (total events, events today, active/errored subs), events-per-day line chart, and events-by-type doughnut chart - TwitchEventLogResource: list/view event logs with type badges and payload viewer - TwitchSubscriptionResource: list subscriptions with sync and delete actions - RegisterSubscriptionsAction: standalone Filament action with detailed modal showing broadcaster config, callback URL, secret, and grouped event types - RegisterTwitchSubscriptionsAction: domain action that creates EventSub subscriptions via Twitch API and persists them locally with tenant_id - All widgets and resources are tenant-scoped
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app-modules/integration-twitch/src/Http/Controllers/TwitchWebhookController.php (1)
27-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard empty message IDs before insert/dispatch path.
Without a non-empty message ID check,
where('twitch_message_id', $messageId)can becomewhere nulland fetch an unrelated row, causing wrong event dispatch.Proposed fix
$messageId = $request->header('Twitch-Eventsub-Message-Id'); + if (!is_string($messageId) || $messageId === '') { + return response('', 403); + } $inserted = TwitchEventLog::query()->insertOrIgnore([ 'tenant_id' => $tenant->getKey(), @@ if ($inserted > 0) { $eventLog = TwitchEventLog::query() + ->where('tenant_id', $tenant->getKey()) ->where('twitch_message_id', $messageId) ->first();🤖 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-twitch/src/Http/Controllers/TwitchWebhookController.php` around lines 27 - 50, Ensure the code guards against empty Twitch message IDs in TwitchWebhookController: check that $messageId is non-empty before calling TwitchEventLog::query()->insertOrIgnore(...) and before the subsequent TwitchEventLog::query()->where('twitch_message_id', $messageId)->first(); if $messageId is empty, skip the insert/lookup/dispatch path (or return early) to avoid executing where(null) and fetching unrelated rows.
🧹 Nitpick comments (3)
app-modules/panel-admin/src/Twitch/Resources/TwitchSubscriptionResource/Pages/ListTwitchSubscriptions.php (1)
79-85: ⚡ Quick winRecommended: Catch broader exception types.
Currently only
RequestExceptionis caught. Other exceptions (e.g., JSON parsing errors, database errors during upsert/delete, or Saloon-internal errors) would propagate uncaught, potentially crashing the request or leaving partial state.♻️ Proposed fix: catch all exceptions with specific handling
- } catch (RequestException $requestException) { + } catch (RequestException $requestException) { Notification::make() ->danger() ->title(__('panel-admin::twitch.subscriptions.actions.sync_failed')) ->body($requestException->getMessage()) ->send(); + } catch (\Throwable $exception) { + Notification::make() + ->danger() + ->title(__('panel-admin::twitch.subscriptions.actions.sync_failed')) + ->body('An unexpected error occurred: ' . $exception->getMessage()) + ->send(); + + report($exception); }🤖 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/panel-admin/src/Twitch/Resources/TwitchSubscriptionResource/Pages/ListTwitchSubscriptions.php` around lines 79 - 85, The current try/catch only handles RequestException and will let JSON, DB, or Saloon internal errors escape; update the error handling in ListTwitchSubscriptions (the try block that currently catches RequestException) to add a broader catch for \Throwable (or at minimum \Exception) after the existing RequestException catch so all errors are handled, and in that new catch mirror the Notification::make()->danger()->title(...)->body(...)->send() behavior (include $e->getMessage() and/or $e->getTraceAsString()) and also call a logging/reporting helper (e.g., report($e) or Log::error) to ensure unseen exceptions are recorded while preserving the RequestException-specific handling above.app-modules/panel-admin/src/Twitch/Pages/TwitchDashboard.php (1)
28-28: ⚡ Quick winMake the
$viewproperty static for consistency.The
$viewproperty should be declared asprotected static stringto align with Filament's conventions and match the pattern used by other page metadata properties like$slug,$cluster, and$navigationIcon.♻️ Proposed fix
- protected string $view = 'panel-admin::twitch.dashboard'; + protected static string $view = 'panel-admin::twitch.dashboard';🤖 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/panel-admin/src/Twitch/Pages/TwitchDashboard.php` at line 28, The TwitchDashboard class currently declares the page view as an instance property "protected string $view = 'panel-admin::twitch.dashboard';" — change this to a static property by updating the declaration to "protected static string $view = 'panel-admin::twitch.dashboard';" so it matches Filament conventions and the other metadata properties like $slug, $cluster, and $navigationIcon; locate the property in the TwitchDashboard class and make this single-line change.app-modules/integration-twitch/tests/Feature/TwitchWebhookTest.php (1)
119-137: 💤 Low valueConsider asserting
tenant_idfor consistency.For consistency with the notification event test (line 114), consider asserting that the revocation event also has the correct
tenant_id.🧪 Suggested assertion
expect($log)->not->toBeNull() - ->and($log->event_type)->toBe('stream.online'); + ->and($log->event_type)->toBe('stream.online') + ->and($log->tenant_id)->toBe($this->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/integration-twitch/tests/Feature/TwitchWebhookTest.php` around lines 119 - 137, The revocation test in TwitchWebhookTest stores a TwitchEventLog but doesn't assert its tenant_id; add an assertion after fetching TwitchEventLog (in the test named "stores revocation event in twitch_event_logs") to verify $log->tenant_id equals the same tenant id used when posting the webhook (i.e., the tenant value used in the notification event test), ensuring consistency with the notification event assertion; locate the check near the existing expect($log)->... block that follows postTwitchWebhook.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@app-modules/panel-admin/src/Twitch/Resources/TwitchSubscriptionResource.php`:
- Around line 89-95: The before hook in TwitchSubscriptionResource currently
swallows all RequestException errors when calling TwitchHelixConnector->send(new
DeleteSubscription(...)), which can hide non-404 failures and lead to silent
desync; change the catch to capture the exception (catch RequestException $e),
inspect the HTTP response/status code (e.g., $e->hasResponse() &&
$e->getResponse()->getStatusCode()), allow the deletion to continue only when
the remote response is 404/Not Found, and rethrow or propagate the exception for
any other status so the local delete is aborted and failure is visible; target
the closure passed to ->before and the DeleteSubscription call when making this
change.
In
`@app-modules/panel-admin/src/Twitch/Resources/TwitchSubscriptionResource/Pages/ListTwitchSubscriptions.php`:
- Around line 46-67: The sync currently trusts every entry in
$remoteSubscriptions and upserts them into TwitchSubscription, which can
associate other broadcasters' subscriptions with the current tenant; fix this in
ListTwitchSubscriptions by first resolving the current tenant's linked
broadcaster ID (e.g. $tenantBroadcasterId) and filter $remoteSubscriptions to
only process entries where $sub['condition']['broadcaster_user_id'] or
$sub['condition']['to_broadcaster_user_id'] equals that ID; for non-matching
subscriptions skip (and optionally log a warning) and ensure the updateOrCreate
uses the resolved $tenantId for tenant_id rather than deriving it from the DB
fallback when the broadcaster doesn't match.
- Around line 60-62: Currently the code falls back to 0 for 'tenant_id' using
TwitchSubscription::query()->where('subscription_id',
$sub['id'])->value('tenant_id') which risks creating orphaned or cross-tenant
records when filament()->getTenant() is null; instead require a tenant context
or fail fast by checking filament()->getTenant() early (ensure $tenantId is set)
and throw/skip if null, and when looking up an existing subscription include
tenant-scoping (e.g., add ->where('broadcaster_id', $currentBroadcasterId) or
->where('tenant_id', $tenantId) to the TwitchSubscription::query() so you never
inherit a tenant_id from another tenant; remove the default 0 fallback so
orphaned records cannot be created and ensure the delete logic still uses
tenant-scoped queries.
In `@composer.json`:
- Line 45: The composer dependency "phiki/phiki" appears unused in application
code; either confirm and document its intended use or remove it: search for
references to "phiki", "Phiki\\" and files like config/debugbar.php and any
controller/service/view that might rely on Phiki rendering, add documentation in
README or composer.json "extra" section explaining where and why Phiki is
required (including any dev vs prod usage), or remove the "phiki/phiki" entry
from composer.json and update composer.lock and any docs to reflect removal.
---
Outside diff comments:
In
`@app-modules/integration-twitch/src/Http/Controllers/TwitchWebhookController.php`:
- Around line 27-50: Ensure the code guards against empty Twitch message IDs in
TwitchWebhookController: check that $messageId is non-empty before calling
TwitchEventLog::query()->insertOrIgnore(...) and before the subsequent
TwitchEventLog::query()->where('twitch_message_id', $messageId)->first(); if
$messageId is empty, skip the insert/lookup/dispatch path (or return early) to
avoid executing where(null) and fetching unrelated rows.
---
Nitpick comments:
In `@app-modules/integration-twitch/tests/Feature/TwitchWebhookTest.php`:
- Around line 119-137: The revocation test in TwitchWebhookTest stores a
TwitchEventLog but doesn't assert its tenant_id; add an assertion after fetching
TwitchEventLog (in the test named "stores revocation event in
twitch_event_logs") to verify $log->tenant_id equals the same tenant id used
when posting the webhook (i.e., the tenant value used in the notification event
test), ensuring consistency with the notification event assertion; locate the
check near the existing expect($log)->... block that follows postTwitchWebhook.
In `@app-modules/panel-admin/src/Twitch/Pages/TwitchDashboard.php`:
- Line 28: The TwitchDashboard class currently declares the page view as an
instance property "protected string $view = 'panel-admin::twitch.dashboard';" —
change this to a static property by updating the declaration to "protected
static string $view = 'panel-admin::twitch.dashboard';" so it matches Filament
conventions and the other metadata properties like $slug, $cluster, and
$navigationIcon; locate the property in the TwitchDashboard class and make this
single-line change.
In
`@app-modules/panel-admin/src/Twitch/Resources/TwitchSubscriptionResource/Pages/ListTwitchSubscriptions.php`:
- Around line 79-85: The current try/catch only handles RequestException and
will let JSON, DB, or Saloon internal errors escape; update the error handling
in ListTwitchSubscriptions (the try block that currently catches
RequestException) to add a broader catch for \Throwable (or at minimum
\Exception) after the existing RequestException catch so all errors are handled,
and in that new catch mirror the
Notification::make()->danger()->title(...)->body(...)->send() behavior (include
$e->getMessage() and/or $e->getTraceAsString()) and also call a
logging/reporting helper (e.g., report($e) or Log::error) to ensure unseen
exceptions are recorded while preserving the RequestException-specific handling
above.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1e6dcf14-cc7c-4cb6-a84e-3e2db2a669d4
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
app-modules/integration-twitch/database/migrations/2026_05_22_000001_create_twitch_subscriptions_table.phpapp-modules/integration-twitch/database/migrations/2026_05_22_000002_add_tenant_id_to_twitch_event_logs_table.phpapp-modules/integration-twitch/routes/twitch-webhook-routes.phpapp-modules/integration-twitch/src/Actions/RegisterTwitchSubscriptionsAction.phpapp-modules/integration-twitch/src/Enums/TwitchSubscriptionStatus.phpapp-modules/integration-twitch/src/Http/Controllers/TwitchWebhookController.phpapp-modules/integration-twitch/src/Models/TwitchEventLog.phpapp-modules/integration-twitch/src/Models/TwitchSubscription.phpapp-modules/integration-twitch/tests/Feature/TwitchWebhookTest.phpapp-modules/panel-admin/lang/en/twitch.phpapp-modules/panel-admin/resources/views/twitch/dashboard.blade.phpapp-modules/panel-admin/resources/views/twitch/register-subscriptions-modal.blade.phpapp-modules/panel-admin/src/PanelAdminServiceProvider.phpapp-modules/panel-admin/src/Twitch/Actions/RegisterSubscriptionsAction.phpapp-modules/panel-admin/src/Twitch/Pages/TwitchDashboard.phpapp-modules/panel-admin/src/Twitch/Resources/TwitchEventLogResource.phpapp-modules/panel-admin/src/Twitch/Resources/TwitchEventLogResource/Pages/ListTwitchEventLogs.phpapp-modules/panel-admin/src/Twitch/Resources/TwitchEventLogResource/Pages/ViewTwitchEventLog.phpapp-modules/panel-admin/src/Twitch/Resources/TwitchSubscriptionResource.phpapp-modules/panel-admin/src/Twitch/Resources/TwitchSubscriptionResource/Pages/ListTwitchSubscriptions.phpapp-modules/panel-admin/src/Twitch/TwitchCluster.phpapp-modules/panel-admin/src/Twitch/Widgets/EventsByTypeChartWidget.phpapp-modules/panel-admin/src/Twitch/Widgets/EventsPerDayChartWidget.phpapp-modules/panel-admin/src/Twitch/Widgets/TwitchStatsWidget.phpcomposer.json
✅ Files skipped from review due to trivial changes (7)
- app-modules/integration-twitch/src/Enums/TwitchSubscriptionStatus.php
- app-modules/integration-twitch/database/migrations/2026_05_22_000001_create_twitch_subscriptions_table.php
- app-modules/panel-admin/lang/en/twitch.php
- app-modules/panel-admin/src/Twitch/Resources/TwitchEventLogResource/Pages/ViewTwitchEventLog.php
- app-modules/panel-admin/resources/views/twitch/dashboard.blade.php
- app-modules/panel-admin/src/Twitch/Widgets/EventsPerDayChartWidget.php
- app-modules/panel-admin/src/Twitch/Resources/TwitchEventLogResource.php
|
LGTM |
) Principais pontos: - Novo pacote he4rt/profile registrado em composer.json e composer.lock. - Migration user_profiles com UUID, escopo por tenant, unique(user_id, tenant_id) e índice parcial para available_for_proposals = true. - Model Profile com HasUuids, factory, relacionamentos, casts de enum/date/bool/array e validação das chaves de social_links. - Enums SeniorityLevel, StartAvailability e SocialPlatform com labels pt-BR. - ProfileServiceProvider registrando migrations, morph map e criação eager quando há insert em tenant_users via attach. - Testes cobrindo criação automática, não duplicação, múltiplos tenants, factory, labels e validação de social_links. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Description This PR introduces a new `he4rt/profile` module providing tenant-scoped user profiles with automatic creation when users join tenants. It includes database migrations with UUID primary keys, Eloquent models with relationships and validation, enums for seniority levels and job availability, factories, comprehensive tests, and localized translations. The PR also refactors console commands across multiple modules to use PHP 8 attributes for metadata declarations, and relocates the `TenantUserObserver` to the identity module with proper observer registration. ## References - Related to ongoing modularization efforts (see commits: refactor(profile): move TenantUserObserver to identity module) - Complements recent integration modules: Twitch (`#272`), Admin Panel (`#206`), Discord ETL (`#205`) - Code quality improvements (`#276`) ## Dependencies & Requirements - New dependency added: `he4rt/profile` (>=1) - Updated dependencies: - `filament/filament`: ^5.6.4 → ^5.6.5 - `filament/spatie-laravel-media-library-plugin`: ^5.6.4 → ^5.6.5 - `guzzlehttp/guzzle`: ^7.10.3 → ^7.10.4 - `driftingly/rector-laravel`: ^2.3.0 → ^2.4.0 (dev) ## Contributor Summary | Contributor | Lines Added | Lines Removed | Files Changed | |---|---|---|---| | BrunoSFreschi | 510+ | 95+ | 40+ | ## Changes Summary | File Path | Change Description | |---|---| | `app-modules/profile/composer.json` | New profile module package with PSR-4 autoloading and service provider registration | | `app-modules/profile/src/Models/Profile.php` | Profile model with tenant relationships, social links validation, enum casting | | `app-modules/profile/database/migrations/2026_05_21_000000_create_user_profiles_table.php` | user_profiles table with UUID PK, composite unique constraint, partial index | | `app-modules/profile/database/factories/ProfileFactory.php` | ProfileFactory with definition and complete states | | `app-modules/profile/src/Enums/SeniorityLevel.php` | Enum for professional levels (Junior-Lead) with Filament integration | | `app-modules/profile/src/Enums/SocialPlatform.php` | Enum for social platforms with icon/label support | | `app-modules/profile/src/Enums/StartAvailability.php` | Enum for job availability with Filament contracts | | `app-modules/profile/src/ProfileServiceProvider.php` | Service provider for migrations, translations, morph maps | | `app-modules/profile/tests/Feature/ProfileCreationTest.php` | Feature tests for automatic profile creation | | `app-modules/profile/tests/Unit/ProfileEnumTest.php` | Unit tests for enum implementations | | `app-modules/profile/lang/en/enums.php`, `pt_BR/enums.php` | Enum label translations | | `app-modules/identity/src/Tenant/Models/TenantUser.php` | Pivot model with ObservedBy attribute | | `app-modules/identity/src/Tenant/Observers/TenantUserObserver.php` | Observer for automatic profile creation | | Multiple console commands (10+ files) | Converted to #[Signature] and #[Description] attributes | | `composer.json` | Updated dependencies including he4rt/profile | <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/he4rt/heartdevs.com/pull/274?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 --> --------- Co-authored-by: danielhe4rt <danielhe4rt@gmail.com>
…rovements (#290) ## Summary - **OAuth error handling**: Gracefully handle token exchange failures and missing credentials instead of crashing with unhandled exceptions - **ConnectionHub UX**: Compact layout for profile sidebar, light/dark theme support, refined disconnect button styling - **Portal improvements**: Add "Área do Usuário" navbar link, Twitch as login provider, fix hero avatar metadata key (`name` vs `username` for legacy/new GitHub records) - **Provider reconnection**: Clear `disconnected_at` when reconnecting an existing provider ## Changes ### Identity module - `OAuthController`: catch `OAuthFlowException` on token exchange, redirect with error notification - `AttachProviderToUser`: clear `disconnected_at` on reconnect - OAuth clients (Discord, GitHub, Twitch): throw `OAuthFlowException` on missing credentials ### Panel App - `ConnectionHub`: compact card layout, dark/light theme support, pill-style disconnect button, timestamp next to title - `LoginPage`: add Twitch OAuth button - Profile wrapper: light mode compatible (`bg-white dark:bg-gray-900`) ### Portal - Navbar: outlined "Área do Usuário" button linking to `/app` - `HeroSection`: support both `username` (new) and `name` (legacy) metadata keys for GitHub avatars ## Test plan - [ ] OAuth login with Discord, GitHub, and Twitch - [ ] Disconnect/reconnect a provider from profile ConnectionHub - [ ] Verify ConnectionHub renders correctly in both light and dark mode - [ ] Portal landing page loads without errors (hero avatars) - [ ] "Área do Usuário" navbar link navigates to `/app` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Description This PR enhances OAuth security and user experience across the platform. It adds comprehensive error handling for OAuth token exchanges, validates provider credentials at initialization, and resets disconnected provider states on reconnection. The ConnectionHub UI is updated with improved light/dark theme support and refined styling, while the portal gains Twitch OAuth support and fixes GitHub avatar metadata handling. The identity module prevents unhandled exceptions by catching OAuth failures with explicit error notifications. ## References - PR `#288`: [OAuth flow refactor with account merge strategy](#288) — Related OAuth improvements - PR `#272`: [Twitch integration with admin panel, tenant-scoped EventSub, and subscription management](#272) — Twitch integration foundation - PR `#289`: [Data migration indexes](#289) — Related database changes ## Dependencies & Requirements No new dependencies were added or updated in this PR. All changes utilize existing framework and module capabilities. ## Contributor Summary | Contributor | Lines Added | Lines Removed | Files Changed | |---|---|---|---| | danielhe4rt | 136 | 45 | 12 | ## Changes Summary | File Path | Change Description | |---|---| | app-modules/identity/src/Auth/Exceptions/OAuthFlowException.php | Added `tokenExchangeFailed()` factory method for formatted OAuth token exchange error messages | | app-modules/identity/src/Auth/Http/Controllers/OAuthController.php | Enhanced error handling for missing OAuth configurations, denied callbacks, and token exchange failures with try/catch blocks and redirects | | app-modules/identity/src/Auth/Actions/AttachProviderToUser.php | Reset `disconnected_at` to null on reconnection to mark providers as active again | | app-modules/integration-discord/src/OAuth/DiscordOAuthClient.php | Added validation for `access_token` presence in Discord token response; throws `OAuthFlowException` on missing token | | app-modules/integration-github/src/IntegrationGithubServiceProvider.php | Added validation for GitHub client credentials with explicit RuntimeException on missing configuration | | app-modules/integration-github/src/OAuth/GitHubOAuthClient.php | Added validation for `access_token` in GitHub token response; throws `OAuthFlowException` on missing token | | app-modules/integration-twitch/src/OAuth/TwitchOAuthClient.php | Added validation for `access_token` in Twitch token response; throws `OAuthFlowException` on missing token | | app-modules/panel-app/resources/views/auth/login.blade.php | Added Twitch OAuth login button alongside Discord and GitHub options | | app-modules/panel-app/resources/views/pages/profile.blade.php | Updated profile card styling with rounded backgrounds, rings, and dark mode variants | | app-modules/portal/resources/views/components/navbar.blade.php | Added responsive "Área do Usuário" link and restructured Discord button for mobile/desktop variants | | app-modules/portal/src/Livewire/HeroSection.php | Updated avatar metadata mapping to support both legacy `name` and new `username` keys for GitHub records | | resources/views/livewire/connection-hub.blade.php | Refined ConnectionHub styling for light/dark theme compatibility, updated disconnect button and permissions section appearance | <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/he4rt/heartdevs.com/pull/290?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 -->
Summary
/api/webhooks/twitch/eventsub/{tenant:slug}), controller resolves tenant via route bindingTest plan
/api/webhooks/twitch/eventsub/{tenant_slug}Description
References
Dependencies & Requirements
Contributor Summary
Changes Summary