Conversation
Implement complete profile system with JWT auth, caching, React Query hooks, ProfileCard component, settings page, and menu avatar with user initials. Includes real-time updates and error handling.
# Conflicts: # src/main/utils/constants.ts
…dio into feature/authentication
…dio into feature/authentication
…dio into feature/authentication
…port
- Add environment prop ('local' | 'cloud') to ModelSplitButton and ProjectDbtSplitButton
- Filter menu items based on environment:
- Cloud mode: hide local-only commands (Rosetta layers, debug, docs generation/serving, compile, preview)
- Local mode: show all commands (existing behavior)
- Integrate PushToCloudModal for cloud execution:
- ModelSplitButton: populate dbt arguments with model selection patterns (--select model, +model, model+, +model+)
- ProjectDbtSplitButton: use existing cloud modal integration
- Update PushToCloudModal to accept initialDbtArguments prop
- Pass environment setting from ProjectDetails to both split buttons
- Maintain backward compatibility with default 'local' environment
Commands available in cloud mode:
- Production dbt: run, test, build, compile, clean, deps, seed
- Model operations: all run/test/build variants with dependency selection
Commands hidden in cloud mode:
- Rosetta layer generation (raw, staging, incremental, business)
- Local development tools (debug, serve docs, generate docs, compile, preview)
# Conflicts: # src/main/services/index.ts # src/renderer/config/constants.ts
# Conflicts: # src/renderer/components/dbtModelButtons/ProjectDbtSplitButton.tsx # src/renderer/components/settings/GeneralSettings.tsx
# Conflicts: # src/main/ipcHandlers/index.ts # src/main/ipcSetup.ts # src/main/main.ts # src/main/services/index.ts # src/renderer/App.tsx # src/renderer/components/settings/GeneralSettings.tsx # src/renderer/controllers/index.ts # src/renderer/screens/settings/index.tsx # src/renderer/screens/settings/settingsElements.tsx # src/renderer/services/index.ts # src/types/backend.ts # src/types/ipc.ts
# Conflicts: # src/main/ipcHandlers/git.ipcHandlers.ts # src/main/services/git.service.ts # src/renderer/components/menu/index.tsx # src/renderer/config/constants.ts # src/renderer/screens/projectDetails/index.tsx # src/renderer/services/git.service.ts # src/types/ipc.ts
📝 WalkthroughWalkthroughThis pull request introduces comprehensive Rosetta Cloud integration with API key-based authentication, cloud execution support for DBT commands, environment switching (local/cloud), and new git status inquiry methods. It replaces the previous cloud deployment flow with a modular RosettaCloudService and refactors UI components to support environment-aware execution paths. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App/MainProcess
participant DeepLink as Deep Link Handler
participant RCS as RosettaCloudService
participant SecureStorage as Secure Storage
participant Renderer as Renderer/UI
User->>App: Open rosetta://auth?token=xyz
App->>DeepLink: Handle custom protocol
DeepLink->>DeepLink: Parse token from URL
DeepLink->>RCS: Call storeApiKey(token)
RCS->>SecureStorage: Store API key
SecureStorage-->>RCS: Stored
DeepLink->>Renderer: Send rosettaCloud:authSuccess event
Renderer->>Renderer: Invalidate auth cache
Renderer->>RCS: Fetch profile with stored API key
RCS-->>Renderer: User profile
Renderer->>Renderer: Update UI with authenticated state
sequenceDiagram
participant User
participant UI as DBT Component
participant Renderer as App Context
participant RCS as RosettaCloudService
participant Modal as PushToCloudModal
participant MainSvc as Main/RosettaCloudService
participant Cloud as Rosetta Cloud API
User->>UI: Click "Run in Cloud" (env=cloud)
UI->>Renderer: Check environment from AppContext
Renderer-->>UI: env='cloud'
UI->>Modal: Open with command & args
Modal->>RCS: Fetch repo info & local changes
RCS-->>Modal: gitUrl, branch, changes status
Modal->>Modal: Display validation errors if needed
User->>Modal: Configure env vars & confirm
Modal->>MainSvc: Call pushProjectToCloud
MainSvc->>Cloud: Create/update project
Cloud-->>MainSvc: Project created (externalId)
MainSvc->>Cloud: Add secrets
Cloud-->>MainSvc: Secrets added
MainSvc->>Cloud: Trigger run with CUSTOM_DBT_COMMANDS
Cloud-->>MainSvc: Run initiated
MainSvc-->>Modal: Success
Modal->>UI: Close with toast notification
UI->>Renderer: Clear modal state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/hooks/useDbt.ts (1)
269-278: Missing dependencies inuseCallback.
envandcloudRunCbare used insideexecuteCommandbut are not listed in the dependency array, which can cause stale closure issues.🔎 Proposed fix
[ isRunning, connections, setupConnectionEnv, buildCommand, runCommand, successCallback, settings?.dbtPath, + env, + cloudRunCb, ],src/main/ipcHandlers/git.ipcHandlers.ts (1)
13-35: Missing new IPC channels in handler cleanup array.The new
git:getLocalChangesandgit:repoInfohandlers (lines 206-224) are not included in thehandlerChannelsarray. This meansremoveGitIpcHandlers()won't clean them up, potentially causing "handler already registered" errors ifregisterGitHandlers()is called multiple times.🔎 Proposed fix
const handlerChannels = [ 'git:init', 'git:clone', 'git:listBranches', 'git:checkout', 'git:addRemote', 'git:isInitialized', 'git:getRemotes', 'git:add', 'git:unstage', 'git:stageAll', 'git:unstageAll', 'git:discardChanges', 'git:commit', 'git:pull', 'git:push', 'git:fileDiff', 'git:fileStatusList', 'git:aheadBehind', 'git:createBranch', 'git:deleteBranch', 'git:renameBranch', + 'git:getLocalChanges', + 'git:repoInfo', ];
🤖 Fix all issues with AI Agents
In @src/main/services/git.service.ts:
- Around line 1144-1180: getRepoInfo currently forces ".git" on remoteUrl
(variable remoteUrl inside getRepoInfo), which can produce incorrect addresses
for some remotes; remove the conditional that appends ".git" (or make it
conditional on a new optional flag like normalizeRemoteUrl) so the function
returns the original fetch URL from remotes.find(...).refs.fetch without
modification, and adjust callers if they relied on the forced ".git" suffix.
In @src/main/services/rosettaCloud.service.ts:
- Around line 28-41: The postJson helper (function postJson) currently returns
response.json() without checking response.ok; update it to check response.ok
after fetch and, if false, read and include the error body (prefer JSON if
possible, fallback to text) along with response.status/statusText in a thrown
Error (or custom error) so callers can handle failures; keep the Authorization
logic using this.getApiKey() as-is and ensure successful responses still return
the parsed JSON.
In @src/renderer/components/dbtModelButtons/ModelSplitButton.tsx:
- Around line 52-79: The component and the hook are using different environment
sources; align them by reading the environment from AppContext in this component
(like useDbt does) instead of using the environment prop. Import and call
useContext(AppContext) to obtain the same env value/profile fallback, replace
all uses of the environment prop in executeCommand and handlers (e.g.,
executeCommand, handleRunModel) with that context-derived env, and remove
reliance on props?.env so the component and useDbt share the exact same
environment decision logic.
In @src/renderer/components/menu/index.tsx:
- Around line 244-247: The code uses a non-null assertion on settings when
calling updateSettings({...settings!, env: newEnv}), which can crash if settings
is undefined; change the call to guard against undefined by either
early-returning when settings is falsy or merging with a safe default (e.g.,
updateSettings({ ...(settings ?? {}), env: newEnv })) and ensure this logic
lives where updateSettings, settings (from useGetSettings) and newEnv are
referenced so the switch toggle cannot dereference undefined.
In @src/renderer/components/modals/pushToCloudModal/index.tsx:
- Around line 883-893: The key TextField is incorrectly wired to handleEnvFocus
and handleEnvBlur which clear and mark the value as edited; remove the onFocus
and onBlur props from the key TextField (the instance using value={env.key} and
updateEnvironmentVariable(env.id, e.target.value, env.value)) and ensure those
focus/blur handlers remain only on the value TextField (the one that edits
env.value) so focus behavior only affects the value field.
In @src/renderer/controllers/git.controller.ts:
- Around line 117-149: Both hooks use a static queryKey which omits the path,
causing cached responses to bleed across repositories; update useGetLocalChanges
and useGetRepoInfo to include the path in their queryKey (e.g., append the path
to QUERY_KEYS.GIT_LOCAL_CHANGES and QUERY_KEYS.GIT_REPO_INFO) so the queryKey
uniquely identifies the repo, leaving the rest of the useQuery config and
queryFn (gitServices.getLocalChanges(path) / gitServices.getRepoInfo(path))
unchanged.
In @src/renderer/controllers/rosettaCloud.controller.ts:
- Around line 76-85: The query key for useGetSecrets must include the projectId
to avoid cross-project cache collisions; update the queryKey in the
useGetSecrets function (currently using QUERY_KEYS.CLOUD_SECRETS) to include the
projectId (e.g., [QUERY_KEYS.CLOUD_SECRETS, projectId ?? '']) so each project
has an isolated cache entry while keeping the existing queryFn that calls
rosettaCloudServices.getSecrets(projectId ?? '') and preserving passed options.
In @src/renderer/hooks/useDbt.ts:
- Around line 227-230: When env === 'cloud' the early return prevents the
finally block from clearing activeCommand, leaving the state stuck; update the
cloud branch in the run flow (in useDbt) to ensure setActiveCommand(null) is
invoked before returning — either call setActiveCommand(null) immediately after
cloudRunCb?.(command) or refactor so the finally block always runs (e.g., avoid
returning early in the try and instead short-circuit after performing cleanup).
Target symbols: activeCommand, setActiveCommand, cloudRunCb, and the run
function in useDbt to apply the fix.
In @src/types/profile.ts:
- Around line 16-18: UserProfile declares emailVerified, createdAt and updatedAt
as Date but RosettaCloudService.getProfile() returns raw JSON ISO strings; fix
by parsing those ISO strings into Date objects inside
RosettaCloudService.getProfile() (convert emailVerified nullable ISO to
Date|null and createdAt/updatedAt to Date) before returning, or alternatively
change UserProfile types to string (and emailVerified to string|null) and update
all callers that use Date methods; reference UserProfile and
RosettaCloudService.getProfile() when making the change so callers like
components expecting .getTime() or .toLocaleDateString() are updated
accordingly.
🧹 Nitpick comments (10)
src/renderer/components/profile/ProfileCard.tsx (1)
41-50: Consider reusing the existinggetInitialsutility and add a guard for empty email.A more robust
getInitialshelper already exists insrc/renderer/helpers/utils.tsthat handles edge cases like underscores in names. Additionally,email[0]will throw if the email is an empty string.🔎 Proposed fix
+import { getInitials as getNameInitials } from '../../helpers/utils'; + export const ProfileCard: React.FC = () => { const { data: profile, isLoading, error } = useProfile(); // ... loading/error states ... - const getInitials = (name: string | null, email: string) => { - if (name) { - return name - .split(' ') - .map((n) => n[0]) - .join('') - .toUpperCase(); - } - return email[0].toUpperCase(); - }; + const getInitials = (name: string | null, email: string) => { + if (name) { + return getNameInitials(name); + } + return email?.length ? email[0].toUpperCase() : '?'; + };src/main/main.ts (1)
63-67: Consider not passing apiKey in the authSuccess payload.The API key is already stored securely via
RosettaCloudService.storeApiKey. Sending it to the renderer in theauthSuccessevent payload may be unnecessary if the renderer only needs to know authentication succeeded (to trigger profile/API key refetch).🔎 Proposed change
windowManager ?.getMainWindow() - ?.webContents.send('rosettaCloud:authSuccess', { - apiKey, - }); + ?.webContents.send('rosettaCloud:authSuccess');src/renderer/controllers/profile.controller.ts (1)
28-31: Fragile error check based on message string.Checking
error?.message?.includes('401')relies on error message format which could change. Consider checking for a more structured error property (e.g., status code) if available inCustomError.retry: (failureCount, error) => { // Don't retry on auth errors - if (error?.message?.includes('401')) return false; + if (error?.code === '401' || error?.message?.includes('401')) return false; return failureCount < 3; },src/renderer/components/menu/index.tsx (1)
76-76: UnuseduseAuthLogouthook.Only
isLoadingis destructured, but without callingmutate,logoutLoadingwill never betrue. If logout is handled elsewhere, consider removing this hook or moving the logout UI to this component.src/main/services/git.service.ts (2)
1009-1055: Consider making fetch optional or caching its result.This method calls
git.fetch()on every invocation, which is a network operation that can be slow or fail. WhenhasLocalChangescalls this alongside other methods, multiple fetches may occur. Consider:
- Adding an optional parameter to skip fetch
- Caching fetch results with a TTL
- Documenting that callers should use
getLocalChangesStatusinstead for combined queries
1057-1072: Potential for multiple git fetch operations.
hasLocalChangescallshasUnpushedChangeswhich performs agit.fetch(). If the caller also needs detailed info fromgetLocalChangesStatus, another fetch occurs. Consider usinggetLocalChangesStatusinternally or refactoring to share fetch results.src/types/apiKey.ts (1)
20-22: Convert empty interface to a type alias.The static analysis tool (Biome) correctly flags that an empty interface is equivalent to
{}. For a void/empty payload, use a type alias instead.🔎 Proposed fix
-export interface ApiKeyUpdatePayload { - // Void type - no payload data needed for API key updates -} +// Void type - no payload data needed for API key updates +export type ApiKeyUpdatePayload = Record<string, never>;src/main/services/rosettaCloud.service.ts (2)
178-196: Clarify error handling behavior for non-401 failures.Lines 185 throws an error for non-401/non-OK responses, which is then caught at line 191 and silently returns the cached profile. This may hide server errors (e.g., 500) from the user. Consider differentiating between network errors (where cache fallback is appropriate) and server errors (where explicit failure may be preferred).
220-234: Consider removing success log for API key storage.Logging "API key stored successfully" (line 228) is not a security issue per se, but it's generally a good practice to avoid logging about credential operations in production. The error logging on failure is appropriate.
src/renderer/components/modals/pushToCloudModal/index.tsx (1)
347-353: Consider using a more robust ID generator.Using
Date.now().toString()for IDs works for client-side state but could theoretically produce duplicates ifaddEnvironmentVariableis called multiple times within the same millisecond. Consider using a counter or UUID for more robustness.🔎 Proposed alternative
+import { v4 as uuidv4 } from 'uuid'; +// or use a simple counter: +// let envVarIdCounter = 0; const newEnv: EnvironmentVariable = { - id: Date.now().toString(), + id: uuidv4(), + // or: id: `env-${++envVarIdCounter}`, key: trimmedKey, value: trimmedValue, originalValue: trimmedValue, isEdited: true, };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.gitignoreassets/index.tssrc/main/ipcHandlers/git.ipcHandlers.tssrc/main/ipcHandlers/index.tssrc/main/ipcHandlers/projects.ipcHandlers.tssrc/main/ipcHandlers/rosettaCloud.ipcHandlers.tssrc/main/ipcSetup.tssrc/main/main.tssrc/main/services/git.service.tssrc/main/services/index.tssrc/main/services/projects.service.tssrc/main/services/rosettaCloud.service.tssrc/main/utils/constants.tssrc/main/utils/fileHelper.tssrc/renderer/App.tsxsrc/renderer/components/dbtModelButtons/ModelSplitButton.tsxsrc/renderer/components/dbtModelButtons/ProjectDbtSplitButton.tsxsrc/renderer/components/menu/index.tsxsrc/renderer/components/menu/styles.tssrc/renderer/components/modals/pushToCloudModal/index.tsxsrc/renderer/components/profile/ProfileCard.tsxsrc/renderer/components/profile/index.tssrc/renderer/components/settings/CloudSettings.tsxsrc/renderer/components/settings/ProfileSettings.tsxsrc/renderer/components/settings/index.tssrc/renderer/config/constants.tssrc/renderer/context/AppProvider.tsxsrc/renderer/controllers/git.controller.tssrc/renderer/controllers/index.tssrc/renderer/controllers/profile.controller.tssrc/renderer/controllers/projects.controller.tssrc/renderer/controllers/rosettaCloud.controller.tssrc/renderer/hooks/useApiKeySync.tssrc/renderer/hooks/useDbt.tssrc/renderer/hooks/useSecureStorage.tssrc/renderer/screens/projectDetails/index.tsxsrc/renderer/screens/settings/index.tsxsrc/renderer/screens/settings/settingsElements.tsxsrc/renderer/services/git.service.tssrc/renderer/services/index.tssrc/renderer/services/profile.service.tssrc/renderer/services/projects.service.tssrc/renderer/services/rosettaCloud.service.tssrc/types/apiKey.tssrc/types/backend.tssrc/types/frontend.tssrc/types/ipc.tssrc/types/profile.ts
💤 Files with no reviewable changes (5)
- src/renderer/services/projects.service.ts
- src/main/ipcHandlers/projects.ipcHandlers.ts
- src/main/services/projects.service.ts
- src/main/utils/fileHelper.ts
- src/renderer/controllers/projects.controller.ts
🧰 Additional context used
🧬 Code graph analysis (22)
src/renderer/components/profile/ProfileCard.tsx (3)
src/renderer/components/profile/index.ts (1)
ProfileCard(1-1)src/renderer/controllers/profile.controller.ts (1)
useProfile(17-35)src/renderer/helpers/utils.ts (1)
getInitials(71-76)
src/renderer/hooks/useSecureStorage.ts (2)
src/renderer/services/secureStorage.service.ts (1)
secureStorageService(4-29)src/main/utils/constants.ts (1)
CLOUD_DASHBOARD_API_KEY(26-26)
src/main/ipcHandlers/rosettaCloud.ipcHandlers.ts (3)
src/main/ipcHandlers/index.ts (1)
registerRosettaCloudIpcHandlers(27-27)src/types/backend.ts (1)
CloudDeploymentPayload(205-214)src/main/services/rosettaCloud.service.ts (1)
RosettaCloudService(11-303)
src/renderer/components/settings/CloudSettings.tsx (3)
src/renderer/controllers/rosettaCloud.controller.ts (3)
useApiKey(49-64)useValidateApiKey(66-71)useAuthLogin(87-100)src/main/services/rosettaCloud.service.ts (1)
validateApiKey(266-302)src/renderer/hooks/useApiKeySync.ts (1)
useApiKeySync(10-25)
src/renderer/components/settings/ProfileSettings.tsx (3)
src/renderer/controllers/profile.controller.ts (3)
useProfile(17-35)useRefreshProfile(37-55)useProfileSubscription(57-112)src/renderer/components/settings/CloudSettings.tsx (1)
CloudSettings(29-346)src/renderer/components/profile/ProfileCard.tsx (1)
ProfileCard(14-77)
src/renderer/components/dbtModelButtons/ModelSplitButton.tsx (3)
src/types/backend.ts (1)
DbtCommandType(673-684)src/renderer/helpers/utils.ts (1)
extractModelNameFromPath(271-284)src/renderer/components/modals/pushToCloudModal/index.tsx (1)
PushToCloudModal(61-1107)
src/renderer/hooks/useApiKeySync.ts (2)
src/renderer/controllers/rosettaCloud.controller.ts (1)
useAuthSubscription(133-173)src/renderer/config/constants.ts (1)
QUERY_KEYS(46-88)
src/renderer/controllers/git.controller.ts (2)
src/types/backend.ts (3)
GitChangesRes(734-741)CustomError(342-344)RepoInfoRes(743-747)src/renderer/config/constants.ts (1)
QUERY_KEYS(46-88)
src/types/frontend.ts (1)
src/types/profile.ts (1)
UserProfile(11-22)
src/renderer/controllers/profile.controller.ts (3)
src/types/profile.ts (1)
UserProfile(11-22)src/types/backend.ts (1)
CustomError(342-344)src/renderer/services/profile.service.ts (1)
profileService(25-29)
src/renderer/screens/settings/index.tsx (1)
src/renderer/components/settings/ProfileSettings.tsx (1)
ProfileSettings(21-107)
src/main/services/git.service.ts (1)
src/types/backend.ts (2)
GitChangesRes(734-741)RepoInfoRes(743-747)
src/main/ipcSetup.ts (1)
src/main/ipcHandlers/index.ts (1)
registerRosettaCloudIpcHandlers(27-27)
src/renderer/App.tsx (1)
src/renderer/screens/index.ts (1)
Settings(13-13)
src/renderer/components/menu/index.tsx (5)
src/renderer/services/settings.services.ts (1)
updateSettings(21-23)src/renderer/controllers/rosettaCloud.controller.ts (4)
useApiKey(49-64)useAuthLogin(87-100)useAuthLogout(102-131)useAuthSubscription(133-173)src/renderer/controllers/profile.controller.ts (2)
useProfileSubscription(57-112)useProfile(17-35)assets/index.ts (1)
rosettaIcon(5-5)src/main/utils/constants.ts (1)
ROSETTA_CLOUD_BASE_URL(28-28)
src/renderer/context/AppProvider.tsx (3)
src/renderer/controllers/settings.controller.ts (1)
useGetSettings(20-30)src/renderer/controllers/aiProviders.controller.ts (1)
useGetActiveAIProvider(89-103)src/renderer/controllers/profile.controller.ts (1)
useProfile(17-35)
src/main/ipcHandlers/git.ipcHandlers.ts (1)
src/types/backend.ts (2)
GitChangesRes(734-741)RepoInfoRes(743-747)
src/renderer/screens/settings/settingsElements.tsx (1)
src/renderer/components/sidebar/icons.tsx (1)
CloudIcon(56-56)
src/renderer/components/modals/pushToCloudModal/index.tsx (3)
src/types/backend.ts (2)
Project(184-203)DbtCommandType(673-684)src/renderer/controllers/git.controller.ts (2)
useGetLocalChanges(117-132)useGetRepoInfo(134-149)src/renderer/controllers/rosettaCloud.controller.ts (2)
usePushProjectToCloud(25-47)useGetSecrets(76-85)
src/main/main.ts (4)
src/main/utils/iconUtils.ts (1)
setupApplicationIcon(39-50)src/main/windows/index.ts (1)
WindowManager(11-136)src/main/services/index.ts (1)
RosettaCloudService(31-31)src/main/services/rosettaCloud.service.ts (1)
RosettaCloudService(11-303)
src/renderer/hooks/useDbt.ts (3)
src/renderer/hooks/index.ts (2)
useDbt(24-24)useAppContext(19-19)src/types/backend.ts (1)
DbtCommandType(673-684)src/renderer/controllers/settings.controller.ts (1)
useGetSettings(20-30)
src/main/services/rosettaCloud.service.ts (4)
src/types/profile.ts (1)
UserProfile(11-22)src/types/backend.ts (2)
CloudDeploymentPayload(205-214)Secret(749-753)src/main/services/projects.service.ts (1)
ProjectsService(44-1162)src/main/utils/constants.ts (1)
ROSETTA_CLOUD_BASE_URL(28-28)
🪛 Biome (2.1.2)
src/types/apiKey.ts
[error] 20-22: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (76)
.gitignore (1)
36-36: No issues detected. This appears to be a minor cosmetic adjustment with no functional impact on the ignore patterns.src/renderer/screens/settings/settingsElements.tsx (1)
4-4: LGTM!The new CloudIcon import and "Rosetta Cloud" sidebar entry follow the established pattern in this file. The path
/app/settings/profilecorrectly aligns with the route added inApp.tsx.Also applies to: 69-73
src/renderer/components/profile/index.ts (1)
1-1: LGTM!Standard barrel export following the project's module structure conventions.
src/renderer/App.tsx (1)
57-57: LGTM!The new route for
settings/profilefollows the established pattern for settings sub-routes, correctly rendering theSettingscomponent.src/renderer/components/settings/index.ts (1)
2-2: LGTM!Clean addition of the
ProfileSettingsre-export following the existing pattern.src/renderer/services/git.service.ts (1)
137-151: LGTM!The new
getLocalChangesandgetRepoInfofunctions follow the established IPC communication pattern in this service, with proper typing and nullable return types for graceful failure handling.src/renderer/screens/settings/index.tsx (1)
27-27: LGTM!The
ProfileSettingsintegration is clean:
- Import added appropriately
- Section title correctly mapped to "Rosetta Cloud"
- Switch case follows the existing pattern with the prop-less
ProfileSettingscomponentAlso applies to: 105-105, 127-128
src/main/ipcHandlers/index.ts (1)
12-12: LGTM! Clean integration of Rosetta Cloud IPC handlers.The new handler registration follows the established pattern and is properly integrated into the module exports.
Also applies to: 27-27
src/renderer/services/index.ts (1)
10-10: LGTM! Rosetta Cloud service properly exposed.The service integration follows the established module export pattern.
Also applies to: 23-23
assets/index.ts (1)
3-3: LGTM! Asset export addition is clean.The Rosetta icon is properly imported and exposed alongside existing assets.
Also applies to: 5-5
src/renderer/controllers/index.ts (1)
8-9: LGTM! Controller exports properly added.The new profile and Rosetta Cloud controllers are integrated following the established export pattern.
src/types/frontend.ts (1)
4-4: Theenvproperty is already supplied in all AppContext creation locations.The
envproperty onAppContextType(required, non-optional) is correctly provided both in the default context value (line 20 with'local') and in the actual provider value (line 171 withprofile ? (settings?.env ?? 'local') : 'local'). No additional updates are needed.src/main/services/index.ts (1)
12-12: LGTM! Clean service registration.The RosettaCloudService is properly imported and exported, maintaining consistency with the existing service registration pattern.
Also applies to: 31-31
src/main/ipcSetup.ts (1)
14-14: LGTM! IPC handler registration follows established pattern.The Rosetta Cloud IPC handlers are properly wired into the registration sequence alongside existing handlers.
Also applies to: 30-30
src/renderer/hooks/useSecureStorage.ts (1)
2-2: LGTM! Good refactor to eliminate magic strings.Replacing the hard-coded
'cloud-api-key'string with theCLOUD_DASHBOARD_API_KEYconstant centralizes the key name and reduces the risk of typos across the codebase.Also applies to: 150-150, 154-154, 158-158
src/renderer/hooks/useApiKeySync.ts (1)
1-25: LGTM! Clean authentication state management.The hook properly subscribes to auth events and provides a well-structured mechanism to refresh authentication-related queries. The use of
useCallbackfor memoization is appropriate.src/renderer/components/settings/ProfileSettings.tsx (1)
21-107: LGTM! Well-structured profile management component.The component effectively handles:
- Loading states with appropriate UI feedback
- Conditional rendering based on authentication status
- Real-time profile updates via subscription
- Error states with user-friendly alerts
The pattern of calling
useProfile()here for loading/error state whileProfileCardalso calls it internally is a standard React Query practice—the library efficiently deduplicates and caches the queries.src/renderer/config/constants.ts (1)
65-66: LGTM!The new query keys follow the existing naming conventions and align with the new IPC channels and controller hooks introduced in this PR for git repository status and Rosetta Cloud authentication features.
Also applies to: 85-87
src/renderer/screens/projectDetails/index.tsx (1)
796-797: LGTM!The
environmentprop is correctly propagated from settings to both button components, enabling environment-aware behavior (local vs. cloud). Optional chaining handles the case where settings may still be loading.Also applies to: 807-808
src/types/ipc.ts (2)
55-69: LGTM!The new
RosettaCloudChannelstype cleanly defines all IPC channels for the Rosetta Cloud integration, including request/response handlers (push, profile, API key management, secrets) and event channels for the OAuth deep-link flow (authSuccess,authError,apiKeyUpdated).
204-205: LGTM!New git IPC channels align with the corresponding hooks (
useGetLocalChanges,useGetRepoInfo) and backend service methods.src/renderer/components/menu/styles.ts (2)
29-90: LGTM!The environment switch styling is well-structured with proper MUI theme integration. The
SwitchIconcomponent uses a class-based approach (.checked/.unchecked) for positioning, which works correctly when the parent component toggles classes based on switch state.
92-108: LGTM!Clean styled components for the authentication button content with appropriate theme token usage for consistent styling.
src/renderer/components/profile/ProfileCard.tsx (1)
52-76: LGTM!The profile card UI is well-structured with appropriate MUI components. The conditional role chip coloring (primary for ADMIN, default for USER) provides clear visual distinction.
src/main/ipcHandlers/rosettaCloud.ipcHandlers.ts (1)
5-61: LGTM!The IPC handlers correctly delegate to
RosettaCloudServicemethods. The event-based channels (rosettaCloud:authSuccess,rosettaCloud:authError, androsettaCloud:apiKeyUpdated) are properly sent from the main process viawebContents.send()in response to the OAuth deep-link flow and API key updates, and are listened to by the renderer viaipcRenderer.on().src/main/utils/constants.ts (1)
26-28: Verify URL endpoint for production readiness.The
CLOUD_DASHBOARD_API_KEYconstant name is clear as a storage key identifier. However,ROSETTA_CLOUD_BASE_URLpoints todashboard.tolstudios.netwhich appears to be a staging or development environment rather than a production Rosetta Cloud domain.Ensure this URL is intentional for the target branch (
dev) and that there's a plan to update it for production releases.src/main/ipcHandlers/git.ipcHandlers.ts (1)
206-224: LGTM!The new
git:getLocalChangesandgit:repoInfohandlers follow the established patterns in this file with proper typing and consistent structure.src/renderer/context/AppProvider.tsx (1)
169-170: LGTM - Good security pattern.The conditional
envderivation correctly ensures cloud environment is only available when a user profile exists, preventing unauthorized cloud access. The fallback to'local'is appropriate.src/main/main.ts (1)
35-41: LGTM!The
rosetta://protocol is correctly registered withsecure: true, which is appropriate for handling authentication callbacks.src/types/profile.ts (1)
1-31: LGTM!The profile types are well-structured with appropriate nullable fields and clear role definitions.
src/renderer/controllers/profile.controller.ts (1)
57-111: LGTM!The IPC subscription hook properly sets up and cleans up all event listeners, with appropriate query invalidation on auth events.
src/renderer/components/menu/index.tsx (2)
160-231: LGTM!The conditional rendering of auth/dashboard buttons based on
apiKeystate is clean, with informative tooltips and consistent styling.
234-290: LGTM!The environment switch is correctly gated behind profile authentication and provides clear visual feedback with appropriate icons and labels.
src/renderer/components/settings/CloudSettings.tsx (5)
1-28: LGTM!The imports are well-organized and include all necessary MUI components, icons, and custom hooks for the cloud settings functionality.
29-66: LGTM!Component initialization is clean. The hooks are properly configured with callbacks for OAuth login success/error handling, and the
useEffectcorrectly clears input state when an API key is received (e.g., from OAuth flow).
117-134: LGTM!The
handleRemoveApiKeyfunction properly handles async cleanup with appropriate error handling and state reset in thefinallyblock.
153-345: LGTM!The JSX structure is well-organized with clear conditional rendering for OAuth login, connection status, and API key management sections. The show/hide toggle for API keys is a good security practice.
76-115:isSavingnot reset when validation fails.When
validation.validis false at line 96-98, the function returns early without reaching thefinallyblock, leavingisSavingastrue. This will disable the UI indefinitely after a failed validation.🔎 Proposed fix
if (!validation.valid) { setApiKeyError(validation.error || 'Invalid API key'); + setIsSaving(false); return; }Likely an incorrect or invalid review comment.
src/types/backend.ts (4)
201-202: LGTM!The new optional properties
externalIdandlastRunextend the Project type appropriately for cloud integration.
205-214: LGTM!
CloudDeploymentPayloadis well-structured with the requiredidandsecretsfields for cloud deployment. The optionalCUSTOM_DBT_COMMANDSprovides flexibility for custom dbt command configuration.
280-281: LGTM!The
envproperty with'local' | 'cloud'union type provides clear environment switching capability.
733-753: LGTM!The new
GitChangesRes,RepoInfoRes, andSecrettypes are well-defined and align with the git service implementations.src/renderer/services/profile.service.ts (1)
1-31: LGTM!Clean service implementation with consistent structure. The functions properly wrap IPC calls and return typed responses. Error handling is appropriately delegated to the consuming code (controllers/hooks).
src/renderer/components/dbtModelButtons/ProjectDbtSplitButton.tsx (6)
7-19: LGTM!The imports are properly extended to include cloud-related types and components.
104-202: LGTM!The Rosetta layer generation commands (Raw Layer, Staging Layer, Incremental/Enhanced Layer, Business Layer) are correctly marked as
localOnly: truesince they require local Rosetta CLI execution.
203-348: LGTM!The dbt commands are appropriately categorized - production commands like Run, Test, Build, Compile, Clean, Deps, Seed are available in both environments, while Debug, Generate Docs, and Serve Docs are correctly marked as local-only development tools.
350-374: LGTM!The filtering logic correctly excludes
localOnlyitems in cloud mode while showing all items in local mode. The eslint disable comment for the unusedlocalOnlydestructuring is acceptable.
440-447: LGTM!The
PushToCloudModalis properly integrated with appropriate props for project and command context.
49-64: Clarify the purpose of theuseDbtcallback in local mode.The callback
(command) => { setRunInCloudModal(command); }is passed touseDbt, but this will trigger the cloud modal even whenenvironment === 'local'. Is this intentional? If the modal should only appear in cloud mode, the callback should check the environment first.#!/bin/bash # Check how useDbt uses the callback parameter ast-grep --pattern $'export function useDbt($_, $_) { $$$ }' ast-grep --pattern $'export const useDbt = ($_, $_) => { $$$ }' rg -n -A 20 'export.*useDbt' --type tssrc/main/services/git.service.ts (3)
6-11: LGTM!Import additions for the new Git-related types are correct.
976-1007: LGTM!The
hasUntrackedChangesandhasUncommittedChangesmethods are straightforward implementations that check the git status correctly.
1074-1142: LGTM!The
getLocalChangesStatusmethod provides a comprehensive view of repository changes. The error handling for fetch failures is appropriate - silently returning false for unpushed status when network is unavailable is reasonable behavior.src/renderer/components/dbtModelButtons/ModelSplitButton.tsx (4)
11-18: LGTM!Cloud-related imports are properly added.
226-253: LGTM!The
handleRunModelfunction correctly routes execution based on environment. The model name extraction happens before the environment check, which is appropriate since both paths need it.
587-702: LGTM!The
allMenuItemsarray is well-structured with appropriatelocalOnlyflags. Compile and Preview are correctly marked as local-only since they require direct database connectivity and local dbt execution.
749-761: LGTM!The
PushToCloudModalis properly integrated withinitialDbtArgumentspassed through, and theonClosehandler correctly clears both the modal state and the arguments.src/renderer/controllers/rosettaCloud.controller.ts (5)
1-23: LGTM!Imports are properly organized with React Query hooks and type definitions.
25-47: LGTM!The
usePushProjectToCloudmutation properly invalidates the selected project query on success and forwards custom callbacks.
49-71: LGTM!The
useApiKeyanduseValidateApiKeyhooks are well-implemented with appropriate defaults and retry configuration.
87-131: LGTM!The
useAuthLoginanduseAuthLogouthooks are properly implemented. The logout hook correctly invalidates both API key and user profile caches and provides user feedback via toasts.
133-173: LGTM!The
useAuthSubscriptionhook properly subscribes to authentication events and cleans up subscriptions on unmount. The query invalidation on auth state changes ensures the UI stays in sync.src/types/apiKey.ts (1)
1-73: Well-structured type definitions for the new API key authentication system.The types are well-organized and clearly define the contract for API key operations, event payloads, and React Query hooks. The separation between state types, payload types, and hook result types is clean.
src/renderer/services/rosettaCloud.service.ts (3)
36-55: Subscription cleanup function correctly implemented.The pattern of returning a cleanup function that removes the listener is correct and prevents memory leaks. The validation for
apiKeypresence before invoking the callback is a good defensive measure.
57-88: Consistent subscription patterns with proper cleanup.The subscription functions follow a consistent pattern and handle edge cases well (e.g., providing a fallback error message in
subscribeToAuthError).
96-102: IPC handler alignment is correct.The
getSecretsfunction properly aligns with the IPC handler inrosettaCloud.ipcHandlers.ts. Both the client call and the handler expectprojectIdas a string parameter, which is correctly passed through toRosettaCloudService.getSecrets(projectId).src/main/services/rosettaCloud.service.ts (3)
57-89: Project deployment logic is well-structured.The branching logic correctly handles both existing projects (with
externalId) and new projects. The flow of create → add secrets → run is appropriate.
91-155: Good error handling in getSecrets and deleteSecret.Both methods properly validate preconditions (project exists, has externalId) and check
response.okbefore processing. This is consistent and robust.
266-302: Comprehensive API key validation with proper status handling.The
validateApiKeymethod handles various response scenarios (200 OK, 401, 404, other errors, and network failures) with appropriate error messages. This provides good feedback to users during authentication.src/renderer/components/modals/pushToCloudModal/index.tsx (8)
43-49: Well-defined local interface for environment variable tracking.The
EnvironmentVariableinterface withisEditedandoriginalValuetracking is a clean solution for managing edit state and enabling reversion.
69-76: Data fetching hooks are well-integrated.The use of separate hooks for local changes, repo info, and secrets provides clean separation of concerns and enables individual loading states.
136-168: Proper separation of Git credentials from environment variables.The effect correctly filters out reserved keys (ROSETTA_GIT_USER, ROSETTA_GIT_PASSWORD) from general environment variables and handles them separately, preventing duplication.
170-197: Comprehensive blocking error detection for deployment prerequisites.The validation checks for repository info availability, remote URL configuration, and branch existence on remote provide clear, actionable error messages to users.
305-306: Verify intended behavior for Git credentials in run mode.In run mode (
isRunMode),githubUsernameandgithubPasswordare set toundefined(lines 305-306). However, lines 289-294 still add them toreducedSecretsif edited. Confirm this is the intended behavior—it appears Git credentials are only submitted as secrets in run mode, not as top-level fields.
394-421: Smart focus/blur handlers for secret editing.The pattern of clearing on focus (for non-edited values) and reverting on blur (if empty or unchanged) provides a good UX for editing masked secret values while preventing accidental data loss.
556-593: Git URL and Branch fields appropriately disabled.These fields are auto-filled from repository configuration and correctly disabled to prevent user modification, ensuring consistency with the actual repository state.
1000-1106: Modal structure and submission gating look correct.The modal properly prevents closing during push operation (
isPushing), and the submit button is appropriately disabled based oncanSubmitwhich checks all required conditions.
| // Handle deep links on Windows/Linux | ||
| app.on('ready', () => { | ||
| // Check if app was opened with a deep link | ||
| const url = process.argv.find((arg) => arg.startsWith('rosetta://')); | ||
| if (url) { | ||
| handleDeepLink(url); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Deep links on cold start may be silently lost.
When the app is launched via a deep link on Windows/Linux, this ready handler fires before windowManager and the main window are created (lines 119-190). The optional chaining prevents crashes, but the rosettaCloud:authSuccess notification will be silently dropped.
Consider deferring deep link handling until the main window is ready, or queuing the URL for later processing.
🔎 Proposed approach
+let pendingDeepLinkUrl: string | null = null;
+
async function handleDeepLink(url: string) {
+ // If main window not ready, queue the URL
+ if (!windowManager?.getMainWindow()) {
+ pendingDeepLinkUrl = url;
+ return;
+ }
try {
// ... existing logic
}
}
// In the splash.webContents.once('did-finish-load') callback, after showMainWindow():
+if (pendingDeepLinkUrl) {
+ handleDeepLink(pendingDeepLinkUrl);
+ pendingDeepLinkUrl = null;
+}Committable suggestion skipped: line range outside the PR's diff.
| async getRepoInfo(repoPath: string): Promise<RepoInfoRes | null> { | ||
| const git = this.getGitInstance(repoPath); | ||
|
|
||
| try { | ||
| const remotes = await git.getRemotes(true); | ||
| const origin = remotes.find((r) => r.name === 'origin'); | ||
| let remoteUrl = origin?.refs?.fetch || null; | ||
|
|
||
| if (remoteUrl && !remoteUrl.endsWith('.git')) { | ||
| remoteUrl = `${remoteUrl}.git`; | ||
| } | ||
|
|
||
| const branchSummary = await git.branch(); | ||
| const currentBranch = branchSummary.current; | ||
|
|
||
| let branchExistsOnRemote = false; | ||
| if (currentBranch) { | ||
| try { | ||
| await git.fetch(); | ||
| const remoteBranches = await git.branch(['-r']); | ||
| branchExistsOnRemote = remoteBranches.all.includes( | ||
| `origin/${currentBranch}`, | ||
| ); | ||
| } catch (err) { | ||
| branchExistsOnRemote = false; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| remoteUrl, | ||
| currentBranch, | ||
| branchExistsOnRemote, | ||
| }; | ||
| } catch (err: any) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Normalizing remote URL by appending .git may not always be correct.
Lines 1152-1154 append .git to the remote URL if it doesn't end with .git. However, some Git remotes (especially internal/enterprise ones or those using different protocols) may not follow this convention. This could produce incorrect URLs.
🔎 Suggested approach
Consider removing the normalization or making it optional, as the original remote URL should be authoritative:
let remoteUrl = origin?.refs?.fetch || null;
- if (remoteUrl && !remoteUrl.endsWith('.git')) {
- remoteUrl = `${remoteUrl}.git`;
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getRepoInfo(repoPath: string): Promise<RepoInfoRes | null> { | |
| const git = this.getGitInstance(repoPath); | |
| try { | |
| const remotes = await git.getRemotes(true); | |
| const origin = remotes.find((r) => r.name === 'origin'); | |
| let remoteUrl = origin?.refs?.fetch || null; | |
| if (remoteUrl && !remoteUrl.endsWith('.git')) { | |
| remoteUrl = `${remoteUrl}.git`; | |
| } | |
| const branchSummary = await git.branch(); | |
| const currentBranch = branchSummary.current; | |
| let branchExistsOnRemote = false; | |
| if (currentBranch) { | |
| try { | |
| await git.fetch(); | |
| const remoteBranches = await git.branch(['-r']); | |
| branchExistsOnRemote = remoteBranches.all.includes( | |
| `origin/${currentBranch}`, | |
| ); | |
| } catch (err) { | |
| branchExistsOnRemote = false; | |
| } | |
| } | |
| return { | |
| remoteUrl, | |
| currentBranch, | |
| branchExistsOnRemote, | |
| }; | |
| } catch (err: any) { | |
| return null; | |
| } | |
| } | |
| async getRepoInfo(repoPath: string): Promise<RepoInfoRes | null> { | |
| const git = this.getGitInstance(repoPath); | |
| try { | |
| const remotes = await git.getRemotes(true); | |
| const origin = remotes.find((r) => r.name === 'origin'); | |
| let remoteUrl = origin?.refs?.fetch || null; | |
| const branchSummary = await git.branch(); | |
| const currentBranch = branchSummary.current; | |
| let branchExistsOnRemote = false; | |
| if (currentBranch) { | |
| try { | |
| await git.fetch(); | |
| const remoteBranches = await git.branch(['-r']); | |
| branchExistsOnRemote = remoteBranches.all.includes( | |
| `origin/${currentBranch}`, | |
| ); | |
| } catch (err) { | |
| branchExistsOnRemote = false; | |
| } | |
| } | |
| return { | |
| remoteUrl, | |
| currentBranch, | |
| branchExistsOnRemote, | |
| }; | |
| } catch (err: any) { | |
| return null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @src/main/services/git.service.ts around lines 1144 - 1180, getRepoInfo
currently forces ".git" on remoteUrl (variable remoteUrl inside getRepoInfo),
which can produce incorrect addresses for some remotes; remove the conditional
that appends ".git" (or make it conditional on a new optional flag like
normalizeRemoteUrl) so the function returns the original fetch URL from
remotes.find(...).refs.fetch without modification, and adjust callers if they
relied on the forced ".git" suffix.
| const postJson = async (url: string, data?: object): Promise<any> => { | ||
| const apiKey = await this.getApiKey(); | ||
| const response = await fetch(url, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/json', | ||
| Authorization: `Bearer ${apiKey}`, | ||
| }, | ||
| body: data ? JSON.stringify(data) : undefined, | ||
| }); | ||
|
|
||
| return response.json(); | ||
| }; |
There was a problem hiding this comment.
Missing error handling for non-OK responses in postJson.
The nested postJson helper function returns response.json() without checking response.ok. If the API returns an error status (4xx/5xx), this will still attempt to parse JSON which may lead to unexpected behavior or silent failures.
🔎 Proposed fix
const postJson = async (url: string, data?: object): Promise<any> => {
const apiKey = await this.getApiKey();
const response = await fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Accept: 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: data ? JSON.stringify(data) : undefined,
});
+ if (!response.ok) {
+ throw new Error(`Request failed: ${response.status}`);
+ }
+
return response.json();
};🤖 Prompt for AI Agents
In @src/main/services/rosettaCloud.service.ts around lines 28 - 41, The postJson
helper (function postJson) currently returns response.json() without checking
response.ok; update it to check response.ok after fetch and, if false, read and
include the error body (prefer JSON if possible, fallback to text) along with
response.status/statusText in a thrown Error (or custom error) so callers can
handle failures; keep the Authorization logic using this.getApiKey() as-is and
ensure successful responses still return the parsed JSON.
| // Cloud execution state | ||
| const [runInCloudModal, setRunInCloudModal] = useState<DbtCommandType>(); | ||
| const [cloudDbtArguments, setCloudDbtArguments] = useState<string>(''); | ||
|
|
||
| const { | ||
| compile: dbtCompileModel, | ||
| run: dbtRunModel, | ||
| test: dbtTestModel, | ||
| isRunning: isRunningDbtModel, | ||
| list: dbtList, | ||
| build: dbtBuildModel, | ||
| } = useDbt(); | ||
| } = useDbt(undefined, (command) => { | ||
| setRunInCloudModal(command); | ||
| }); | ||
|
|
||
| // Helper function to handle cloud vs local execution | ||
| const executeCommand = async ( | ||
| command: DbtCommandType, | ||
| localHandler: () => Promise<void>, | ||
| dbtArgs?: string, | ||
| ) => { | ||
| if (environment === 'cloud') { | ||
| setCloudDbtArguments(dbtArgs || ''); | ||
| setRunInCloudModal(command); | ||
| } else { | ||
| await localHandler(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find src -name "*useDbt*" -type fRepository: rosettadb/dbt-studio
Length of output: 92
🏁 Script executed:
rg -n "export.*useDbt|function useDbt" --type ts --type tsx src/Repository: rosettadb/dbt-studio
Length of output: 91
🏁 Script executed:
cat -n src/renderer/components/dbtModelButtons/ModelSplitButton.tsx | head -100Repository: rosettadb/dbt-studio
Length of output: 3848
🏁 Script executed:
cat -n src/renderer/hooks/useDbt.tsRepository: rosettadb/dbt-studio
Length of output: 18121
🏁 Script executed:
cat -n src/renderer/components/dbtModelButtons/ModelSplitButton.tsx | sed -n '80,200p'Repository: rosettadb/dbt-studio
Length of output: 4438
🏁 Script executed:
cat -n src/renderer/components/dbtModelButtons/ModelSplitButton.tsx | sed -n '200,350p'Repository: rosettadb/dbt-studio
Length of output: 5777
🏁 Script executed:
cat -n src/renderer/components/dbtModelButtons/ModelSplitButton.tsx | sed -n '350,450p'Repository: rosettadb/dbt-studio
Length of output: 4028
🏁 Script executed:
# Search for direct calls to dbtRunModel, dbtTestModel, dbtBuildModel outside of executeCommand wrapper
rg -n "dbtRunModel\(|dbtTestModel\(|dbtBuildModel\(" src/renderer/components/dbtModelButtons/ModelSplitButton.tsxRepository: rosettadb/dbt-studio
Length of output: 768
🏁 Script executed:
# Also check if executeCommand helper is the ONLY way these handlers are called
rg -n "handleRunModel\|handleTestModel\|handleBuildModel" src/renderer/components/dbtModelButtons/ModelSplitButton.tsxRepository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
rg -n "useAppContext" src/renderer/hooks/useDbt.ts -A 2 -B 2Repository: rosettadb/dbt-studio
Length of output: 470
🏁 Script executed:
# Check where ModelSplitButton is used to see if environment prop comes from the same source
rg -n "ModelSplitButton" src/renderer/components --type ts | head -20Repository: rosettadb/dbt-studio
Length of output: 399
🏁 Script executed:
# Check useAppContext to understand env source
fd "appContext" src/renderer -type f | head -5Repository: rosettadb/dbt-studio
Length of output: 234
🏁 Script executed:
find src/renderer -name "*context*" -o -name "*Context*" | head -10Repository: rosettadb/dbt-studio
Length of output: 383
🏁 Script executed:
rg -n "createContext\|React.createContext" src/renderer --type tsRepository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
# Check where ModelSplitButton receives the environment prop
rg -n "environment=" src/renderer/components -B 2 -A 2 | head -40Repository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
cat src/renderer/hooks/useAppContext.tsRepository: rosettadb/dbt-studio
Length of output: 221
🏁 Script executed:
# Check where ModelSplitButton is instantiated/used
rg -n "<ModelSplitButton" src/renderer --type ts -A 5 | head -60Repository: rosettadb/dbt-studio
Length of output: 1215
🏁 Script executed:
sed -n '789,810p' src/renderer/screens/projectDetails/index.tsxRepository: rosettadb/dbt-studio
Length of output: 1196
🏁 Script executed:
# Check the AppContext definition
cat src/renderer/context/*.tsx 2>/dev/null | head -100Repository: rosettadb/dbt-studio
Length of output: 3145
🏁 Script executed:
sed -n '40,150p' src/renderer/context/index.tsxRepository: rosettadb/dbt-studio
Length of output: 137
🏁 Script executed:
# Check if AppContext.env is ever updated based on settings
rg -n "env:" src/renderer/context -A 2 -B 2Repository: rosettadb/dbt-studio
Length of output: 702
🏁 Script executed:
cat -n src/renderer/context/AppProvider.tsx | sed -n '160,175p'Repository: rosettadb/dbt-studio
Length of output: 558
🏁 Script executed:
# Verify that both environment and env ultimately come from the same settings?.env
rg -n "settings\?.env" src/renderer --type ts | grep -E "(environment=|env:)" | head -10Repository: rosettadb/dbt-studio
Length of output: 371
The two environment mechanisms don't double-trigger, but there's an inconsistent environment check that could cause unexpected behavior.
The handlers (e.g., handleRunModel) always route through the component's executeCommand helper, which checks the environment prop. When environment === 'cloud', it sets the modal state and returns early without calling dbtRunModel, so the useDbt callback never fires.
However, there is an inconsistency: the component receives environment={settings?.env} directly, while the useDbt hook reads env from AppContext, which applies fallback logic (profile ? (settings?.env ?? 'local') : 'local'). For an unauthenticated user with a cloud environment setting, these values could diverge, causing the component to open the cloud modal while the context would otherwise default to local execution.
To avoid confusion, ensure both the component and hook use the same source for environment determination—either both use the context value or both use the prop consistently.
🤖 Prompt for AI Agents
In @src/renderer/components/dbtModelButtons/ModelSplitButton.tsx around lines 52
- 79, The component and the hook are using different environment sources; align
them by reading the environment from AppContext in this component (like useDbt
does) instead of using the environment prop. Import and call
useContext(AppContext) to obtain the same env value/profile fallback, replace
all uses of the environment prop in executeCommand and handlers (e.g.,
executeCommand, handleRunModel) with that context-derived env, and remove
reliance on props?.env so the component and useDbt share the exact same
environment decision logic.
| updateSettings({ | ||
| ...settings!, | ||
| env: newEnv, | ||
| }); |
There was a problem hiding this comment.
Potential crash if settings is undefined.
The non-null assertion settings! assumes settings is always defined when the switch is toggled. If useGetSettings hasn't resolved yet or returns undefined, this will crash.
🔎 Proposed fix
onChange={(event) => {
+ if (!settings) return;
const newEnv = event.target.checked ? 'cloud' : 'local';
updateSettings({
- ...settings!,
+ ...settings,
env: newEnv,
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/renderer/components/menu/index.tsx around lines 244 - 247, The code uses
a non-null assertion on settings when calling updateSettings({...settings!, env:
newEnv}), which can crash if settings is undefined; change the call to guard
against undefined by either early-returning when settings is falsy or merging
with a safe default (e.g., updateSettings({ ...(settings ?? {}), env: newEnv }))
and ensure this logic lives where updateSettings, settings (from useGetSettings)
and newEnv are referenced so the switch toggle cannot dereference undefined.
| <TextField | ||
| value={env.key} | ||
| onChange={(e) => | ||
| updateEnvironmentVariable( | ||
| env.id, | ||
| e.target.value, | ||
| env.value, | ||
| ) | ||
| } | ||
| onFocus={() => handleEnvFocus(env.id)} | ||
| onBlur={() => handleEnvBlur(env.id)} |
There was a problem hiding this comment.
Focus handler attached to key field may cause unintended behavior.
The handleEnvFocus and handleEnvBlur handlers (lines 892-893) are attached to the key field, but these handlers clear the value and track value edits. This means focusing on the key field will clear the value field and mark it as edited, which seems unintended.
🔎 Proposed fix
Remove focus/blur handlers from the key field—they should only be on the value field:
<TextField
value={env.key}
onChange={(e) =>
updateEnvironmentVariable(
env.id,
e.target.value,
env.value,
)
}
- onFocus={() => handleEnvFocus(env.id)}
- onBlur={() => handleEnvBlur(env.id)}
variant="outlined"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TextField | |
| value={env.key} | |
| onChange={(e) => | |
| updateEnvironmentVariable( | |
| env.id, | |
| e.target.value, | |
| env.value, | |
| ) | |
| } | |
| onFocus={() => handleEnvFocus(env.id)} | |
| onBlur={() => handleEnvBlur(env.id)} | |
| <TextField | |
| value={env.key} | |
| onChange={(e) => | |
| updateEnvironmentVariable( | |
| env.id, | |
| e.target.value, | |
| env.value, | |
| ) | |
| } | |
| variant="outlined" |
🤖 Prompt for AI Agents
In @src/renderer/components/modals/pushToCloudModal/index.tsx around lines 883 -
893, The key TextField is incorrectly wired to handleEnvFocus and handleEnvBlur
which clear and mark the value as edited; remove the onFocus and onBlur props
from the key TextField (the instance using value={env.key} and
updateEnvironmentVariable(env.id, e.target.value, env.value)) and ensure those
focus/blur handlers remain only on the value TextField (the one that edits
env.value) so focus behavior only affects the value field.
| export const useGetLocalChanges = ( | ||
| path: string, | ||
| customOptions?: UseQueryOptions< | ||
| GitChangesRes | null, | ||
| CustomError, | ||
| GitChangesRes | null | ||
| >, | ||
| ) => { | ||
| return useQuery({ | ||
| queryKey: [QUERY_KEYS.GIT_LOCAL_CHANGES], | ||
| queryFn: async () => { | ||
| return gitServices.getLocalChanges(path); | ||
| }, | ||
| ...customOptions, | ||
| }); | ||
| }; | ||
|
|
||
| export const useGetRepoInfo = ( | ||
| path: string, | ||
| customOptions?: UseQueryOptions< | ||
| RepoInfoRes | null, | ||
| CustomError, | ||
| RepoInfoRes | null | ||
| >, | ||
| ) => { | ||
| return useQuery({ | ||
| queryKey: [QUERY_KEYS.GIT_REPO_INFO], | ||
| queryFn: async () => { | ||
| return gitServices.getRepoInfo(path); | ||
| }, | ||
| ...customOptions, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Query keys missing path parameter will cause incorrect caching across projects.
Both useGetLocalChanges and useGetRepoInfo pass path to the query function but don't include it in the queryKey. This means React Query will return cached results from one project when querying a different project, causing incorrect data to be displayed.
🔎 Proposed fix
export const useGetLocalChanges = (
path: string,
customOptions?: UseQueryOptions<
GitChangesRes | null,
CustomError,
GitChangesRes | null
>,
) => {
return useQuery({
- queryKey: [QUERY_KEYS.GIT_LOCAL_CHANGES],
+ queryKey: [QUERY_KEYS.GIT_LOCAL_CHANGES, path],
queryFn: async () => {
return gitServices.getLocalChanges(path);
},
...customOptions,
});
};
export const useGetRepoInfo = (
path: string,
customOptions?: UseQueryOptions<
RepoInfoRes | null,
CustomError,
RepoInfoRes | null
>,
) => {
return useQuery({
- queryKey: [QUERY_KEYS.GIT_REPO_INFO],
+ queryKey: [QUERY_KEYS.GIT_REPO_INFO, path],
queryFn: async () => {
return gitServices.getRepoInfo(path);
},
...customOptions,
});
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useGetLocalChanges = ( | |
| path: string, | |
| customOptions?: UseQueryOptions< | |
| GitChangesRes | null, | |
| CustomError, | |
| GitChangesRes | null | |
| >, | |
| ) => { | |
| return useQuery({ | |
| queryKey: [QUERY_KEYS.GIT_LOCAL_CHANGES], | |
| queryFn: async () => { | |
| return gitServices.getLocalChanges(path); | |
| }, | |
| ...customOptions, | |
| }); | |
| }; | |
| export const useGetRepoInfo = ( | |
| path: string, | |
| customOptions?: UseQueryOptions< | |
| RepoInfoRes | null, | |
| CustomError, | |
| RepoInfoRes | null | |
| >, | |
| ) => { | |
| return useQuery({ | |
| queryKey: [QUERY_KEYS.GIT_REPO_INFO], | |
| queryFn: async () => { | |
| return gitServices.getRepoInfo(path); | |
| }, | |
| ...customOptions, | |
| }); | |
| }; | |
| export const useGetLocalChanges = ( | |
| path: string, | |
| customOptions?: UseQueryOptions< | |
| GitChangesRes | null, | |
| CustomError, | |
| GitChangesRes | null | |
| >, | |
| ) => { | |
| return useQuery({ | |
| queryKey: [QUERY_KEYS.GIT_LOCAL_CHANGES, path], | |
| queryFn: async () => { | |
| return gitServices.getLocalChanges(path); | |
| }, | |
| ...customOptions, | |
| }); | |
| }; | |
| export const useGetRepoInfo = ( | |
| path: string, | |
| customOptions?: UseQueryOptions< | |
| RepoInfoRes | null, | |
| CustomError, | |
| RepoInfoRes | null | |
| >, | |
| ) => { | |
| return useQuery({ | |
| queryKey: [QUERY_KEYS.GIT_REPO_INFO, path], | |
| queryFn: async () => { | |
| return gitServices.getRepoInfo(path); | |
| }, | |
| ...customOptions, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In @src/renderer/controllers/git.controller.ts around lines 117 - 149, Both
hooks use a static queryKey which omits the path, causing cached responses to
bleed across repositories; update useGetLocalChanges and useGetRepoInfo to
include the path in their queryKey (e.g., append the path to
QUERY_KEYS.GIT_LOCAL_CHANGES and QUERY_KEYS.GIT_REPO_INFO) so the queryKey
uniquely identifies the repo, leaving the rest of the useQuery config and
queryFn (gitServices.getLocalChanges(path) / gitServices.getRepoInfo(path))
unchanged.
| export const useGetSecrets = ( | ||
| projectId?: string, | ||
| options?: UseQueryOptions<Secret[], CustomError, Secret[]>, | ||
| ) => { | ||
| return useQuery({ | ||
| queryKey: [QUERY_KEYS.CLOUD_SECRETS], | ||
| queryFn: () => rosettaCloudServices.getSecrets(projectId ?? ''), | ||
| ...options, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Query key should include projectId for proper cache isolation.
The queryKey only includes QUERY_KEYS.CLOUD_SECRETS, but the query depends on projectId. If different projects are queried, React Query will return cached data from the previous project until the cache is invalidated.
🔎 Proposed fix
export const useGetSecrets = (
projectId?: string,
options?: UseQueryOptions<Secret[], CustomError, Secret[]>,
) => {
return useQuery({
- queryKey: [QUERY_KEYS.CLOUD_SECRETS],
+ queryKey: [QUERY_KEYS.CLOUD_SECRETS, projectId],
queryFn: () => rosettaCloudServices.getSecrets(projectId ?? ''),
+ enabled: !!projectId, // Only fetch when projectId is provided
...options,
});
};🤖 Prompt for AI Agents
In @src/renderer/controllers/rosettaCloud.controller.ts around lines 76 - 85,
The query key for useGetSecrets must include the projectId to avoid
cross-project cache collisions; update the queryKey in the useGetSecrets
function (currently using QUERY_KEYS.CLOUD_SECRETS) to include the projectId
(e.g., [QUERY_KEYS.CLOUD_SECRETS, projectId ?? '']) so each project has an
isolated cache entry while keeping the existing queryFn that calls
rosettaCloudServices.getSecrets(projectId ?? '') and preserving passed options.
| if (env === 'cloud') { | ||
| cloudRunCb?.(command); | ||
| return; | ||
| } |
There was a problem hiding this comment.
activeCommand is never reset when env === 'cloud'.
When the cloud path is taken, setActiveCommand(command) at line 225 sets the active command, but the early return bypasses the finally block (lines 265-267) that resets it to null. This leaves activeCommand stuck indefinitely.
🔎 Proposed fix
if (env === 'cloud') {
cloudRunCb?.(command);
+ setActiveCommand(null);
return;
}🤖 Prompt for AI Agents
In @src/renderer/hooks/useDbt.ts around lines 227 - 230, When env === 'cloud'
the early return prevents the finally block from clearing activeCommand, leaving
the state stuck; update the cloud branch in the run flow (in useDbt) to ensure
setActiveCommand(null) is invoked before returning — either call
setActiveCommand(null) immediately after cloudRunCb?.(command) or refactor so
the finally block always runs (e.g., avoid returning early in the try and
instead short-circuit after performing cleanup). Target symbols: activeCommand,
setActiveCommand, cloudRunCb, and the run function in useDbt to apply the fix.
| emailVerified: Date | null; | ||
| createdAt: Date; | ||
| updatedAt: Date; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the profile.ts file to confirm the current state
cat -n src/types/profile.tsRepository: rosettadb/dbt-studio
Length of output: 919
🏁 Script executed:
# Search for profile service and how data is fetched
fd -t f 'profile.service' src/ && find src/ -name '*profile*.service*' -o -name '*profile*service*'Repository: rosettadb/dbt-studio
Length of output: 145
🏁 Script executed:
# Look for any date transformation utilities
rg -n 'new Date|Date\.parse|toDate|parseDate|dateTransform' --type ts src/ -A 2Repository: rosettadb/dbt-studio
Length of output: 38838
🏁 Script executed:
# Check how these fields are actually used in the codebase
rg -n 'emailVerified|createdAt|updatedAt' --type ts src/ -B 2 -A 2 | head -100Repository: rosettadb/dbt-studio
Length of output: 5571
🏁 Script executed:
cat -n src/renderer/services/profile.service.tsRepository: rosettadb/dbt-studio
Length of output: 1036
🏁 Script executed:
# Also check for any API response interceptors or transformers
rg -n 'axios|fetch|interceptor|transform' src/renderer/services/profile.service.ts -A 3 -B 1Repository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
# Check the actual API endpoint and response handling
rg -n 'getProfile|ProfileResponse|UserProfile' src/renderer/services/profile.service.ts -A 5Repository: rosettadb/dbt-studio
Length of output: 826
🏁 Script executed:
# Check the client configuration for response interceptors or transformers
cat -n src/renderer/config/client.tsRepository: rosettadb/dbt-studio
Length of output: 987
🏁 Script executed:
# Search for any IPC or response interceptor patterns that might handle date transformation
rg -n 'ipcRenderer|transform|Date|parse' src/renderer/config/ -A 2 -B 1Repository: rosettadb/dbt-studio
Length of output: 1749
🏁 Script executed:
# Check how the profile data is used in components to understand if dates are expected as Date objects
rg -n 'emailVerified|profile\.createdAt|profile\.updatedAt' src/renderer/ -A 2 -B 1Repository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
# Check the main process to see how rosettaCloud:getProfile is implemented
rg -n 'rosettaCloud.*getProfile|getProfile.*Rosetta' src/main/ -A 5Repository: rosettadb/dbt-studio
Length of output: 596
🏁 Script executed:
# Search for the actual IPC handler for getProfile
rg -n 'handle.*getProfile|getProfile.*handle' src/main/ -A 5Repository: rosettadb/dbt-studio
Length of output: 596
🏁 Script executed:
# Check the rosettaCloud.service for profile fetching
rg -n 'getProfile|class.*Cloud|UserProfile' src/main/services/rosettaCloud.service.ts -A 5 | head -80Repository: rosettadb/dbt-studio
Length of output: 1247
🏁 Script executed:
# Get the full implementation of getProfile in RosettaCloudService
sed -n '157,197p' src/main/services/rosettaCloud.service.tsRepository: rosettadb/dbt-studio
Length of output: 1220
🏁 Script executed:
# Check if there's any JSON.parse or date transformation happening during profile retrieval
rg -n 'JSON.parse|new Date|Date\.parse|parseISO' src/main/services/rosettaCloud.service.ts -A 2 -B 2Repository: rosettadb/dbt-studio
Length of output: 329
🏁 Script executed:
# Look for the actual API response and how it's handled
rg -n 'fetch.*getProfile|axios.*profile|response.*profile' src/main/services/rosettaCloud.service.ts -A 5Repository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
# Check how profile is used in renderer components - look for date operations
rg -n 'useProfile|profile\.|getProfile\|refreshProfile' src/renderer/ --type tsx --type ts -A 2 | grep -A 2 'createdAt\|updatedAt\|emailVerified' | head -40Repository: rosettadb/dbt-studio
Length of output: 91
🏁 Script executed:
# Look for any custom hooks or context that might transform the profile data
fd -t f 'useProfile\|ProfileContext\|ProfileProvider' src/renderer/Repository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
# Check if there are any usages in React components that indicate how the dates are being used
rg -n 'profile\.createdAt\|profile\.updatedAt\|profile\.emailVerified' src/renderer/screens/ src/renderer/components/ -A 2Repository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
# Search more broadly for profile usage in all components
rg -n 'profile\.' src/renderer/components/ src/renderer/screens/ src/renderer/controllers/ -A 1 | head -80Repository: rosettadb/dbt-studio
Length of output: 1717
🏁 Script executed:
# Check if profile context or hooks exist
rg -n 'useAuth|useProfile|ProfileContext|useUser' src/renderer/ -A 3 | head -60Repository: rosettadb/dbt-studio
Length of output: 4135
🏁 Script executed:
# Look for any date parsing or usage of profile date fields
rg -n 'toLocaleDateString|toISOString|getTime|moment.*profile' src/renderer/ -B 2 -A 1 | head -40Repository: rosettadb/dbt-studio
Length of output: 3299
🏁 Script executed:
# Check profile.controller.ts to see if useProfile does any transformation
cat -n src/renderer/controllers/profile.controller.tsRepository: rosettadb/dbt-studio
Length of output: 3996
🏁 Script executed:
# Look for any query transformers or response processors in React Query setup
rg -n 'queryClient|QueryClient|transformer|select' src/renderer/controllers/profile.controller.ts -A 3Repository: rosettadb/dbt-studio
Length of output: 1166
🏁 Script executed:
# Check if the profile date fields are ever used with Date methods anywhere
rg -n 'profile\.createdAt\.|profile\.updatedAt\.|profile\.emailVerified\.' src/renderer/ -A 2Repository: rosettadb/dbt-studio
Length of output: 46
🏁 Script executed:
# Also check how the profile object is serialized/used to understand if this causes actual issues
rg -n 'JSON\.stringify.*profile|profileService\|useProfile' src/renderer/components/settings/ src/renderer/context/ -A 3Repository: rosettadb/dbt-studio
Length of output: 46
Ensure Date fields are properly transformed when parsing API responses, or adjust type definitions.
emailVerified, createdAt, and updatedAt are typed as Date in the UserProfile interface, but JSON API responses deserialize these fields as ISO strings. The profile service returns the raw API response without transformation, creating a type-to-runtime mismatch. If components attempt to call Date methods on these fields (e.g., .getTime(), .toLocaleDateString()), they will fail at runtime.
Either add date parsing in RosettaCloudService.getProfile() to convert these fields to Date objects, or update the type definitions to reflect that they arrive as strings.
🤖 Prompt for AI Agents
In @src/types/profile.ts around lines 16 - 18, UserProfile declares
emailVerified, createdAt and updatedAt as Date but
RosettaCloudService.getProfile() returns raw JSON ISO strings; fix by parsing
those ISO strings into Date objects inside RosettaCloudService.getProfile()
(convert emailVerified nullable ISO to Date|null and createdAt/updatedAt to
Date) before returning, or alternatively change UserProfile types to string (and
emailVerified to string|null) and update all callers that use Date methods;
reference UserProfile and RosettaCloudService.getProfile() when making the
change so callers like components expecting .getTime() or .toLocaleDateString()
are updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/publish.yml:
- Line 8: Remove the temporary feature branch entry "feature/authentication"
from the publish workflow triggers: edit the publish.yml workflow's
push/branches array and delete the "feature/authentication" entry so the
workflow only triggers on the intended branches (e.g., dev/main); ensure no
leftover references to that branch remain before merging.
🧹 Nitpick comments (2)
release/app/package.json (1)
3-3: Consider a minor version bump for new features.This PR introduces significant new functionality (Rosetta Cloud integration, authentication, API key storage, Git repository status, deployment features). Per semver conventions, new backwards-compatible features typically warrant a MINOR version increment (e.g.,
1.3.0) rather than a PATCH increment (1.2.7).If the project follows a different versioning policy, feel free to disregard.
src/types/backend.ts (1)
205-214: Consider consistent naming convention forCUSTOM_DBT_COMMANDS.The field
CUSTOM_DBT_COMMANDSuses SCREAMING_SNAKE_CASE while all other fields in this type use camelCase (gitUrl,gitBranch,githubUsername, etc.). Consider renaming tocustomDbtCommandsfor consistency.Suggested change
export type CloudDeploymentPayload = { id: string; title: string; gitUrl: string; gitBranch: string; githubUsername?: string; githubPassword?: string; secrets: Record<string, string>; - CUSTOM_DBT_COMMANDS?: string; + customDbtCommands?: string; };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonrelease/app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.github/workflows/publish.ymlpackage.jsonrelease/app/package.jsonsrc/renderer/screens/projectDetails/index.tsxsrc/types/backend.tssrc/types/ipc.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/ipc.ts
🔇 Additional comments (7)
src/renderer/screens/projectDetails/index.tsx (2)
796-804: LGTM!The
environmentprop is appropriately passed toModelSplitButton, enabling environment-aware behavior (cloud vs local). The optional chaining onsettings?.envcorrectly handles cases where settings may be undefined.
806-817: LGTM!The
environmentprop is consistently passed toProjectDbtSplitButton, matching the pattern used forModelSplitButton. This enables the component to internally manage cloud vs local execution paths based on the current environment setting.src/types/backend.ts (5)
201-202: LGTM!The new optional fields
externalIdandlastRunare well-typed and consistent with the existing patterns in theProjecttype (e.g.,createdAt: string).
280-281: LGTM!The
envfield with literal union type'local' | 'cloud'is a clean approach for environment switching. Being optional allows backward compatibility.
738-745: LGTM!Well-structured type with both boolean flags for quick checks and numeric counts for detailed information. The naming is clear and self-documenting.
747-751: LGTM!The type correctly handles the case where a repository might not have a remote configured by making
remoteUrlnullable.
753-757: Verify plaintext transmission of RosettaCloud secrets over IPC.The
Secrettype values are fetched from the RosettaCloud API and transmitted to the renderer as plaintext over the IPC channel viarosettaCloud:getSecrets. If these secrets contain sensitive credentials, consider whether they should be:
- Encrypted before IPC transmission, or
- Kept in the main process and only referenced by ID in the renderer
Note: Local cloud credentials (AWS, Azure, GCS) are properly secured via
secureStorageService.
| branches: | ||
| - main | ||
| - release/dev | ||
| - feature/authentication |
There was a problem hiding this comment.
Remove feature branch from publish triggers before merging.
Adding a feature branch to the publish workflow will create draft releases from unfinished code. This appears to be a temporary addition for testing purposes. Ensure this line is removed before merging to dev to avoid:
- Orphan draft releases from the feature branch
- Unnecessary CI/CD resource consumption (macOS runners, code signing)
- Potential release tag conflicts if the version matches an existing release
🤖 Prompt for AI Agents
In @.github/workflows/publish.yml at line 8, Remove the temporary feature branch
entry "feature/authentication" from the publish workflow triggers: edit the
publish.yml workflow's push/branches array and delete the
"feature/authentication" entry so the workflow only triggers on the intended
branches (e.g., dev/main); ensure no leftover references to that branch remain
before merging.
# Conflicts: # .gitignore # package-lock.json # package.json # release/app/package-lock.json # release/app/package.json
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/ipcHandlers/git.ipcHandlers.ts (1)
13-35:⚠️ Potential issue | 🟠 MajorAdd new channels to
handlerChannelsto avoid duplicate handler registration.
removeGitIpcHandlers()won’t remove the newly added handlers, so re-registering can throw due to existing handlers.Proposed fix
const handlerChannels = [ 'git:init', 'git:clone', 'git:listBranches', 'git:checkout', 'git:addRemote', 'git:isInitialized', 'git:getRemotes', 'git:add', 'git:unstage', 'git:stageAll', 'git:unstageAll', 'git:discardChanges', 'git:commit', 'git:pull', 'git:push', 'git:fileDiff', 'git:fileStatusList', 'git:aheadBehind', 'git:createBranch', 'git:deleteBranch', 'git:renameBranch', + 'git:getLocalChanges', + 'git:repoInfo', ];
🧹 Nitpick comments (1)
src/renderer/components/dbtModelButtons/ModelSplitButton.tsx (1)
727-731: Avoid disabling eslint by renaming the unused property.You can avoid the inline eslint disable by aliasing the unused key.
🛠️ Suggested tweak
- menuItems={filteredMenuItems.map((item) => { - // eslint-disable-next-line `@typescript-eslint/no-unused-vars` - const { localOnly, ...menuItem } = item; - return menuItem; - })} + menuItems={filteredMenuItems.map((item) => { + const { localOnly: _localOnly, ...menuItem } = item; + return menuItem; + })}
2 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes