fix(db): remove app version placeholders#2301
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 makes channels.version nullable, removes synthetic builtin/unknown app_versions, updates DB schema/types/migrations, adjusts backend queries and handlers to treat missing versions as null, normalizes missing versions client-side, and updates seeds and tests. ChangesNullable channel versions and builtin normalization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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 |
Merging this PR will not alter performance
Comparing Footnotes
|
8f64418 to
3c6b9ca
Compare
3c6b9ca to
ff0b574
Compare
|
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.
🧹 Nitpick comments (1)
tests/stats.test.ts (1)
186-186: ⚡ Quick winUse
it.concurrentfor this new isolated test case.This test uses a unique app ID (
${APP_NAME}.deleted.unknown.${randomUUID().split('-')[0]}) and explicit cleanup in the finally block, making it safe for concurrent execution and aligning with the parallel-test guideline.♻️ Proposed change
- it('does not recreate unknown placeholder rows for missing versions', async () => { + it.concurrent('does not recreate unknown placeholder rows for missing versions', async () => {🤖 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 `@tests/stats.test.ts` at line 186, Replace the non-concurrent test declaration with a concurrent one: change the test block that begins with "it('does not recreate unknown placeholder rows for missing versions'" to use it.concurrent(...) so the isolated test (which uses the unique app ID and has its own cleanup in finally) runs in parallel safely; keep the async signature and existing finally cleanup intact when making the swap.
🤖 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.
Nitpick comments:
In `@tests/stats.test.ts`:
- Line 186: Replace the non-concurrent test declaration with a concurrent one:
change the test block that begins with "it('does not recreate unknown
placeholder rows for missing versions'" to use it.concurrent(...) so the
isolated test (which uses the unique app ID and has its own cleanup in finally)
runs in parallel safely; keep the async signature and existing finally cleanup
intact when making the swap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ba123f1-0b22-474b-8f0b-b1d97d9ca59d
📒 Files selected for processing (31)
cli/src/api/channels.tscli/src/channel/add.tscli/src/types/supabase.types.tscli/src/utils.tssrc/components/tables/BundleTable.vuesrc/components/tables/ChannelTable.vuesrc/pages/app/[app].bundle.[bundle].vuesrc/pages/app/[app].channel.[channel].devices.vuesrc/pages/app/[app].channel.[channel].vuesrc/pages/app/[app].device.[device].vuesrc/services/supabase.tssrc/services/versions.tssrc/types/supabase.types.tssupabase/functions/_backend/files/preview.tssupabase/functions/_backend/plugins/stats.tssupabase/functions/_backend/public/app/demo.tssupabase/functions/_backend/public/channel/post.tssupabase/functions/_backend/triggers/cron_clear_versions.tssupabase/functions/_backend/triggers/on_app_create.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sqlsupabase/seed.sqlsupabase/tests/18_test_utility_functions_extended.sqlsupabase/tests/33_test_rbac_phase1.sqltests/bundle.test.tstests/channel-post.unit.test.tstests/cli-channel.test.tstests/stats.test.tstests/updates.test.ts
💤 Files with no reviewable changes (2)
- src/pages/app/[app].device.[device].vue
- supabase/functions/_backend/triggers/on_app_create.ts
ff0b574 to
575dfde
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/pages/app/`[app].channel.[channel].vue:
- Around line 280-282: The handler (the dialog action that calls
saveChannelChange('version', null)) is missing an await; update the anonymous
async handler to await saveChannelChange('version', null) so the save finishes
before the dialog resolves—mirror how handleRevert awaits saveChannelChange to
ensure consistency and reliable ordering (look for saveChannelChange and the
handleRevert handler for reference).
🪄 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: 76e9d738-c6e5-4250-b7a1-fa7dc14f94a3
📒 Files selected for processing (31)
cli/src/api/channels.tscli/src/channel/add.tscli/src/types/supabase.types.tscli/src/utils.tssrc/components/tables/BundleTable.vuesrc/components/tables/ChannelTable.vuesrc/pages/app/[app].bundle.[bundle].vuesrc/pages/app/[app].channel.[channel].devices.vuesrc/pages/app/[app].channel.[channel].vuesrc/pages/app/[app].device.[device].vuesrc/services/supabase.tssrc/services/versions.tssrc/types/supabase.types.tssupabase/functions/_backend/files/preview.tssupabase/functions/_backend/plugins/stats.tssupabase/functions/_backend/public/app/demo.tssupabase/functions/_backend/public/channel/post.tssupabase/functions/_backend/triggers/cron_clear_versions.tssupabase/functions/_backend/triggers/on_app_create.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sqlsupabase/seed.sqlsupabase/tests/18_test_utility_functions_extended.sqlsupabase/tests/33_test_rbac_phase1.sqltests/bundle.test.tstests/channel-post.unit.test.tstests/cli-channel.test.tstests/stats.test.tstests/updates.test.ts
💤 Files with no reviewable changes (2)
- src/pages/app/[app].device.[device].vue
- supabase/functions/_backend/triggers/on_app_create.ts
✅ Files skipped from review due to trivial changes (2)
- supabase/tests/33_test_rbac_phase1.sql
- src/types/supabase.types.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- supabase/functions/_backend/plugins/stats.ts
- tests/cli-channel.test.ts
- src/services/supabase.ts
- tests/bundle.test.ts
- supabase/functions/_backend/triggers/cron_clear_versions.ts
- supabase/functions/_backend/utils/postgres_schema.ts
- cli/src/channel/add.ts
- tests/stats.test.ts
- tests/channel-post.unit.test.ts
- cli/src/utils.ts
- supabase/functions/_backend/utils/supabase.types.ts
- src/pages/app/[app].bundle.[bundle].vue
- src/pages/app/[app].channel.[channel].devices.vue
- supabase/functions/_backend/public/app/demo.ts
- cli/src/types/supabase.types.ts
- cli/src/api/channels.ts
- supabase/functions/_backend/public/channel/post.ts
- tests/updates.test.ts
- supabase/functions/_backend/files/preview.ts
- src/services/versions.ts
- src/components/tables/BundleTable.vue
- supabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sql
- src/components/tables/ChannelTable.vue
- supabase/seed.sql
575dfde to
560d9c9
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/app/[app].bundle.[bundle].vue (1)
224-247:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
setChannel()throw on failed updates.This returns the raw Supabase mutation without error propagation, so DB failures resolve silently instead of hitting the
catchblocks. Callers at lines ~335 and ~395 show success toasts even when the channel update fails.In
@supabase/supabase-jsv2,.update(...).eq(...)resolves with{ data, error }and does not reject by default; add.throwOnError()to enable proper error handling.Suggested fix
async function setChannel(channel: Database['public']['Tables']['channels']['Row'], id: number | null) { if (!canPromoteChannel(channel.id)) { toast.error(t('no-permission')) return Promise.reject(new Error('No permission')) } @@ return supabase .from('channels') .update({ version: id, }) .eq('id', channel.id) + .throwOnError() }🤖 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 `@src/pages/app/`[app].bundle.[bundle].vue around lines 224 - 247, The setChannel function currently returns the raw Supabase update promise which resolves to {data, error} and won’t throw on DB failures, causing callers (e.g., where setChannel is invoked around the success-toasts) to miss errors; update the supabase call in setChannel (the chain starting with supabase.from('channels').update({version: id}).eq('id', channel.id)) to use .throwOnError() so that failed updates throw and propagate to the callers' catch blocks, ensuring errors trigger the existing error handling paths.
🤖 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.
Outside diff comments:
In `@src/pages/app/`[app].bundle.[bundle].vue:
- Around line 224-247: The setChannel function currently returns the raw
Supabase update promise which resolves to {data, error} and won’t throw on DB
failures, causing callers (e.g., where setChannel is invoked around the
success-toasts) to miss errors; update the supabase call in setChannel (the
chain starting with supabase.from('channels').update({version: id}).eq('id',
channel.id)) to use .throwOnError() so that failed updates throw and propagate
to the callers' catch blocks, ensuring errors trigger the existing error
handling paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f277bf9e-a311-4bd5-9528-ca6fb164af65
📒 Files selected for processing (31)
cli/src/api/channels.tscli/src/channel/add.tscli/src/types/supabase.types.tscli/src/utils.tssrc/components/tables/BundleTable.vuesrc/components/tables/ChannelTable.vuesrc/pages/app/[app].bundle.[bundle].vuesrc/pages/app/[app].channel.[channel].devices.vuesrc/pages/app/[app].channel.[channel].vuesrc/pages/app/[app].device.[device].vuesrc/services/supabase.tssrc/services/versions.tssrc/types/supabase.types.tssupabase/functions/_backend/files/preview.tssupabase/functions/_backend/plugins/stats.tssupabase/functions/_backend/public/app/demo.tssupabase/functions/_backend/public/channel/post.tssupabase/functions/_backend/triggers/cron_clear_versions.tssupabase/functions/_backend/triggers/on_app_create.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sqlsupabase/seed.sqlsupabase/tests/18_test_utility_functions_extended.sqlsupabase/tests/33_test_rbac_phase1.sqltests/bundle.test.tstests/channel-post.unit.test.tstests/cli-channel.test.tstests/stats.test.tstests/updates.test.ts
💤 Files with no reviewable changes (2)
- src/pages/app/[app].device.[device].vue
- supabase/functions/_backend/triggers/on_app_create.ts
✅ Files skipped from review due to trivial changes (1)
- cli/src/types/supabase.types.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- tests/channel-post.unit.test.ts
- supabase/tests/33_test_rbac_phase1.sql
- supabase/functions/_backend/plugins/stats.ts
- cli/src/utils.ts
- supabase/functions/_backend/public/channel/post.ts
- src/services/supabase.ts
- supabase/tests/18_test_utility_functions_extended.sql
- supabase/functions/_backend/files/preview.ts
- tests/updates.test.ts
- tests/stats.test.ts
- supabase/functions/_backend/public/app/demo.ts
- tests/bundle.test.ts
- supabase/functions/_backend/utils/postgres_schema.ts
- cli/src/channel/add.ts
- supabase/functions/_backend/triggers/cron_clear_versions.ts
- src/types/supabase.types.ts
- supabase/functions/_backend/utils/supabase.types.ts
- src/components/tables/BundleTable.vue
- src/pages/app/[app].channel.[channel].devices.vue
- src/services/versions.ts
- src/components/tables/ChannelTable.vue
- supabase/seed.sql
- cli/src/api/channels.ts
- supabase/functions/_backend/utils/pg.ts
- src/pages/app/[app].channel.[channel].vue
- supabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sql
560d9c9 to
85e4e2b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/tables/ChannelTable.vue`:
- Line 301: Replace the hardcoded fallback string 'builtin' in the
displayFunction (the arrow function named displayFunction in ChannelTable.vue)
with a translation key lookup (e.g., use the component's i18n
t('channels.builtin') call) so the table uses localized text; add the
corresponding key "channels.builtin" to your locale messages (messages/en.json)
and do not use an inline fallback string as a second argument to t().
🪄 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: 595b4b61-0c01-46d0-951c-560acbd20f6a
📒 Files selected for processing (31)
cli/src/api/channels.tscli/src/channel/add.tscli/src/types/supabase.types.tscli/src/utils.tssrc/components/tables/BundleTable.vuesrc/components/tables/ChannelTable.vuesrc/pages/app/[app].bundle.[bundle].vuesrc/pages/app/[app].channel.[channel].devices.vuesrc/pages/app/[app].channel.[channel].vuesrc/pages/app/[app].device.[device].vuesrc/services/supabase.tssrc/services/versions.tssrc/types/supabase.types.tssupabase/functions/_backend/files/preview.tssupabase/functions/_backend/plugins/stats.tssupabase/functions/_backend/public/app/demo.tssupabase/functions/_backend/public/channel/post.tssupabase/functions/_backend/triggers/cron_clear_versions.tssupabase/functions/_backend/triggers/on_app_create.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sqlsupabase/seed.sqlsupabase/tests/18_test_utility_functions_extended.sqlsupabase/tests/33_test_rbac_phase1.sqltests/bundle.test.tstests/channel-post.unit.test.tstests/cli-channel.test.tstests/stats.test.tstests/updates.test.ts
💤 Files with no reviewable changes (2)
- src/pages/app/[app].device.[device].vue
- supabase/functions/_backend/triggers/on_app_create.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- tests/cli-channel.test.ts
- cli/src/utils.ts
- supabase/tests/18_test_utility_functions_extended.sql
- tests/channel-post.unit.test.ts
- supabase/functions/_backend/plugins/stats.ts
- src/services/supabase.ts
- supabase/functions/_backend/triggers/cron_clear_versions.ts
- supabase/functions/_backend/utils/postgres_schema.ts
- supabase/functions/_backend/utils/supabase.types.ts
- supabase/tests/33_test_rbac_phase1.sql
- src/types/supabase.types.ts
- tests/bundle.test.ts
- cli/src/channel/add.ts
- supabase/functions/_backend/files/preview.ts
- supabase/functions/_backend/public/app/demo.ts
- src/components/tables/BundleTable.vue
- supabase/functions/_backend/public/channel/post.ts
- src/services/versions.ts
- src/pages/app/[app].channel.[channel].devices.vue
- cli/src/types/supabase.types.ts
- cli/src/api/channels.ts
- src/pages/app/[app].channel.[channel].vue
- src/pages/app/[app].bundle.[bundle].vue
- supabase/functions/_backend/utils/pg.ts
- supabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sql
- supabase/seed.sql
85e4e2b to
63c4cbe
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/app/[app].bundle.[bundle].vue (1)
744-744: 💤 Low valueLocalize the 'builtin' fallback for i18n consistency.
The hardcoded
'builtin'string in the dialog description bypasses localization. For consistency withChannelTable.vue(which usest('channel-builtin')), this should also use a translation key.Proposed fix
- channels: channelFound.map((ch: any) => `${ch.name} (${ch.version?.name ?? 'builtin'})`).join(', '), + channels: channelFound.map((ch: any) => `${ch.name} (${ch.version?.name ?? t('channel-builtin')})`).join(', '),🤖 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 `@src/pages/app/`[app].bundle.[bundle].vue at line 744, Replace the hardcoded fallback 'builtin' in the channels mapping with the i18n translation (e.g. t('channel-builtin')), i.e. change `${ch.name} (${ch.version?.name ?? 'builtin'})` to use the translator; ensure the component provides a t function (import/use useI18n and call t(...) in setup or use this.$t(...) in options API) so the mapping becomes `${ch.name} (${ch.version?.name ?? t('channel-builtin')})`, referencing the existing channelFound variable and the channels property in this component.
🤖 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.
Nitpick comments:
In `@src/pages/app/`[app].bundle.[bundle].vue:
- Line 744: Replace the hardcoded fallback 'builtin' in the channels mapping
with the i18n translation (e.g. t('channel-builtin')), i.e. change `${ch.name}
(${ch.version?.name ?? 'builtin'})` to use the translator; ensure the component
provides a t function (import/use useI18n and call t(...) in setup or use
this.$t(...) in options API) so the mapping becomes `${ch.name}
(${ch.version?.name ?? t('channel-builtin')})`, referencing the existing
channelFound variable and the channels property in this component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e075038e-8ef8-45b9-b547-570c354f2eb8
📒 Files selected for processing (32)
cli/src/api/channels.tscli/src/channel/add.tscli/src/types/supabase.types.tscli/src/utils.tsmessages/en.jsonsrc/components/tables/BundleTable.vuesrc/components/tables/ChannelTable.vuesrc/pages/app/[app].bundle.[bundle].vuesrc/pages/app/[app].channel.[channel].devices.vuesrc/pages/app/[app].channel.[channel].vuesrc/pages/app/[app].device.[device].vuesrc/services/supabase.tssrc/services/versions.tssrc/types/supabase.types.tssupabase/functions/_backend/files/preview.tssupabase/functions/_backend/plugins/stats.tssupabase/functions/_backend/public/app/demo.tssupabase/functions/_backend/public/channel/post.tssupabase/functions/_backend/triggers/cron_clear_versions.tssupabase/functions/_backend/triggers/on_app_create.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sqlsupabase/seed.sqlsupabase/tests/18_test_utility_functions_extended.sqlsupabase/tests/33_test_rbac_phase1.sqltests/bundle.test.tstests/channel-post.unit.test.tstests/cli-channel.test.tstests/stats.test.tstests/updates.test.ts
💤 Files with no reviewable changes (2)
- src/pages/app/[app].device.[device].vue
- supabase/functions/_backend/triggers/on_app_create.ts
✅ Files skipped from review due to trivial changes (1)
- messages/en.json
🚧 Files skipped from review as they are similar to previous changes (26)
- supabase/tests/33_test_rbac_phase1.sql
- tests/cli-channel.test.ts
- cli/src/utils.ts
- supabase/tests/18_test_utility_functions_extended.sql
- supabase/functions/_backend/triggers/cron_clear_versions.ts
- supabase/functions/_backend/plugins/stats.ts
- supabase/functions/_backend/utils/postgres_schema.ts
- supabase/functions/_backend/public/app/demo.ts
- tests/channel-post.unit.test.ts
- src/components/tables/BundleTable.vue
- supabase/functions/_backend/files/preview.ts
- src/types/supabase.types.ts
- cli/src/types/supabase.types.ts
- src/services/versions.ts
- supabase/functions/_backend/utils/supabase.types.ts
- cli/src/channel/add.ts
- cli/src/api/channels.ts
- tests/updates.test.ts
- supabase/functions/_backend/public/channel/post.ts
- src/pages/app/[app].channel.[channel].devices.vue
- supabase/functions/_backend/utils/pg.ts
- tests/bundle.test.ts
- src/pages/app/[app].channel.[channel].vue
- tests/stats.test.ts
- supabase/migrations/20260519151250_remove_builtin_unknown_app_versions.sql
- supabase/seed.sql
63c4cbe to
a2af008
Compare
…ion-placeholders # Conflicts: # supabase/functions/_backend/public/app/demo.ts
|



Summary
AI-Generated: Codex prepared this section.
unknownandbuiltinplaceholder rows fromapp_versions.channels.version = NULLwithON DELETE SET NULL.Motivation
AI-Generated: Codex prepared this section.
The placeholder rows were coming back from several writers: app creation, stats fallback, CLI/frontend unlink flows, demo app creation, seed helpers, and legacy SQL helpers. This PR removes those writers and migrates existing references away from placeholder rows.
Business Impact
AI-Generated: Codex prepared this section.
This keeps customer bundle lists and app version history clean, avoids confusing
unknown/builtinentries reappearing after cleanup, and preserves native rollback behavior through a nullable channel version instead of fake bundle rows.Test Plan
AI-Generated: Codex prepared this section.
bun lintbun lint:backendbun run cli:lintbun typecheckbunx vitest run tests/channel-post.unit.test.tsbun run supabase:db:resetbun run supabase:with-env -- bunx vitest run tests/updates.test.ts tests/stats.test.ts tests/bundle.test.ts tests/cli-channel.test.tsbun test:backendapp_versionsrows namedunknownorbuiltinafter resetNULLand no placeholder rows are recreatedSummary by CodeRabbit
New Features
Bug Fixes
Refactoring