fix(channel): enforce public channel uniqueness#1945
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:
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/public-channel-uniqueness.test.ts (2)
143-174: Useit.concurrent()for parallel test execution.Both test cases use unique
appIdvalues generated withrandomUUID(), so they have no shared state dependencies. Per coding guidelines, useit.concurrent()to maximize parallelism and speed up CI/CD.♻️ Proposed fix to enable concurrent tests
- it('demotes overlapping public channels on insert while preserving other platforms', async () => { + it.concurrent('demotes overlapping public channels on insert while preserving other platforms', async () => {- it('demotes overlapping public channels on update while preserving other platforms', async () => { + it.concurrent('demotes overlapping public channels on update while preserving other platforms', async () => {As per coding guidelines: "Use
it.concurrent()instead ofit()when possible to run tests in parallel within the same file, maximizing parallelism for faster CI/CD."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/public-channel-uniqueness.test.ts` around lines 143 - 174, The test uses it(...) but should run in parallel per guidelines; change the test declaration to use it.concurrent(...) for the test titled 'demotes overlapping public channels on insert while preserving other platforms', keeping the same body that calls createAppFixture, insertChannel and getChannelStates (and uses randomUUID for appId/channel names) so the unique appId remains; ensure no shared state is introduced and run the spec with it.concurrent to enable parallel execution.
142-216: Consider adding test coverage for Electron platform.The migration and TypeScript trigger explicitly handle Electron as a separate platform with its own unique index (
channels_one_public_electron_per_app_key), but the test suite only covers iOS and Android scenarios. Adding an Electron-focused test would ensure parity and catch any platform-specific regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/public-channel-uniqueness.test.ts` around lines 142 - 216, Add a test (or two) mirroring the existing iOS/Android cases to cover the Electron platform: use createAppFixture to make a test app, create unique channel names and call insertChannel with public: true and electron: true (and ios/android false) as well as another overlapping platform channel, then call getChannelStates (or perform an UPDATE via executeSQL for the update case) and assert that the older Electron public channel is demoted while other platforms remain unchanged; reference insertChannel, getChannelStates, createAppFixture and executeSQL to locate where to add the Electron-focused assertions.supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql (1)
11-19: Advisory lock acquired before public-channel check may cause unnecessary serialization.The lock is taken unconditionally on line 15, before checking
NEW.publicon line 17. This means all channel writes for the same app (including non-public updates) will serialize, even when they don't affect public-channel state.If this is intentional for safety, consider adding a brief comment. Otherwise, moving the early return before the lock would reduce contention for high-volume apps:
♻️ Optional: Move early return before lock acquisition
BEGIN + -- Skip lock acquisition for non-public channel changes + IF NEW.public IS DISTINCT FROM true THEN + RETURN NEW; + END IF; + -- Serialize public-channel changes per app so concurrent writers cannot -- reintroduce overlapping public state between the normalization update and -- the row write itself. PERFORM pg_advisory_xact_lock(hashtext(NEW.app_id)); - - IF NEW.public IS DISTINCT FROM true THEN - RETURN NEW; - END IF;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql` around lines 11 - 19, The code currently acquires an advisory lock via pg_advisory_xact_lock(hashtext(NEW.app_id)) before checking NEW.public, causing all writes for an app to serialize; either move the early-return IF NEW.public IS DISTINCT FROM true THEN RETURN NEW; so the routine returns before taking the lock (placing the IF directly above the lock) or, if unconditional locking is intentional, add a concise comment next to pg_advisory_xact_lock explaining why non-public writes must be serialized; update the block around the lock and the IF (referencing NEW.public and pg_advisory_xact_lock(hashtext(NEW.app_id))) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql`:
- Around line 11-19: The code currently acquires an advisory lock via
pg_advisory_xact_lock(hashtext(NEW.app_id)) before checking NEW.public, causing
all writes for an app to serialize; either move the early-return IF NEW.public
IS DISTINCT FROM true THEN RETURN NEW; so the routine returns before taking the
lock (placing the IF directly above the lock) or, if unconditional locking is
intentional, add a concise comment next to pg_advisory_xact_lock explaining why
non-public writes must be serialized; update the block around the lock and the
IF (referencing NEW.public and pg_advisory_xact_lock(hashtext(NEW.app_id)))
accordingly.
In `@tests/public-channel-uniqueness.test.ts`:
- Around line 143-174: The test uses it(...) but should run in parallel per
guidelines; change the test declaration to use it.concurrent(...) for the test
titled 'demotes overlapping public channels on insert while preserving other
platforms', keeping the same body that calls createAppFixture, insertChannel and
getChannelStates (and uses randomUUID for appId/channel names) so the unique
appId remains; ensure no shared state is introduced and run the spec with
it.concurrent to enable parallel execution.
- Around line 142-216: Add a test (or two) mirroring the existing iOS/Android
cases to cover the Electron platform: use createAppFixture to make a test app,
create unique channel names and call insertChannel with public: true and
electron: true (and ios/android false) as well as another overlapping platform
channel, then call getChannelStates (or perform an UPDATE via executeSQL for the
update case) and assert that the older Electron public channel is demoted while
other platforms remain unchanged; reference insertChannel, getChannelStates,
createAppFixture and executeSQL to locate where to add the Electron-focused
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3191eac-608e-4046-9786-c98257c4cc33
📒 Files selected for processing (3)
supabase/functions/_backend/triggers/on_channel_update.tssupabase/migrations/20260424090854_enforce_public_channel_uniqueness.sqltests/public-channel-uniqueness.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 596b44596a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7e0fc034f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18c6af3e72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql (1)
36-37: Remove unnecessary REVOKE on trigger function (optional cleanup).For
RETURNS triggerfunctions, explicitREVOKE ALL ... FROM PUBLICis typically unnecessary because they are not directly callable in normal usage.♻️ Proposed cleanup
ALTER FUNCTION public.normalize_public_channel_overlap() OWNER TO "postgres"; -REVOKE ALL ON FUNCTION public.normalize_public_channel_overlap() FROM PUBLIC;Based on learnings: "In supabase/migrations/**/*.sql ... do not require REVOKE ALL ON FUNCTION ... FROM PUBLIC for PostgreSQL trigger functions (RETURNS trigger)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql` around lines 36 - 37, Remove the unnecessary REVOKE ALL ON FUNCTION public.normalize_public_channel_overlap() FROM PUBLIC line: locate the trigger function declaration for normalize_public_channel_overlap and delete the explicit REVOKE statement since trigger functions (RETURNS trigger) aren't directly callable by PUBLIC and do not require that revoke; leave any other grants/revokes for non-trigger functions intact or add a brief comment if you want to keep the line for historical reasons.tests/public-channel-uniqueness.test.ts (1)
145-316: Run independent cases withit.concurrent()to match test guideline.These tests generate isolated app IDs and don’t depend on each other’s state, so they can be parallelized.
⚡ Suggested change
- it('uses the most recently updated public electron channel as the deterministic winner', async () => { + it.concurrent('uses the most recently updated public electron channel as the deterministic winner', async () => { @@ - it('allows one public iOS channel and one public Android channel when both keep electron enabled', async () => { + it.concurrent('allows one public iOS channel and one public Android channel when both keep electron enabled', async () => { @@ - it('demotes overlapping public channels on insert while preserving other platforms', async () => { + it.concurrent('demotes overlapping public channels on insert while preserving other platforms', async () => { @@ - it('demotes overlapping public channels on update while preserving other platforms', async () => { + it.concurrent('demotes overlapping public channels on update while preserving other platforms', async () => {As per coding guidelines: "Use
it.concurrent()instead ofit()when possible to run tests in parallel within the same file, maximizing parallelism for faster CI/CD".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/public-channel-uniqueness.test.ts` around lines 145 - 316, The tests in this file are independent and should be run in parallel; replace the four top-level it(...) calls (the tests with names starting "uses the most recently updated public electron channel as the deterministic winner", "allows one public iOS channel and one public Android channel when both keep electron enabled", "demotes overlapping public channels on insert while preserving other platforms", and "demotes overlapping public channels on update while preserving other platforms") with it.concurrent(...) so Jest runs them concurrently; update each test declaration that calls insertChannel/createAppFixture/getChannelStates/requestInfosChannelPostgres to use it.concurrent and keep the test bodies unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql`:
- Line 15: The PERFORM call uses unqualified built-ins inside a function with
search_path = '' — update the call to use fully qualified pg_catalog names:
replace uses of pg_advisory_xact_lock and hashtext with
pg_catalog.pg_advisory_xact_lock and pg_catalog.hashtext so the function
references are explicit and safe when search_path is empty (locate the PERFORM
statement that references NEW.app_id).
- Around line 38-43: The async fallback in the on_channel_update handler
currently de-publicizes peers without re-validating the triggering record;
update that fallback (in the on_channel_update async function) to re-fetch the
latest row for the triggering id, confirm that the row still has public = true,
and verify it is still the active winner according to the same ordering logic
used by normalize_public_channel_overlap() (compare updated_at, created_at, id
in that priority); only proceed to de-publicize peers if both checks pass,
otherwise abort.
---
Nitpick comments:
In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql`:
- Around line 36-37: Remove the unnecessary REVOKE ALL ON FUNCTION
public.normalize_public_channel_overlap() FROM PUBLIC line: locate the trigger
function declaration for normalize_public_channel_overlap and delete the
explicit REVOKE statement since trigger functions (RETURNS trigger) aren't
directly callable by PUBLIC and do not require that revoke; leave any other
grants/revokes for non-trigger functions intact or add a brief comment if you
want to keep the line for historical reasons.
In `@tests/public-channel-uniqueness.test.ts`:
- Around line 145-316: The tests in this file are independent and should be run
in parallel; replace the four top-level it(...) calls (the tests with names
starting "uses the most recently updated public electron channel as the
deterministic winner", "allows one public iOS channel and one public Android
channel when both keep electron enabled", "demotes overlapping public channels
on insert while preserving other platforms", and "demotes overlapping public
channels on update while preserving other platforms") with it.concurrent(...) so
Jest runs them concurrently; update each test declaration that calls
insertChannel/createAppFixture/getChannelStates/requestInfosChannelPostgres to
use it.concurrent and keep the test bodies unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: edc02898-6f82-4713-818e-d799defab5ce
📒 Files selected for processing (4)
supabase/functions/_backend/triggers/on_channel_update.tssupabase/functions/_backend/utils/pg.tssupabase/migrations/20260424090854_enforce_public_channel_uniqueness.sqltests/public-channel-uniqueness.test.ts
💤 Files with no reviewable changes (1)
- supabase/functions/_backend/triggers/on_channel_update.ts
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql (1)
36-36: Consider droppingREVOKEon this trigger function to reduce permission-noise.For
RETURNS triggerfunctions, explicitREVOKE ALL ... FROM PUBLICis usually unnecessary since they are not directly callable by client roles.Based on learnings: "In supabase/migrations/**/*.sql ... do not require REVOKE ALL ON FUNCTION ... FROM PUBLIC for PostgreSQL trigger functions (RETURNS trigger)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql` at line 36, The REVOKE ALL ON FUNCTION public.normalize_public_channel_overlap() FROM PUBLIC line is unnecessary for a trigger function (RETURNS trigger) and adds permission noise; remove that REVOKE statement referencing public.normalize_public_channel_overlap() from the migration so the trigger function creation is kept minimal, and ensure no other unintended permission changes are present for that function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql`:
- Around line 21-30: The migration's UPDATE only checks ios/android overlaps and
must also include Electron so add an OR branch for electron in the
public.channels UPDATE predicates (e.g., include "OR (NEW.electron = true AND
existing.electron = true)" alongside the ios/android checks in every similar
UPDATE block). Also update the application-level demotion logic in
on_channel_update.ts to treat electron like ios/android: include NEW.electron
checks when selecting/demoting other public channels so Electron-only channels
can't produce concurrent public winners.
---
Nitpick comments:
In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql`:
- Line 36: The REVOKE ALL ON FUNCTION public.normalize_public_channel_overlap()
FROM PUBLIC line is unnecessary for a trigger function (RETURNS trigger) and
adds permission noise; remove that REVOKE statement referencing
public.normalize_public_channel_overlap() from the migration so the trigger
function creation is kept minimal, and ensure no other unintended permission
changes are present for that function.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4ea8223-be8d-4ba0-a0ed-3c640c075a8b
📒 Files selected for processing (2)
supabase/functions/_backend/triggers/on_channel_update.tssupabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- supabase/functions/_backend/triggers/on_channel_update.ts
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/public-channel-uniqueness.test.ts (1)
143-264: Run these cases withit.concurrent().These tests already isolate themselves with unique app ids, so they look like good candidates for parallel execution and faster CI.
As per coding guidelines, "Use
it.concurrent()instead ofit()when possible to run tests in parallel within the same file, maximizing parallelism for faster CI/CD".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/public-channel-uniqueness.test.ts` around lines 143 - 264, Change the three synchronous tests to run concurrently by replacing the calls to it(...) with it.concurrent(...); specifically update the tests titled "allows one public channel per platform", "demotes overlapping public electron channels on insert while preserving other platforms", and "demotes overlapping public electron channels on update while preserving other platforms" so they use it.concurrent to enable parallel execution (these tests already use unique app ids and isolated fixtures).supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql (1)
36-38: Skip theREVOKEon the trigger function.For
RETURNS triggerfunctions, this repo's SQL guidance treats the extraREVOKE ALL ... FROM PUBLICas unnecessary. I'd drop it here and keep privilege hardening focused on callable functions.Based on learnings, "In supabase/migrations/**/*.sql for Cap-go/capgo, when auditing function permissions, do not require REVOKE ALL ON FUNCTION ... FROM PUBLIC for PostgreSQL trigger functions (RETURNS trigger). Triggered functions cannot be called directly by roles. Apply REVOKE only to regular (callable) functions, not to trigger functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql` around lines 36 - 38, Drop the unnecessary REVOKE for the trigger function: keep the ALTER FUNCTION public.normalize_public_channel_overlap() OWNER TO "postgres"; but remove the REVOKE ALL ON FUNCTION public.normalize_public_channel_overlap() FROM PUBLIC line because normalize_public_channel_overlap is a RETURNS trigger function and trigger functions should not have a REVOKE applied here (only apply privilege hardening to regular/callable functions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/triggers/on_channel_update.ts`:
- Around line 60-93: The check uses the reloaded currentRecord (via
getCurrentChannel) but later cleanup/update calls still use the webhook
snapshot's record.app_id; change all follow-up updates that demote peers to use
currentRecord.app_id instead of record.app_id (e.g., in isCurrentPublicWinner
flow and the similar cleanup logic later) so the async fallback updates target
the reloaded row's app_id when resolving/demoting public winners.
---
Nitpick comments:
In `@supabase/migrations/20260424090854_enforce_public_channel_uniqueness.sql`:
- Around line 36-38: Drop the unnecessary REVOKE for the trigger function: keep
the ALTER FUNCTION public.normalize_public_channel_overlap() OWNER TO
"postgres"; but remove the REVOKE ALL ON FUNCTION
public.normalize_public_channel_overlap() FROM PUBLIC line because
normalize_public_channel_overlap is a RETURNS trigger function and trigger
functions should not have a REVOKE applied here (only apply privilege hardening
to regular/callable functions).
In `@tests/public-channel-uniqueness.test.ts`:
- Around line 143-264: Change the three synchronous tests to run concurrently by
replacing the calls to it(...) with it.concurrent(...); specifically update the
tests titled "allows one public channel per platform", "demotes overlapping
public electron channels on insert while preserving other platforms", and
"demotes overlapping public electron channels on update while preserving other
platforms" so they use it.concurrent to enable parallel execution (these tests
already use unique app ids and isolated fixtures).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f363cd17-d3be-4c7f-ab40-0bee90cd4aaf
📒 Files selected for processing (5)
supabase/functions/_backend/triggers/on_channel_update.tssupabase/migrations/20260424090854_enforce_public_channel_uniqueness.sqlsupabase/seed.sqlsupabase/tests/35_test_deploy_install_stats_email.sqltests/public-channel-uniqueness.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
supabase/functions/_backend/triggers/on_channel_update.ts (1)
109-155: Previous review concern addressed; code is correct.The demotion logic now correctly uses
currentWinner.app_id(lines 117, 133, 149) instead of the webhook snapshot'srecord.app_id, ensuring that if the channel was moved to another app before this async fallback runs, the demotion targets the correct app.The addition of electron support (lines 141-155) aligns with the database-level partial unique index
channels_one_public_electron_per_app_key.Optional: The three platform blocks share identical structure. If this pattern grows or needs maintenance, consider extracting a helper:
♻️ Optional refactor to reduce duplication
const scopes: ChannelPlatformScope[] = ['ios', 'android', 'electron'] for (const scope of scopes) { if (record.public && record[scope]) { const currentWinner = await getCurrentPublicWinner(c, record, scope) if (currentWinner) { await updateChannelsWithRetry( c, async () => await supabaseAdmin(c) .from('channels') .update({ public: false }) .eq('app_id', currentWinner.app_id) .eq(scope, true) .neq('id', record.id), { app_id: currentWinner.app_id, record_id: record.id, scope }, ) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/triggers/on_channel_update.ts` around lines 109 - 155, Replace the three nearly identical platform-specific blocks with a single loop over scopes to reduce duplication: iterate over scopes = ['ios','android','electron'] (type ChannelPlatformScope), and for each scope check if record.public && record[scope], call getCurrentPublicWinner(c, record, scope), and if it returns a winner call updateChannelsWithRetry with the same async supabaseAdmin(...) update but using .eq(scope, true) and the same metadata object { app_id: currentWinner.app_id, record_id: record.id, scope }; keep references to getCurrentPublicWinner, updateChannelsWithRetry, supabaseAdmin and record, ensuring TypeScript indexing on record[scope] is type-safe (cast to the appropriate key type if needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@supabase/functions/_backend/triggers/on_channel_update.ts`:
- Around line 109-155: Replace the three nearly identical platform-specific
blocks with a single loop over scopes to reduce duplication: iterate over scopes
= ['ios','android','electron'] (type ChannelPlatformScope), and for each scope
check if record.public && record[scope], call getCurrentPublicWinner(c, record,
scope), and if it returns a winner call updateChannelsWithRetry with the same
async supabaseAdmin(...) update but using .eq(scope, true) and the same metadata
object { app_id: currentWinner.app_id, record_id: record.id, scope }; keep
references to getCurrentPublicWinner, updateChannelsWithRetry, supabaseAdmin and
record, ensuring TypeScript indexing on record[scope] is type-safe (cast to the
appropriate key type if needed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e75e87d1-e664-4182-871d-a01d4d58c8a7
📒 Files selected for processing (1)
supabase/functions/_backend/triggers/on_channel_update.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d7c153162
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 795b8b8c03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
1 participant
Summary (AI generated)
on_channel_updatefallback with the same overlap rules and add a direct Postgres regression testMotivation (AI generated)
The GHSA-3cmp-pm5x-8464 advisory shows that overlapping public channels could coexist long enough for unnamed update routing to silently pick an implicit winner. The fix needs to happen at the write boundary, not only in asynchronous cleanup, so the database never persists ambiguous public-channel state.
Business Impact (AI generated)
This removes an OTA release-routing integrity issue that could make production update behavior unpredictable for customers. It reduces the risk of shipping devices from the wrong channel and improves trust in default channel behavior.
Test Plan (AI generated)
bun run lint:backendbunx eslint tests/public-channel-uniqueness.test.tsbun run supabase:with-env -- bunx vitest run tests/public-channel-uniqueness.test.tsGenerated with AI
Summary by CodeRabbit