feat: Add CI/CD workflows and build infrastructure improvements #74
Conversation
- Add GitHub Actions workflows for CI/CD - Add Claude integration for AI-powered development - Implement user profile store with multi-profile support - Add encryption service for secure data storage - Refactor update service with improved scheduling and rollback - Add user analytics service for cohort tracking - Add performance monitoring utilities - Update store infrastructure with initialization lifecycle This is the foundational layer (1/6) from PR #45 that other features depend on. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughThis update introduces a wide-ranging set of new features, modules, and infrastructure for an Electron application and its development workflow. Major additions include a robust user profile store, update management services, analytics tracking, performance monitoring, new utility modules, and comprehensive workflow documentation and configuration for CI, PRs, and codebase research. The changes also add several new TypeScript interfaces and classes to support these features. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant MainProcess
participant UpdateService
participant UpdateNotifier
participant ActivityDetector
participant UpdateScheduler
participant UpdateRollback
Renderer->>MainProcess: IPC - Check for updates
MainProcess->>UpdateService: checkForUpdates()
UpdateService->>UpdateNotifier: showUpdateNotification()
Note right of UpdateService: Tracks download progress, schedules, and user activity
UpdateService->>ActivityDetector: isUserInactive()
alt User is inactive
UpdateService->>UpdateScheduler: scheduleUpdate()
UpdateScheduler-->>UpdateService: Confirmation
UpdateService->>UpdateNotifier: showScheduledUpdateNotification()
else User is active
UpdateService->>UpdateNotifier: showUpdateReadyNotification()
end
UpdateService->>UpdateRollback: rollbackToVersion(version)
UpdateRollback-->>UpdateService: Rollback result
UpdateService->>Renderer: IPC - Update result/progress
sequenceDiagram
participant App
participant UserAnalyticsService
participant Sentry
participant Umami
App->>UserAnalyticsService: initialize()
App->>UserAnalyticsService: startFeatureTimer(feature)
App->>UserAnalyticsService: endFeatureTimer(feature)
UserAnalyticsService->>Sentry: Record breadcrumb/event
UserAnalyticsService->>Umami: Track event
App->>UserAnalyticsService: trackNavigation(event, data)
App->>UserAnalyticsService: trackSessionEnd()
UserAnalyticsService->>Sentry: Session stats/cohort tagging
UserAnalyticsService->>Umami: Usage stats
sequenceDiagram
participant App
participant UserProfileStore
participant EncryptionService
participant Disk
App->>UserProfileStore: initialize()
UserProfileStore->>Disk: Load profiles JSON
UserProfileStore->>EncryptionService: decrypt(secureSettings)
App->>UserProfileStore: visitPage(url, title)
App->>UserProfileStore: getPasswords()
UserProfileStore->>EncryptionService: decrypt(passwords)
App->>UserProfileStore: setSetting(key, value)
UserProfileStore->>Disk: Save profiles JSON (debounced)
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @maceip's task —— View job PR Review CompleteReview Checklist:
🔒 Security ConcernsCritical:
Moderate:
🏗️ Code Quality & ArchitectureStrengths:
Areas for Improvement:
🚀 Performance ConsiderationsPotential Issues:
Recommendations:
🔧 GitHub Actions Workflow ReviewSecurity:
Improvements:
📊 Test CoverageMissing Areas:
Recommendations:
🐛 Potential Bugs
💡 RecommendationsHigh Priority:
Medium Priority:
Low Priority:
🎯 Overall AssessmentThis PR introduces significant infrastructure improvements with well-architected services. The code quality is generally good with proper TypeScript usage and error handling. However, there are security concerns that need immediate attention, particularly around hardcoded values and data privacy considerations. Recommendation: Request changes to address the critical security issues before merging. The architecture is solid but needs refinement in key areas. |
|
@maceip Can you clarify why there are 26k lines of changes in this PR for CI/CD? I noticed the CI/CD scripts already exist in main and seem duplicated here. Since the PR title is '[feat: Add CI/CD workflows and build infrastructure improvements]', should I assume most of these changes are infrastructure-related? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🔭 Outside diff range comments (1)
apps/electron-app/src/main/store/user-profile-store.ts (1)
1-2175: Consider breaking down this large store into smaller, focused modules.This 2175-line file handles multiple responsibilities that could be separated:
- Profile management
- Navigation history
- Download management
- Import/export operations
- Session management
- Encryption operations
Consider splitting into:
user-profile-store.ts- Core profile operationsprofile-navigation-store.ts- Navigation historyprofile-downloads-store.ts- Download managementprofile-import-export.ts- Import/export utilitiesprofile-session-manager.ts- Session managementThis would improve maintainability, testability, and code organization.
🧹 Nitpick comments (22)
.claude/settings.local.json (1)
31-34: Remove duplicate permission entries.There are duplicate entries for
pnpm buildandgit commitcommands."Bash(pnpm build:*)", - "Bash(pnpm build:*)", - "Bash(git commit:*)", "Bash(pnpm build:*)",apps/electron-app/src/main/store/types.ts (1)
10-10: Consider timeline for moving to shared-types.The comment indicates this is a temporary local definition. Consider creating an issue to track when this should be moved to shared-types to avoid it being forgotten.
Do you want me to help create a tracking issue for moving DownloadItem to shared-types?
.claude/commands/commit.md (1)
5-5: Fix markdown linting issues with trailing punctuation in headings.The markdownlint tool flagged trailing colons in headings which should be removed for better markdown compliance.
-## Process: +## Process -## Important: +## Important -## Remember: +## RememberAlso applies to: 29-29, 37-37
apps/electron-app/src/main/utils/performanceMonitor.ts (1)
54-54: Use more precise frame timing constant.The hardcoded
16.67should be defined as a constant for better maintainability and clarity.+ private static readonly FRAME_TIME_60FPS = 1000 / 60; // 16.67ms + // Log if update is slow - if (duration > 16.67) { + if (duration > MainProcessPerformanceMonitor.FRAME_TIME_60FPS) { // More than 1 frame at 60fpsapps/electron-app/src/main/services/update/activity-detector.ts (1)
125-133: Improve confidence score calculation for better suggestions.The confidence calculation
1 - activitycan result in very high confidence scores for hours with minimal activity, which might not be ideal for update scheduling.Consider a more nuanced confidence scoring:
inactiveHours.forEach(({ hour, activity }) => { const time = `${hour.toString().padStart(2, "0")}:00`; - const confidence = 1 - activity; // Higher confidence for lower activity + // Scale confidence based on activity level and add minimum threshold + const confidence = Math.max(0.1, Math.min(0.9, (1 - activity) * 0.8)); suggestions.push({ time, confidence, reason: `Low activity period (${(activity * 100).toFixed(1)}% of peak)`, }); });apps/electron-app/src/main/store/store.ts (2)
56-59: Consider encapsulating initialization state within the store object.The module-level state variables could create issues if multiple store instances are needed in the future or during testing.
Consider moving the initialization state into the store interface:
-// Store initialization state -let isInitialized = false; -let initializationPromise: Promise<void> | null = null; -let lastError: Error | null = null; +interface StoreState { + isInitialized: boolean; + initializationPromise: Promise<void> | null; + lastError: Error | null; +} + +const storeState: StoreState = { + isInitialized: false, + initializationPromise: null, + lastError: null, +};This would make the store more testable and prevent potential issues with shared state.
130-137: Consider preserving data during cleanup.The cleanup method completely resets the store to initial state, which might not be desired in all scenarios. Consider whether this should be optional or if there should be a separate reset method.
/** - * Clean up the store state + * Clean up the store state and optionally reset data */ -const cleanup = (): void => { +const cleanup = (resetData: boolean = true): void => { isInitialized = false; initializationPromise = null; lastError = null; - // Reset to initial state - zustandStore.setState(initialState, true); + if (resetData) { + // Reset to initial state + zustandStore.setState(initialState, true); + } };.claude/commands/pr.md (2)
5-5: Fix markdown formatting: remove trailing punctuation from heading.The heading has trailing punctuation which violates markdown linting rules.
-## Steps to follow: +## Steps to follow
63-63: Fix markdown formatting: remove trailing punctuation from heading.The heading has trailing punctuation which violates markdown linting rules.
-## Important notes: +## Important notesapps/electron-app/src/main/services/update/update-scheduler.ts (1)
129-131: Improve ID generation for better uniqueness and security.The current ID generation using
Math.random()could potentially create collisions and is not cryptographically secure.Use Node.js crypto module for better uniqueness:
+import { randomBytes } from "crypto"; private generateId(): string { - return `update_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; + return `update_${Date.now()}_${randomBytes(6).toString('hex')}`; }.claude/commands/learn.md (4)
9-11: Add language specifier to code block.-``` +```text I'm ready to research the codebase. Please provide your research question or area of interest, and I'll analyze it thoroughly by exploring relevant components and connections.
41-49: Add language specifier to code block.- ``` + ```text Research [specific component/pattern] in [directory/module]:
70-122: Add language specifier to markdown code block.- ```markdown + ```markdown ---Note: The language specifier is already present, but ensure all other code blocks in lines 17, 59, 83, 118, 129, 139, and 153 have appropriate language specifiers added.
15-15: Remove redundant phrasing.The phrase "steps to follow after" is redundant.
-## Steps to follow after receiving the research query: +## Steps after receiving the research query:apps/electron-app/src/main/utils/debounce.ts (1)
8-168: Consider refactoring static-only class to module functions.The
DebounceManagerclass contains only static members. This pattern can be confusing and goes against TypeScript best practices. Consider refactoring to module-level functions with a shared state object.// Instead of a static-only class, use module-level state and functions const debounceState = { timers: new Map<string, NodeJS.Timeout>(), callbacks: new Map<string, (...args: any[]) => any>(), }; export function createManagedDebounce<T extends (...args: any[]) => any>( key: string, fn: T, delay: number = 300, ): (...args: Parameters<T>) => void { return (...args: Parameters<T>) => { clearDebounceTimer(key); debounceState.callbacks.set(key, () => fn(...args)); const timer = setTimeout(() => { try { const callback = debounceState.callbacks.get(key); if (callback) { callback(); } } catch (error) { logger.error(`Debounced function error for key '${key}':`, error); } finally { debounceState.timers.delete(key); debounceState.callbacks.delete(key); } }, delay); debounceState.timers.set(key, timer); }; } export function cancelDebounce(key: string): boolean { return clearDebounceTimer(key); } // ... export other functions similarlyThis approach:
- Avoids the confusion of
thisin static context- Is more tree-shakeable
- Follows functional programming patterns
- Is easier to test
apps/electron-app/src/main/config/app-config.ts (2)
360-382: Enhance type safety indeepMergemethod.The
deepMergemethod usesanytypes which reduces type safety. Consider using generic types for better type preservation.- private deepMerge(...objects: any[]): any { - const result: any = {}; + private deepMerge<T extends Record<string, any>>(...objects: Partial<T>[]): T { + const result = {} as T; for (const obj of objects) { if (obj && typeof obj === "object") { for (const key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) { + const objKey = key as keyof T; if ( - typeof obj[key] === "object" && - !Array.isArray(obj[key]) && - obj[key] !== null + typeof obj[objKey] === "object" && + !Array.isArray(obj[objKey]) && + obj[objKey] !== null ) { - result[key] = this.deepMerge(result[key] || {}, obj[key]); + result[objKey] = this.deepMerge( + (result[objKey] || {}) as any, + obj[objKey] as any + ) as T[keyof T]; } else { - result[key] = obj[key]; + result[objKey] = obj[objKey] as T[keyof T]; } } } } } return result; }
433-455: Add return type annotations to getter methods.The configuration getter methods are missing return type annotations.
- public getPerformanceConfig() { + public getPerformanceConfig(): AppConfig['performance'] { return this.config.performance; } - public getNetworkConfig() { + public getNetworkConfig(): AppConfig['network'] { return this.config.network; } - public getUIConfig() { + public getUIConfig(): AppConfig['ui'] { return this.config.ui; } - public getWorkersConfig() { + public getWorkersConfig(): AppConfig['workers'] { return this.config.workers; } - public getDevelopmentConfig() { + public getDevelopmentConfig(): AppConfig['development'] { return this.config.development; } - public getSecurityConfig() { + public getSecurityConfig(): AppConfig['security'] { return this.config.security; }apps/electron-app/src/main/services/encryption-service.ts (1)
212-222: Improve encryption detection heuristicThe current patterns are too broad and may produce false positives. Consider checking for the specific format used by your encryption methods.
Since your fallback encryption uses the format
{iv}:{encrypted}, check for this pattern:public isEncrypted(data: string): boolean { if (!data) { return false; } - // Simple heuristic: encrypted data is typically base64 or hex - const base64Pattern = /^[A-Za-z0-9+/]*={0,2}$/; - const hexPattern = /^[0-9a-fA-F]+$/; - - return base64Pattern.test(data) || hexPattern.test(data); + // Check for safeStorage base64 format (min length for encrypted data) + const base64Pattern = /^[A-Za-z0-9+/]{20,}={0,2}$/; + + // Check for fallback format: {32 hex chars for IV}:{hex encrypted data} + const fallbackPattern = /^[0-9a-fA-F]{32}:[0-9a-fA-F]+$/; + + return base64Pattern.test(data) || fallbackPattern.test(data); }.claude/commands/plan.md (1)
17-17: Add language specifiers to fenced code blocksMultiple code blocks are missing language specifiers, which affects syntax highlighting and readability.
Add appropriate language specifiers to all code blocks. For example:
- Lines 17, 83, 153, 177, 297, 314, 458: Use
```text- Lines 47, 59, 118, 129, 139: Use
```textor```bashdepending on contentExample:
-``` +```text I'll help you create a detailed implementation plan. Let me start by understanding what we're building.Also applies to: 47-47, 59-59, 83-83, 118-118, 129-129, 139-139, 153-153, 177-177, 297-297, 314-314, 458-458
apps/electron-app/src/main/services/update/update-notifier.ts (1)
23-25: Remove unnecessary constructorThe constructor doesn't perform any initialization and can be removed.
Remove the empty constructor:
export class UpdateNotifier { private isInitialized = false; private notificationCallbacks: Map<string, () => void> = new Map(); - constructor() { - // Initialize with default values - } - public async initialize(): Promise<void> {apps/electron-app/src/main/services/user-analytics.ts (1)
483-642: Consider extracting common breadcrumb logicThe
monitorPerformanceandmonitorPerformanceSyncmethods have very similar breadcrumb creation logic that could be extracted to reduce duplication.Extract common breadcrumb creation:
private createPerformanceBreadcrumb( category: string, operationName: string, operationId: string, context?: any, additionalData?: any ): void { Sentry.addBreadcrumb({ category, message: `${category.includes('start') ? 'Started' : category.includes('success') ? 'Completed' : 'Failed'} ${operationName}`, level: category.includes('error') ? 'error' : 'info', data: { operationName, operationId, context, ...additionalData, }, }); }Then use it in both methods to reduce duplication.
apps/electron-app/src/types/metadata.ts (1)
276-310: Consider improving type safety in validation functions.While the validation logic is correct, consider using the same pattern as suggested for the type guards to avoid
as anycasts.validateNavigationHistory(data: unknown): data is NavigationHistoryMetadata { if (typeof data !== "object" || data === null) return false; - const obj = data as any; + const obj = data as Record<string, unknown>; return ( typeof obj.url === "string" && typeof obj.visitCount === "number" && typeof obj.lastVisit === "number" && - obj.visitCount >= 0 && - obj.lastVisit > 0 + Number(obj.visitCount) >= 0 && + Number(obj.lastVisit) > 0 ); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.claude/commands/commit.md(1 hunks).claude/commands/learn.md(1 hunks).claude/commands/plan.md(1 hunks).claude/commands/pr.md(1 hunks).claude/settings.local.json(1 hunks).claude/shared/pr_template.md(1 hunks).github/workflows/pr.yml(1 hunks).github/workflows/push.yml(1 hunks).github/workflows/slack-notifications.yml(1 hunks)apps/electron-app/src/main/config/app-config.ts(1 hunks)apps/electron-app/src/main/ipc/index.ts(3 hunks)apps/electron-app/src/main/services/encryption-service.ts(1 hunks)apps/electron-app/src/main/services/update/activity-detector.ts(1 hunks)apps/electron-app/src/main/services/update/index.ts(1 hunks)apps/electron-app/src/main/services/update/update-notifier.ts(1 hunks)apps/electron-app/src/main/services/update/update-rollback.ts(1 hunks)apps/electron-app/src/main/services/update/update-scheduler.ts(1 hunks)apps/electron-app/src/main/services/update/update-service.ts(1 hunks)apps/electron-app/src/main/services/user-analytics.ts(1 hunks)apps/electron-app/src/main/store/index.ts(2 hunks)apps/electron-app/src/main/store/profile-actions.ts(1 hunks)apps/electron-app/src/main/store/store.ts(3 hunks)apps/electron-app/src/main/store/types.ts(2 hunks)apps/electron-app/src/main/store/user-profile-store.ts(1 hunks)apps/electron-app/src/main/utils/debounce.ts(1 hunks)apps/electron-app/src/main/utils/performanceMonitor.ts(1 hunks)apps/electron-app/src/types/metadata.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
apps/electron-app/src/main/ipc/index.ts (2)
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
apps/electron-app/src/main/utils/performanceMonitor.ts (1)
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
apps/electron-app/src/main/store/index.ts (3)
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:254-261
Timestamp: 2025-07-01T15:07:58.879Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team prefers to maintain key-level encryption even when it results in double encryption scenarios like in the UserDataRecover function. They want to keep the consistent encryption handling approach rather than optimizing for performance by storing some data unencrypted.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:277-294
Timestamp: 2025-07-01T15:07:39.741Z
Learning: In apps/electron-app/src/main/store/desktop-store.ts, the user prefers to maintain key-level encryption in the helper functions getRecoveredData and getAllRecoveredData rather than removing the double encryption approach.
apps/electron-app/src/main/services/update/index.ts (1)
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
apps/electron-app/src/main/services/update/update-service.ts (1)
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
apps/electron-app/src/main/services/user-analytics.ts (2)
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
apps/electron-app/src/main/services/update/update-rollback.ts (1)
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
apps/electron-app/src/main/store/store.ts (3)
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:254-261
Timestamp: 2025-07-01T15:07:58.879Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team prefers to maintain key-level encryption even when it results in double encryption scenarios like in the UserDataRecover function. They want to keep the consistent encryption handling approach rather than optimizing for performance by storing some data unencrypted.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:277-294
Timestamp: 2025-07-01T15:07:39.741Z
Learning: In apps/electron-app/src/main/store/desktop-store.ts, the user prefers to maintain key-level encryption in the helper functions getRecoveredData and getAllRecoveredData rather than removing the double encryption approach.
apps/electron-app/src/main/services/encryption-service.ts (4)
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:277-294
Timestamp: 2025-07-01T15:07:39.741Z
Learning: In apps/electron-app/src/main/store/desktop-store.ts, the user prefers to maintain key-level encryption in the helper functions getRecoveredData and getAllRecoveredData rather than removing the double encryption approach.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:254-261
Timestamp: 2025-07-01T15:07:58.879Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team prefers to maintain key-level encryption even when it results in double encryption scenarios like in the UserDataRecover function. They want to keep the consistent encryption handling approach rather than optimizing for performance by storing some data unencrypted.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
apps/electron-app/src/main/store/types.ts (2)
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:254-261
Timestamp: 2025-07-01T15:07:58.879Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team prefers to maintain key-level encryption even when it results in double encryption scenarios like in the UserDataRecover function. They want to keep the consistent encryption handling approach rather than optimizing for performance by storing some data unencrypted.
apps/electron-app/src/main/config/app-config.ts (3)
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:277-294
Timestamp: 2025-07-01T15:07:39.741Z
Learning: In apps/electron-app/src/main/store/desktop-store.ts, the user prefers to maintain key-level encryption in the helper functions getRecoveredData and getAllRecoveredData rather than removing the double encryption approach.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:254-261
Timestamp: 2025-07-01T15:07:58.879Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team prefers to maintain key-level encryption even when it results in double encryption scenarios like in the UserDataRecover function. They want to keep the consistent encryption handling approach rather than optimizing for performance by storing some data unencrypted.
apps/electron-app/src/main/store/user-profile-store.ts (4)
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:254-261
Timestamp: 2025-07-01T15:07:58.879Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team prefers to maintain key-level encryption even when it results in double encryption scenarios like in the UserDataRecover function. They want to keep the consistent encryption handling approach rather than optimizing for performance by storing some data unencrypted.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:277-294
Timestamp: 2025-07-01T15:07:39.741Z
Learning: In apps/electron-app/src/main/store/desktop-store.ts, the user prefers to maintain key-level encryption in the helper functions getRecoveredData and getAllRecoveredData rather than removing the double encryption approach.
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
apps/electron-app/src/main/services/update/update-scheduler.ts (1)
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
apps/electron-app/src/main/store/profile-actions.ts (4)
Learnt from: maceip
PR: co-browser/vibe#38
File: apps/electron-app/src/main/services/profile-service.ts:131-135
Timestamp: 2025-07-01T18:24:21.101Z
Learning: In the ProfileService implementation (apps/electron-app/src/main/services/profile-service.ts), the team prefers to keep the updateProfile method simple without filtering out immutable fields like `id` and `createdAt` from updates, since these fields are not currently being used elsewhere in the codebase. They prioritize code simplicity over theoretical protection against field overwriting.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:0-0
Timestamp: 2025-07-01T18:22:27.574Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team takes a performance-data-driven approach to optimization decisions. They prefer to keep the current implementation (including double encryption scenarios) until they can properly evaluate performance impact, rather than making premature optimizations.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:254-261
Timestamp: 2025-07-01T15:07:58.879Z
Learning: In the desktop store implementation (apps/electron-app/src/main/store/desktop-store.ts), the team prefers to maintain key-level encryption even when it results in double encryption scenarios like in the UserDataRecover function. They want to keep the consistent encryption handling approach rather than optimizing for performance by storing some data unencrypted.
Learnt from: maceip
PR: co-browser/vibe#37
File: apps/electron-app/src/main/store/desktop-store.ts:277-294
Timestamp: 2025-07-01T15:07:39.741Z
Learning: In apps/electron-app/src/main/store/desktop-store.ts, the user prefers to maintain key-level encryption in the helper functions getRecoveredData and getAllRecoveredData rather than removing the double encryption approach.
🧬 Code Graph Analysis (1)
apps/electron-app/src/main/services/encryption-service.ts (1)
packages/shared-types/src/logger/index.ts (1)
createLogger(65-76)
🪛 GitHub Actions: CI
apps/electron-app/src/main/ipc/index.ts
[error] 41-41: TypeScript error TS2307: Cannot find module '@/ipc/browser/download' or its corresponding type declarations.
🪛 GitHub Actions: pull_request
apps/electron-app/src/main/ipc/index.ts
[error] 41-41: TypeScript error TS2307: Cannot find module '@/ipc/browser/download' or its corresponding type declarations.
🪛 LanguageTool
.claude/commands/learn.md
[style] ~15-~15: This phrase is redundant. Consider writing “follow”.
Context: ...the user's research query. ## Steps to follow after receiving the research query: 1. **Rea...
(FOLLOW_AFTER)
🪛 markdownlint-cli2 (0.17.2)
.claude/commands/learn.md
17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
129-129: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
139-139: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
153-153: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.claude/commands/plan.md
17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
129-129: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
139-139: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
153-153: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
177-177: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
243-243: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
245-245: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
253-253: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
270-270: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
275-275: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
279-279: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
297-297: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
397-397: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
405-405: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
413-413: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
458-458: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.claude/commands/pr.md
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
63-63: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
.claude/commands/commit.md
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
29-29: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
37-37: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Biome (1.9.4)
apps/electron-app/src/main/utils/debounce.ts
[error] 8-168: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 22-22: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 25-25: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 30-30: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 38-38: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 39-39: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 43-43: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 55-55: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 62-62: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 71-71: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 72-72: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 84-84: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 87-87: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 88-88: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 98-98: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 105-105: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 112-112: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 114-114: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 133-133: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 136-136: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 148-148: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 150-150: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 151-151: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 152-152: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 164-164: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 165-165: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
apps/electron-app/src/main/services/update/update-notifier.ts
[error] 23-25: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
apps/electron-app/src/types/metadata.ts
[error] 191-271: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
apps/electron-app/src/main/store/user-profile-store.ts
[error] 2124-2124: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🔇 Additional comments (19)
.claude/shared/pr_template.md (1)
12-21: Well-structured PR template sections.The template provides clear guidance for contributors with appropriate sections for describing changes, verification steps, and changelog entries.
.claude/settings.local.json (1)
3-50: Comprehensive permission configuration for development workflow.The permission policy appropriately covers the necessary development tools and operations while maintaining security with domain-restricted web fetches.
apps/electron-app/src/main/ipc/index.ts (1)
68-88: Well-structured IPC handler registration and initialization.The new handler registrations for profile history, password management, autofill, and top sites are properly organized. The downloads service initialization and test logging provide good debugging information.
apps/electron-app/src/main/services/update/index.ts (1)
1-15: Clean and well-organized service index.The index file provides a clean API surface for the update services with proper separation of class exports and type exports.
apps/electron-app/src/main/store/types.ts (2)
11-20: Well-designed DownloadItem interface.The interface comprehensively covers download tracking needs with appropriate state management and progress tracking properties.
31-31: Clean integration of downloads into AppState.The addition of the downloads array to AppState enables proper IPC synchronization for download management.
.claude/commands/commit.md (1)
1-43: Excellent commit documentation structure!This documentation provides a comprehensive and well-structured approach to commit management with clear guidelines for atomic commits, proper messaging, and user attribution. The 4-step process is logical and the important notes about avoiding co-author attribution are crucial for proper git history.
.github/workflows/slack-notifications.yml (1)
1-87: Well-structured reusable workflow for Slack notifications.The workflow correctly implements the reusable workflow pattern with proper input validation and secret handling. The conditional logic for success/failure messages is appropriate.
apps/electron-app/src/main/store/index.ts (2)
18-22: Well-designed initialization status interface.The
StoreInitializationStatusinterface provides comprehensive state tracking with clear boolean flags and error handling, which is essential for reliable asynchronous store initialization.
39-45: Proper optional lifecycle methods in Store interface.The addition of optional initialization and cleanup methods to the Store interface follows good TypeScript practices and provides a clean API for store lifecycle management. The optional nature allows for backward compatibility.
.github/workflows/pr.yml (1)
78-78: Robust status calculation for Slack notifications.The boolean logic for combining job results into a single status is well-implemented, ensuring notifications accurately reflect the overall workflow state.
.github/workflows/push.yml (1)
1-85: Consistent and well-structured CI workflow for main branch.The workflow maintains consistency with the PR workflow while appropriately targeting the main branch. The job dependencies and Slack notification integration are properly configured.
apps/electron-app/src/main/store/store.ts (1)
64-91: LGTM! Well-implemented async initialization pattern.The initialization logic properly handles:
- Preventing multiple concurrent initializations
- Proper error handling and cleanup
- Promise reuse for concurrent callers
- State management with proper cleanup
The implementation follows good async patterns and aligns with the team's preference for keeping implementations straightforward until performance data suggests optimization is needed.
.claude/commands/pr.md (1)
38-43: LGTM! Comprehensive verification handling.The verification workflow is well-designed:
- Attempts to run automated verification commands
- Properly handles both passing and failing tests
- Clearly separates manual verification steps
- Documents incomplete verification for user follow-up
This approach ensures thorough testing while maintaining transparency about what has been verified.
apps/electron-app/src/main/services/update/update-scheduler.ts (1)
133-148: LGTM! Well-implemented cleanup with proper age-based filtering.The cleanup method properly:
- Uses appropriate time threshold (30 days)
- Only removes completed updates
- Persists changes after cleanup
- Provides appropriate logging
The implementation aligns with the team's preference for straightforward, maintainable code.
apps/electron-app/src/main/services/update/update-rollback.ts (1)
212-229: Implement actual Linux rollback logic.The Linux rollback implementation is also a placeholder.
apps/electron-app/src/main/store/profile-actions.ts (1)
1-276: Well-designed profile management APIThis module provides an excellent abstraction layer over the profile store with:
- Clear, intuitive function names
- Comprehensive JSDoc documentation
- Helpful usage examples
- Good separation of concerns by category
The API design makes it easy for other parts of the application to interact with user profiles without dealing with internal complexity.
apps/electron-app/src/types/metadata.ts (1)
6-174: Well-structured metadata interfaces with comprehensive type definitions.The interfaces are properly documented, use appropriate types, and follow a consistent pattern. Good use of union types for constrained values and proper extension of the BaseMetadata interface.
apps/electron-app/src/main/store/user-profile-store.ts (1)
1819-1907: Excellent initialization pattern with proper race condition handling.The initialization implementation properly handles:
- Race conditions with initialization promises
- App readiness checks
- Error recovery with fallback to default profile
- Proper cleanup on shutdown
This aligns well with the team's preference for data-driven approaches as mentioned in the learnings.
| pull request for inclusion in the changelog: | ||
| --> | ||
|
|
||
| --> |
There was a problem hiding this comment.
Fix the malformed HTML comment.
There's a closing comment tag without a corresponding opening tag.
--->Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .claude/shared/pr_template.md at line 27, there is a closing HTML comment tag
without a matching opening tag, causing malformed HTML. Remove the standalone
closing comment tag or add the appropriate opening comment tag before it to
properly balance the comment syntax.
| Please make sure you've read and understood our contributing guidelines; | ||
| https://github.com/humanlayer/humanlayer/blob/master/CONTRIBUTING.md |
There was a problem hiding this comment.
Fix the repository reference URL.
The template references humanlayer/humanlayer but this appears to be for the co-browser/vibe repository based on the PR objectives.
-https://github.com/humanlayer/humanlayer/blob/master/CONTRIBUTING.md
+https://github.com/co-browser/vibe/blob/master/CONTRIBUTING.md📝 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.
| Please make sure you've read and understood our contributing guidelines; | |
| https://github.com/humanlayer/humanlayer/blob/master/CONTRIBUTING.md | |
| Please make sure you've read and understood our contributing guidelines; | |
| https://github.com/co-browser/vibe/blob/master/CONTRIBUTING.md |
🤖 Prompt for AI Agents
In .claude/shared/pr_template.md around lines 2 to 3, the contributing
guidelines URL references the wrong repository (humanlayer/humanlayer). Update
the URL to point to the correct repository for this project, which is
co-browser/vibe, ensuring the link directs to the appropriate CONTRIBUTING.md
file for this repository.
| // MCP APIs - direct imports (register themselves) | ||
| import "@/ipc/mcp/mcp-status"; | ||
|
|
There was a problem hiding this comment.
Remove duplicate MCP API imports.
The MCP APIs are imported twice - once at lines 44-46 and again at lines 56-57.
-// MCP APIs - direct imports (register themselves)
-import "@/ipc/mcp/mcp-status";
-Also applies to: 56-57
🤖 Prompt for AI Agents
In apps/electron-app/src/main/ipc/index.ts at lines 44-46 and 56-57, the MCP API
imports are duplicated. Remove one of the duplicate import statements to ensure
each MCP API is imported only once, keeping the import that best fits the module
organization and removing the redundant one.
| import "@/ipc/browser/windows"; | ||
| import "@/ipc/browser/navigation"; | ||
| import "@/ipc/browser/content"; | ||
| import { downloads } from "@/ipc/browser/download"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix missing module causing pipeline failures.
The import @/ipc/browser/download is causing TypeScript errors as reported in pipeline failures.
🏁 Script executed:
#!/bin/bash
# Check if the download module exists
fd "download" apps/electron-app/src/main/ipc/browser/Length of output: 53
🏁 Script executed:
#!/bin/bash
# List all files in the `ipc/browser` directory to verify available modules
ls apps/electron-app/src/main/ipc/browser/
# Search the entire repo for any `.ts` file matching "download" or "downloads"
fd -t f -e ts download .
fd -t f -e ts downloads .Length of output: 153
🏁 Script executed:
#!/bin/bash
# List all import statements in the IPC index file with line numbers
rg -n "^import" --color=never apps/electron-app/src/main/ipc/index.ts
# Show lines 30–90 for context around duplicate imports and handler registrations
sed -n '30,90p' apps/electron-app/src/main/ipc/index.tsLength of output: 3643
Fix missing download module and remove duplicate MCP imports
The import @/ipc/browser/download does not exist and is causing build failures. Additionally, @/ipc/mcp/mcp-status is imported twice.
• Create or correct apps/electron-app/src/main/ipc/browser/download.ts (or update the import path) so that
import { downloads } from "@/ipc/browser/download"; resolves successfully.
• Remove the duplicate MCP-status import (second occurrence at line 57) in
apps/electron-app/src/main/ipc/index.ts.
🧰 Tools
🪛 GitHub Actions: CI
[error] 41-41: TypeScript error TS2307: Cannot find module '@/ipc/browser/download' or its corresponding type declarations.
🪛 GitHub Actions: pull_request
[error] 41-41: TypeScript error TS2307: Cannot find module '@/ipc/browser/download' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/electron-app/src/main/ipc/index.ts at line 41, the import for
"@/ipc/browser/download" is missing or incorrect, causing build failures. Ensure
that the file apps/electron-app/src/main/ipc/browser/download.ts exists and
exports "downloads" properly, or update the import path to the correct location.
Additionally, remove the duplicate import of "@/ipc/mcp/mcp-status" found at
line 57 to avoid redundancy.
| response=$(curl -s -X POST -H 'Content-type: application/json; charset=utf-8' --data "$payload" https://slack.com/api/chat.postMessage -H "Authorization: Bearer ${{ secrets.SLACK_BOT_TOKEN }}" ) | ||
| echo "Slack API response: $response" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and response validation for Slack API calls.
The current implementation doesn't validate the Slack API response or handle potential failures, which could lead to silent notification failures.
- response=$(curl -s -X POST -H 'Content-type: application/json; charset=utf-8' --data "$payload" https://slack.com/api/chat.postMessage -H "Authorization: Bearer ${{ secrets.SLACK_BOT_TOKEN }}" )
- echo "Slack API response: $response"
+ response=$(curl -s -X POST -H 'Content-type: application/json; charset=utf-8' --data "$payload" https://slack.com/api/chat.postMessage -H "Authorization: Bearer ${{ secrets.SLACK_BOT_TOKEN }}" )
+ echo "Slack API response: $response"
+
+ # Validate Slack API response
+ if ! echo "$response" | jq -e '.ok == true' > /dev/null; then
+ echo "Slack API call failed"
+ echo "$response" | jq -r '.error // "Unknown error"'
+ exit 1
+ fi
+
+ echo "Slack notification sent successfully"📝 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.
| response=$(curl -s -X POST -H 'Content-type: application/json; charset=utf-8' --data "$payload" https://slack.com/api/chat.postMessage -H "Authorization: Bearer ${{ secrets.SLACK_BOT_TOKEN }}" ) | |
| echo "Slack API response: $response" | |
| response=$(curl -s -X POST \ | |
| -H 'Content-type: application/json; charset=utf-8' \ | |
| --data "$payload" \ | |
| https://slack.com/api/chat.postMessage \ | |
| -H "Authorization: Bearer ${{ secrets.SLACK_BOT_TOKEN }}" ) | |
| echo "Slack API response: $response" | |
| # Validate Slack API response | |
| if ! echo "$response" | jq -e '.ok == true' > /dev/null; then | |
| echo "Slack API call failed" | |
| echo "$response" | jq -r '.error // "Unknown error"' | |
| exit 1 | |
| fi | |
| echo "Slack notification sent successfully" |
🤖 Prompt for AI Agents
In .github/workflows/slack-notifications.yml at lines 84-85, the Slack API call
response is not validated or checked for errors. Modify the script to parse the
JSON response from the Slack API, verify if the message was sent successfully by
checking the "ok" field, and handle any errors by logging them or failing the
workflow step accordingly to prevent silent failures.
| private generateSecureFallbackKey(): string { | ||
| try { | ||
| // Use environment variable if available, otherwise derive from machine-specific data | ||
| const envKey = process.env.VIBE_ENCRYPTION_KEY; | ||
| if (envKey && envKey.length >= 32) { | ||
| return envKey.substring(0, 32); | ||
| } | ||
|
|
||
| // Derive key from machine-specific data | ||
| const machineId = | ||
| process.env.HOSTNAME || process.env.COMPUTERNAME || "vibe-default"; | ||
| const appVersion = process.env.npm_package_version || "1.0.0"; | ||
| const platform = process.platform; | ||
|
|
||
| // Combine machine-specific data | ||
| const keyMaterial = `${machineId}-${platform}-${appVersion}-vibe-encryption`; | ||
|
|
||
| // Use PBKDF2 to derive a secure key | ||
| const salt = Buffer.from("vibe-salt-2024", "utf8"); | ||
| const derivedKey = pbkdf2Sync(keyMaterial, salt, 100000, 32, "sha256"); | ||
|
|
||
| return derivedKey.toString("hex").substring(0, 32); | ||
| } catch { | ||
| logger.warn( | ||
| "Failed to generate secure fallback key, using minimal fallback", | ||
| ); | ||
| // Last resort fallback - still better than hardcoded | ||
| const timestamp = Date.now().toString(); | ||
| return pbkdf2Sync( | ||
| `vibe-${timestamp}`, | ||
| "fallback-salt", | ||
| 10000, | ||
| 32, | ||
| "sha256", | ||
| ) | ||
| .toString("hex") | ||
| .substring(0, 32); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Incorrect key truncation reduces encryption strength
The substring(0, 32) operations on hex-encoded keys only extract 16 bytes worth of key material instead of the intended 32 bytes. A 32-byte key produces 64 hexadecimal characters when encoded.
This reduces the effective key size from 256 bits to 128 bits, significantly weakening the encryption.
Apply this fix to properly handle 32-byte keys:
private generateSecureFallbackKey(): string {
try {
// Use environment variable if available, otherwise derive from machine-specific data
const envKey = process.env.VIBE_ENCRYPTION_KEY;
if (envKey && envKey.length >= 32) {
- return envKey.substring(0, 32);
+ // If the key is hex-encoded, ensure we get 32 bytes (64 hex chars)
+ // If it's raw, take first 32 characters
+ return envKey.length >= 64 && /^[0-9a-fA-F]+$/.test(envKey)
+ ? envKey.substring(0, 64)
+ : envKey.substring(0, 32);
}
// ... existing code ...
// Use PBKDF2 to derive a secure key
const salt = Buffer.from("vibe-salt-2024", "utf8");
const derivedKey = pbkdf2Sync(keyMaterial, salt, 100000, 32, "sha256");
- return derivedKey.toString("hex").substring(0, 32);
+ // Return the full hex-encoded 32-byte key (64 hex characters)
+ return derivedKey.toString("hex");
} catch {
// ... existing code ...
return pbkdf2Sync(
`vibe-${timestamp}`,
"fallback-salt",
10000,
32,
"sha256",
)
- .toString("hex")
- .substring(0, 32);
+ .toString("hex");
}
}📝 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.
| private generateSecureFallbackKey(): string { | |
| try { | |
| // Use environment variable if available, otherwise derive from machine-specific data | |
| const envKey = process.env.VIBE_ENCRYPTION_KEY; | |
| if (envKey && envKey.length >= 32) { | |
| return envKey.substring(0, 32); | |
| } | |
| // Derive key from machine-specific data | |
| const machineId = | |
| process.env.HOSTNAME || process.env.COMPUTERNAME || "vibe-default"; | |
| const appVersion = process.env.npm_package_version || "1.0.0"; | |
| const platform = process.platform; | |
| // Combine machine-specific data | |
| const keyMaterial = `${machineId}-${platform}-${appVersion}-vibe-encryption`; | |
| // Use PBKDF2 to derive a secure key | |
| const salt = Buffer.from("vibe-salt-2024", "utf8"); | |
| const derivedKey = pbkdf2Sync(keyMaterial, salt, 100000, 32, "sha256"); | |
| return derivedKey.toString("hex").substring(0, 32); | |
| } catch { | |
| logger.warn( | |
| "Failed to generate secure fallback key, using minimal fallback", | |
| ); | |
| // Last resort fallback - still better than hardcoded | |
| const timestamp = Date.now().toString(); | |
| return pbkdf2Sync( | |
| `vibe-${timestamp}`, | |
| "fallback-salt", | |
| 10000, | |
| 32, | |
| "sha256", | |
| ) | |
| .toString("hex") | |
| .substring(0, 32); | |
| } | |
| } | |
| private generateSecureFallbackKey(): string { | |
| try { | |
| // Use environment variable if available, otherwise derive from machine-specific data | |
| const envKey = process.env.VIBE_ENCRYPTION_KEY; | |
| if (envKey && envKey.length >= 32) { | |
| // If the key is hex-encoded, ensure we get 32 bytes (64 hex chars) | |
| // If it's raw, take the first 32 characters | |
| return envKey.length >= 64 && /^[0-9a-fA-F]+$/.test(envKey) | |
| ? envKey.substring(0, 64) | |
| : envKey.substring(0, 32); | |
| } | |
| // Derive key from machine-specific data | |
| const machineId = | |
| process.env.HOSTNAME || process.env.COMPUTERNAME || "vibe-default"; | |
| const appVersion = process.env.npm_package_version || "1.0.0"; | |
| const platform = process.platform; | |
| // Combine machine-specific data | |
| const keyMaterial = `${machineId}-${platform}-${appVersion}-vibe-encryption`; | |
| // Use PBKDF2 to derive a secure key | |
| const salt = Buffer.from("vibe-salt-2024", "utf8"); | |
| const derivedKey = pbkdf2Sync(keyMaterial, salt, 100000, 32, "sha256"); | |
| // Return the full hex-encoded 32-byte key (64 hex characters) | |
| return derivedKey.toString("hex"); | |
| } catch { | |
| logger.warn( | |
| "Failed to generate secure fallback key, using minimal fallback", | |
| ); | |
| // Last resort fallback – still better than hardcoded | |
| const timestamp = Date.now().toString(); | |
| return pbkdf2Sync( | |
| `vibe-${timestamp}`, | |
| "fallback-salt", | |
| 10000, | |
| "sha256", | |
| ).toString("hex"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/electron-app/src/main/services/encryption-service.ts between lines 28
and 66, the code incorrectly truncates the hex-encoded derived key to 32
characters, which only represents 16 bytes instead of the intended 32 bytes. To
fix this, remove the substring(0, 32) calls on the hex strings and instead
return the full 64-character hex string representing the full 32-byte key. This
ensures the encryption key maintains its full 256-bit strength.
| private encryptWithFallback(data: string): string { | ||
| try { | ||
| const algorithm = "aes-256-cbc"; | ||
| const key = Buffer.from(this.fallbackKey.padEnd(32, "0").slice(0, 32)); | ||
| const iv = randomBytes(16); | ||
|
|
||
| const cipher = createCipheriv(algorithm, key, iv); | ||
| let encrypted = cipher.update(data, "utf8", "hex"); | ||
| encrypted += cipher.final("hex"); | ||
|
|
||
| // Prepend IV to encrypted data | ||
| return iv.toString("hex") + ":" + encrypted; | ||
| } catch (error) { | ||
| logger.error("Fallback encryption failed:", error); | ||
| throw new Error("Failed to encrypt data"); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Fallback decryption using Node.js crypto | ||
| */ | ||
| private decryptWithFallback(encryptedData: string): string { | ||
| try { | ||
| const algorithm = "aes-256-cbc"; | ||
| const key = Buffer.from(this.fallbackKey.padEnd(32, "0").slice(0, 32)); | ||
|
|
||
| // Split IV and encrypted data | ||
| const parts = encryptedData.split(":"); | ||
| if (parts.length !== 2) { | ||
| throw new Error("Invalid encrypted data format"); | ||
| } | ||
|
|
||
| const iv = Buffer.from(parts[0], "hex"); | ||
| const encrypted = parts[1]; | ||
|
|
||
| const decipher = createDecipheriv(algorithm, key, iv); | ||
| let decrypted = decipher.update(encrypted, "hex", "utf8"); | ||
| decrypted += decipher.final("utf8"); | ||
| return decrypted; | ||
| } catch (error) { | ||
| logger.error("Fallback decryption failed:", error); | ||
| throw new Error("Failed to decrypt data"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix key handling in fallback encryption methods
The current implementation incorrectly handles hex-encoded keys from generateSecureFallbackKey(). The Buffer.from() call treats the hex string as UTF-8 text instead of decoding it from hex.
Apply this fix to properly handle hex-encoded keys:
private encryptWithFallback(data: string): string {
try {
const algorithm = "aes-256-cbc";
- const key = Buffer.from(this.fallbackKey.padEnd(32, "0").slice(0, 32));
+ // Decode hex key to binary (32 bytes)
+ const key = Buffer.from(this.fallbackKey, "hex");
const iv = randomBytes(16);
// ... rest of the method
}
}
private decryptWithFallback(encryptedData: string): string {
try {
const algorithm = "aes-256-cbc";
- const key = Buffer.from(this.fallbackKey.padEnd(32, "0").slice(0, 32));
+ // Decode hex key to binary (32 bytes)
+ const key = Buffer.from(this.fallbackKey, "hex");
// ... rest of the method
}
}📝 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.
| private encryptWithFallback(data: string): string { | |
| try { | |
| const algorithm = "aes-256-cbc"; | |
| const key = Buffer.from(this.fallbackKey.padEnd(32, "0").slice(0, 32)); | |
| const iv = randomBytes(16); | |
| const cipher = createCipheriv(algorithm, key, iv); | |
| let encrypted = cipher.update(data, "utf8", "hex"); | |
| encrypted += cipher.final("hex"); | |
| // Prepend IV to encrypted data | |
| return iv.toString("hex") + ":" + encrypted; | |
| } catch (error) { | |
| logger.error("Fallback encryption failed:", error); | |
| throw new Error("Failed to encrypt data"); | |
| } | |
| } | |
| /** | |
| * Fallback decryption using Node.js crypto | |
| */ | |
| private decryptWithFallback(encryptedData: string): string { | |
| try { | |
| const algorithm = "aes-256-cbc"; | |
| const key = Buffer.from(this.fallbackKey.padEnd(32, "0").slice(0, 32)); | |
| // Split IV and encrypted data | |
| const parts = encryptedData.split(":"); | |
| if (parts.length !== 2) { | |
| throw new Error("Invalid encrypted data format"); | |
| } | |
| const iv = Buffer.from(parts[0], "hex"); | |
| const encrypted = parts[1]; | |
| const decipher = createDecipheriv(algorithm, key, iv); | |
| let decrypted = decipher.update(encrypted, "hex", "utf8"); | |
| decrypted += decipher.final("utf8"); | |
| return decrypted; | |
| } catch (error) { | |
| logger.error("Fallback decryption failed:", error); | |
| throw new Error("Failed to decrypt data"); | |
| } | |
| } | |
| // File: apps/electron-app/src/main/services/encryption-service.ts | |
| private encryptWithFallback(data: string): string { | |
| try { | |
| const algorithm = "aes-256-cbc"; | |
| // Decode hex key to binary (32 bytes) | |
| const key = Buffer.from(this.fallbackKey, "hex"); | |
| const iv = randomBytes(16); | |
| const cipher = createCipheriv(algorithm, key, iv); | |
| let encrypted = cipher.update(data, "utf8", "hex"); | |
| encrypted += cipher.final("hex"); | |
| // Prepend IV to encrypted data | |
| return iv.toString("hex") + ":" + encrypted; | |
| } catch (error) { | |
| logger.error("Fallback encryption failed:", error); | |
| throw new Error("Failed to encrypt data"); | |
| } | |
| } | |
| /** | |
| * Fallback decryption using Node.js crypto | |
| */ | |
| private decryptWithFallback(encryptedData: string): string { | |
| try { | |
| const algorithm = "aes-256-cbc"; | |
| // Decode hex key to binary (32 bytes) | |
| const key = Buffer.from(this.fallbackKey, "hex"); | |
| // Split IV and encrypted data | |
| const parts = encryptedData.split(":"); | |
| if (parts.length !== 2) { | |
| throw new Error("Invalid encrypted data format"); | |
| } | |
| const iv = Buffer.from(parts[0], "hex"); | |
| const encrypted = parts[1]; | |
| const decipher = createDecipheriv(algorithm, key, iv); | |
| let decrypted = decipher.update(encrypted, "hex", "utf8"); | |
| decrypted += decipher.final("utf8"); | |
| return decrypted; | |
| } catch (error) { | |
| logger.error("Fallback decryption failed:", error); | |
| throw new Error("Failed to decrypt data"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/electron-app/src/main/services/encryption-service.ts between lines 150
and 193, the fallback encryption and decryption methods incorrectly handle the
fallbackKey by treating it as UTF-8 text instead of decoding it as a hex string.
To fix this, change the Buffer.from call to decode the fallbackKey from hex
encoding rather than UTF-8 by specifying 'hex' as the second argument. This
ensures the key is correctly interpreted as a hex-encoded value for both
encryptWithFallback and decryptWithFallback methods.
| export class MetadataHelpers { | ||
| /** | ||
| * Type guard to check if metadata is navigation history metadata | ||
| */ | ||
| static isNavigationHistoryMetadata( | ||
| metadata: unknown, | ||
| ): metadata is NavigationHistoryMetadata { | ||
| return ( | ||
| typeof metadata === "object" && | ||
| metadata !== null && | ||
| "url" in metadata && | ||
| "visitCount" in metadata && | ||
| "lastVisit" in metadata | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Type guard to check if metadata is agent action metadata | ||
| */ | ||
| static isAgentActionMetadata( | ||
| metadata: unknown, | ||
| ): metadata is AgentActionMetadata { | ||
| return ( | ||
| typeof metadata === "object" && | ||
| metadata !== null && | ||
| "action" in metadata && | ||
| typeof (metadata as any).action === "string" | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Type guard to check if metadata is search suggestion metadata | ||
| */ | ||
| static isSearchSuggestionMetadata( | ||
| metadata: unknown, | ||
| ): metadata is SearchSuggestionMetadata { | ||
| return ( | ||
| typeof metadata === "object" && | ||
| metadata !== null && | ||
| "source" in metadata && | ||
| "query" in metadata | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Type guard to check if metadata is context suggestion metadata | ||
| */ | ||
| static isContextSuggestionMetadata( | ||
| metadata: unknown, | ||
| ): metadata is ContextSuggestionMetadata { | ||
| return ( | ||
| typeof metadata === "object" && | ||
| metadata !== null && | ||
| "contentType" in metadata && | ||
| typeof (metadata as any).contentType === "string" | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Creates base metadata with timestamp | ||
| */ | ||
| static createBaseMetadata(additional?: Partial<BaseMetadata>): BaseMetadata { | ||
| return { | ||
| timestamp: Date.now(), | ||
| ...additional, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Safely extracts specific metadata type | ||
| */ | ||
| static extractMetadata<T extends SuggestionMetadata>( | ||
| metadata: unknown, | ||
| validator: (data: unknown) => data is T, | ||
| ): T | null { | ||
| if (validator(metadata)) { | ||
| return metadata; | ||
| } | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor static-only class to standalone functions and improve type safety.
The MetadataHelpers class contains only static members, which is an anti-pattern. Additionally, the type guards use unnecessary as any casts that bypass TypeScript's type system.
Convert to standalone functions and fix type safety:
-export class MetadataHelpers {
- /**
- * Type guard to check if metadata is navigation history metadata
- */
- static isNavigationHistoryMetadata(
- metadata: unknown,
- ): metadata is NavigationHistoryMetadata {
- return (
- typeof metadata === "object" &&
- metadata !== null &&
- "url" in metadata &&
- "visitCount" in metadata &&
- "lastVisit" in metadata
- );
- }
-
- /**
- * Type guard to check if metadata is agent action metadata
- */
- static isAgentActionMetadata(
- metadata: unknown,
- ): metadata is AgentActionMetadata {
- return (
- typeof metadata === "object" &&
- metadata !== null &&
- "action" in metadata &&
- typeof (metadata as any).action === "string"
- );
- }
-
- /**
- * Type guard to check if metadata is search suggestion metadata
- */
- static isSearchSuggestionMetadata(
- metadata: unknown,
- ): metadata is SearchSuggestionMetadata {
- return (
- typeof metadata === "object" &&
- metadata !== null &&
- "source" in metadata &&
- "query" in metadata
- );
- }
-
- /**
- * Type guard to check if metadata is context suggestion metadata
- */
- static isContextSuggestionMetadata(
- metadata: unknown,
- ): metadata is ContextSuggestionMetadata {
- return (
- typeof metadata === "object" &&
- metadata !== null &&
- "contentType" in metadata &&
- typeof (metadata as any).contentType === "string"
- );
- }
-
- /**
- * Creates base metadata with timestamp
- */
- static createBaseMetadata(additional?: Partial<BaseMetadata>): BaseMetadata {
- return {
- timestamp: Date.now(),
- ...additional,
- };
- }
-
- /**
- * Safely extracts specific metadata type
- */
- static extractMetadata<T extends SuggestionMetadata>(
- metadata: unknown,
- validator: (data: unknown) => data is T,
- ): T | null {
- if (validator(metadata)) {
- return metadata;
- }
- return null;
- }
-}
+/**
+ * Type guard to check if metadata is navigation history metadata
+ */
+export function isNavigationHistoryMetadata(
+ metadata: unknown,
+): metadata is NavigationHistoryMetadata {
+ return (
+ typeof metadata === "object" &&
+ metadata !== null &&
+ "url" in metadata &&
+ "visitCount" in metadata &&
+ "lastVisit" in metadata
+ );
+}
+
+/**
+ * Type guard to check if metadata is agent action metadata
+ */
+export function isAgentActionMetadata(
+ metadata: unknown,
+): metadata is AgentActionMetadata {
+ if (typeof metadata !== "object" || metadata === null) {
+ return false;
+ }
+
+ const obj = metadata as Record<string, unknown>;
+ return "action" in obj && typeof obj.action === "string";
+}
+
+/**
+ * Type guard to check if metadata is search suggestion metadata
+ */
+export function isSearchSuggestionMetadata(
+ metadata: unknown,
+): metadata is SearchSuggestionMetadata {
+ return (
+ typeof metadata === "object" &&
+ metadata !== null &&
+ "source" in metadata &&
+ "query" in metadata
+ );
+}
+
+/**
+ * Type guard to check if metadata is context suggestion metadata
+ */
+export function isContextSuggestionMetadata(
+ metadata: unknown,
+): metadata is ContextSuggestionMetadata {
+ if (typeof metadata !== "object" || metadata === null) {
+ return false;
+ }
+
+ const obj = metadata as Record<string, unknown>;
+ return "contentType" in obj && typeof obj.contentType === "string";
+}
+
+/**
+ * Creates base metadata with timestamp
+ */
+export function createBaseMetadata(additional?: Partial<BaseMetadata>): BaseMetadata {
+ return {
+ timestamp: Date.now(),
+ ...additional,
+ };
+}
+
+/**
+ * Safely extracts specific metadata type
+ */
+export function extractMetadata<T extends SuggestionMetadata>(
+ metadata: unknown,
+ validator: (data: unknown) => data is T,
+): T | null {
+ if (validator(metadata)) {
+ return metadata;
+ }
+ return null;
+}📝 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 class MetadataHelpers { | |
| /** | |
| * Type guard to check if metadata is navigation history metadata | |
| */ | |
| static isNavigationHistoryMetadata( | |
| metadata: unknown, | |
| ): metadata is NavigationHistoryMetadata { | |
| return ( | |
| typeof metadata === "object" && | |
| metadata !== null && | |
| "url" in metadata && | |
| "visitCount" in metadata && | |
| "lastVisit" in metadata | |
| ); | |
| } | |
| /** | |
| * Type guard to check if metadata is agent action metadata | |
| */ | |
| static isAgentActionMetadata( | |
| metadata: unknown, | |
| ): metadata is AgentActionMetadata { | |
| return ( | |
| typeof metadata === "object" && | |
| metadata !== null && | |
| "action" in metadata && | |
| typeof (metadata as any).action === "string" | |
| ); | |
| } | |
| /** | |
| * Type guard to check if metadata is search suggestion metadata | |
| */ | |
| static isSearchSuggestionMetadata( | |
| metadata: unknown, | |
| ): metadata is SearchSuggestionMetadata { | |
| return ( | |
| typeof metadata === "object" && | |
| metadata !== null && | |
| "source" in metadata && | |
| "query" in metadata | |
| ); | |
| } | |
| /** | |
| * Type guard to check if metadata is context suggestion metadata | |
| */ | |
| static isContextSuggestionMetadata( | |
| metadata: unknown, | |
| ): metadata is ContextSuggestionMetadata { | |
| return ( | |
| typeof metadata === "object" && | |
| metadata !== null && | |
| "contentType" in metadata && | |
| typeof (metadata as any).contentType === "string" | |
| ); | |
| } | |
| /** | |
| * Creates base metadata with timestamp | |
| */ | |
| static createBaseMetadata(additional?: Partial<BaseMetadata>): BaseMetadata { | |
| return { | |
| timestamp: Date.now(), | |
| ...additional, | |
| }; | |
| } | |
| /** | |
| * Safely extracts specific metadata type | |
| */ | |
| static extractMetadata<T extends SuggestionMetadata>( | |
| metadata: unknown, | |
| validator: (data: unknown) => data is T, | |
| ): T | null { | |
| if (validator(metadata)) { | |
| return metadata; | |
| } | |
| return null; | |
| } | |
| } | |
| /** | |
| * Type guard to check if metadata is navigation history metadata | |
| */ | |
| export function isNavigationHistoryMetadata( | |
| metadata: unknown, | |
| ): metadata is NavigationHistoryMetadata { | |
| return ( | |
| typeof metadata === "object" && | |
| metadata !== null && | |
| "url" in metadata && | |
| "visitCount" in metadata && | |
| "lastVisit" in metadata | |
| ); | |
| } | |
| /** | |
| * Type guard to check if metadata is agent action metadata | |
| */ | |
| export function isAgentActionMetadata( | |
| metadata: unknown, | |
| ): metadata is AgentActionMetadata { | |
| if (typeof metadata !== "object" || metadata === null) { | |
| return false; | |
| } | |
| const obj = metadata as Record<string, unknown>; | |
| return "action" in obj && typeof obj.action === "string"; | |
| } | |
| /** | |
| * Type guard to check if metadata is search suggestion metadata | |
| */ | |
| export function isSearchSuggestionMetadata( | |
| metadata: unknown, | |
| ): metadata is SearchSuggestionMetadata { | |
| return ( | |
| typeof metadata === "object" && | |
| metadata !== null && | |
| "source" in metadata && | |
| "query" in metadata | |
| ); | |
| } | |
| /** | |
| * Type guard to check if metadata is context suggestion metadata | |
| */ | |
| export function isContextSuggestionMetadata( | |
| metadata: unknown, | |
| ): metadata is ContextSuggestionMetadata { | |
| if (typeof metadata !== "object" || metadata === null) { | |
| return false; | |
| } | |
| const obj = metadata as Record<string, unknown>; | |
| return "contentType" in obj && typeof obj.contentType === "string"; | |
| } | |
| /** | |
| * Creates base metadata with timestamp | |
| */ | |
| export function createBaseMetadata(additional?: Partial<BaseMetadata>): BaseMetadata { | |
| return { | |
| timestamp: Date.now(), | |
| ...additional, | |
| }; | |
| } | |
| /** | |
| * Safely extracts specific metadata type | |
| */ | |
| export function extractMetadata<T extends SuggestionMetadata>( | |
| metadata: unknown, | |
| validator: (data: unknown) => data is T, | |
| ): T | null { | |
| if (validator(metadata)) { | |
| return metadata; | |
| } | |
| return null; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 191-271: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🤖 Prompt for AI Agents
In apps/electron-app/src/types/metadata.ts from lines 191 to 271, the
MetadataHelpers class only contains static methods and uses unsafe 'as any'
casts in type guards. Refactor by converting each static method into a
standalone exported function. Remove all 'as any' casts by properly typing the
input parameter and using safe property access and type checks to ensure type
safety in the type guard functions.
| // Password storage implementation using encrypted secure settings | ||
| storeImportedPasswords: async ( | ||
| profileId: string, | ||
| source: string, | ||
| passwords: ImportedPasswordEntry[], | ||
| ) => { | ||
| try { | ||
| const passwordData: PasswordImportData = { | ||
| passwords, | ||
| timestamp: Date.now(), | ||
| source, | ||
| count: passwords.length, | ||
| }; | ||
|
|
||
| // Store encrypted password data using secure settings | ||
| const key = `passwords.import.${source}`; | ||
| await get().setSecureSetting( | ||
| profileId, | ||
| key, | ||
| JSON.stringify(passwordData), | ||
| ); | ||
|
|
||
| logger.info( | ||
| `Stored ${passwords.length} passwords from ${source} securely for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error(`Failed to store imported passwords from ${source}:`, error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getImportedPasswords: async ( | ||
| profileId: string, | ||
| source?: string, | ||
| ): Promise<ImportedPasswordEntry[]> => { | ||
| try { | ||
| if (source) { | ||
| // Get passwords from specific source | ||
| const key = `passwords.import.${source}`; | ||
| const encryptedData = await get().getSecureSetting(profileId, key); | ||
|
|
||
| if (!encryptedData) { | ||
| return []; | ||
| } | ||
|
|
||
| const passwordData: PasswordImportData = JSON.parse(encryptedData); | ||
| return passwordData.passwords || []; | ||
| } else { | ||
| // Get passwords from all sources | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const allPasswords: ImportedPasswordEntry[] = []; | ||
|
|
||
| for (const [key, value] of Object.entries(allSecureSettings)) { | ||
| if (key.startsWith("passwords.import.")) { | ||
| try { | ||
| const passwordData: PasswordImportData = JSON.parse(value); | ||
| allPasswords.push(...(passwordData.passwords || [])); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to parse password data for key ${key}:`, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return allPasswords; | ||
| } | ||
| } catch (error) { | ||
| logger.error(`Failed to get imported passwords:`, error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| removeImportedPasswords: async ( | ||
| profileId: string, | ||
| source: string, | ||
| ): Promise<void> => { | ||
| try { | ||
| const key = `passwords.import.${source}`; | ||
| await get().removeSecureSetting(profileId, key); | ||
| logger.info( | ||
| `Removed imported passwords from ${source} for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to remove imported passwords from ${source}:`, | ||
| error, | ||
| ); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| clearAllImportedPasswords: async (profileId: string): Promise<void> => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const passwordKeys = Object.keys(allSecureSettings).filter(key => | ||
| key.startsWith("passwords.import."), | ||
| ); | ||
|
|
||
| for (const key of passwordKeys) { | ||
| const source = key.replace("passwords.import.", ""); | ||
| await get().removeImportedPasswords(profileId, source); | ||
| } | ||
|
|
||
| logger.info(`Cleared all imported passwords for profile ${profileId}`); | ||
| } catch (error) { | ||
| logger.error("Failed to clear all imported passwords:", error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getPasswordImportSources: async (profileId: string): Promise<string[]> => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const sources = Object.keys(allSecureSettings) | ||
| .filter(key => key.startsWith("passwords.import.")) | ||
| .map(key => key.replace("passwords.import.", "")); | ||
|
|
||
| return sources; | ||
| } catch (error) { | ||
| logger.error("Failed to get password import sources:", error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| // Bookmark storage actions implementation | ||
| storeImportedBookmarks: async ( | ||
| profileId: string, | ||
| source: string, | ||
| bookmarks: BookmarkEntry[], | ||
| ) => { | ||
| try { | ||
| const bookmarkData: BookmarkImportData = { | ||
| bookmarks, | ||
| timestamp: Date.now(), | ||
| source, | ||
| count: bookmarks.length, | ||
| }; | ||
|
|
||
| const key = `bookmarks.import.${source}`; | ||
| await get().setSecureSetting( | ||
| profileId, | ||
| key, | ||
| JSON.stringify(bookmarkData), | ||
| ); | ||
|
|
||
| logger.info( | ||
| `Stored ${bookmarks.length} bookmarks from ${source} securely for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error(`Failed to store imported bookmarks from ${source}:`, error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getImportedBookmarks: async ( | ||
| profileId: string, | ||
| source?: string, | ||
| ): Promise<BookmarkEntry[]> => { | ||
| try { | ||
| if (source) { | ||
| const key = `bookmarks.import.${source}`; | ||
| const encryptedData = await get().getSecureSetting(profileId, key); | ||
| if (!encryptedData) return []; | ||
|
|
||
| const bookmarkData: BookmarkImportData = JSON.parse(encryptedData); | ||
| return bookmarkData.bookmarks || []; | ||
| } else { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const allBookmarks: BookmarkEntry[] = []; | ||
|
|
||
| for (const [key, value] of Object.entries(allSecureSettings)) { | ||
| if (key.startsWith("bookmarks.import.")) { | ||
| try { | ||
| const bookmarkData: BookmarkImportData = JSON.parse(value); | ||
| allBookmarks.push(...(bookmarkData.bookmarks || [])); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to parse bookmark data for key ${key}:`, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return allBookmarks; | ||
| } | ||
| } catch (error) { | ||
| logger.error(`Failed to get imported bookmarks:`, error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| removeImportedBookmarks: async (profileId: string, source: string) => { | ||
| try { | ||
| const key = `bookmarks.import.${source}`; | ||
| await get().removeSecureSetting(profileId, key); | ||
| logger.info( | ||
| `Removed imported bookmarks from ${source} for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to remove imported bookmarks from ${source}:`, | ||
| error, | ||
| ); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| clearAllImportedBookmarks: async (profileId: string) => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const bookmarkKeys = Object.keys(allSecureSettings).filter(key => | ||
| key.startsWith("bookmarks.import."), | ||
| ); | ||
|
|
||
| for (const key of bookmarkKeys) { | ||
| const source = key.replace("bookmarks.import.", ""); | ||
| await get().removeImportedBookmarks(profileId, source); | ||
| } | ||
|
|
||
| logger.info(`Cleared all imported bookmarks for profile ${profileId}`); | ||
| } catch (error) { | ||
| logger.error("Failed to clear all imported bookmarks:", error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getBookmarkImportSources: async (profileId: string): Promise<string[]> => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const sources = Object.keys(allSecureSettings) | ||
| .filter(key => key.startsWith("bookmarks.import.")) | ||
| .map(key => key.replace("bookmarks.import.", "")); | ||
|
|
||
| return sources; | ||
| } catch (error) { | ||
| logger.error("Failed to get bookmark import sources:", error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| // Enhanced history storage actions implementation | ||
| storeImportedHistory: async ( | ||
| profileId: string, | ||
| source: string, | ||
| history: NavigationHistoryEntry[], | ||
| ) => { | ||
| try { | ||
| const historyData = { | ||
| entries: history, | ||
| timestamp: Date.now(), | ||
| source, | ||
| count: history.length, | ||
| }; | ||
|
|
||
| const key = `history.import.${source}`; | ||
| await get().setSecureSetting(profileId, key, JSON.stringify(historyData)); | ||
|
|
||
| logger.info( | ||
| `Stored ${history.length} history entries from ${source} securely for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error(`Failed to store imported history from ${source}:`, error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getImportedHistory: async ( | ||
| profileId: string, | ||
| source?: string, | ||
| ): Promise<NavigationHistoryEntry[]> => { | ||
| try { | ||
| if (source) { | ||
| const key = `history.import.${source}`; | ||
| const encryptedData = await get().getSecureSetting(profileId, key); | ||
| if (!encryptedData) return []; | ||
|
|
||
| const historyData = JSON.parse(encryptedData); | ||
| return historyData.entries || []; | ||
| } else { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const allHistory: NavigationHistoryEntry[] = []; | ||
|
|
||
| for (const [key, value] of Object.entries(allSecureSettings)) { | ||
| if (key.startsWith("history.import.")) { | ||
| try { | ||
| const historyData = JSON.parse(value); | ||
| allHistory.push(...(historyData.entries || [])); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to parse history data for key ${key}:`, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return allHistory; | ||
| } | ||
| } catch (error) { | ||
| logger.error(`Failed to get imported history:`, error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| removeImportedHistory: async (profileId: string, source: string) => { | ||
| try { | ||
| const key = `history.import.${source}`; | ||
| await get().removeSecureSetting(profileId, key); | ||
| logger.info( | ||
| `Removed imported history from ${source} for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error(`Failed to remove imported history from ${source}:`, error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| clearAllImportedHistory: async (profileId: string) => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const historyKeys = Object.keys(allSecureSettings).filter(key => | ||
| key.startsWith("history.import."), | ||
| ); | ||
|
|
||
| for (const key of historyKeys) { | ||
| const source = key.replace("history.import.", ""); | ||
| await get().removeImportedHistory(profileId, source); | ||
| } | ||
|
|
||
| logger.info(`Cleared all imported history for profile ${profileId}`); | ||
| } catch (error) { | ||
| logger.error("Failed to clear all imported history:", error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getHistoryImportSources: async (profileId: string): Promise<string[]> => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const sources = Object.keys(allSecureSettings) | ||
| .filter(key => key.startsWith("history.import.")) | ||
| .map(key => key.replace("history.import.", "")); | ||
|
|
||
| return sources; | ||
| } catch (error) { | ||
| logger.error("Failed to get history import sources:", error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| // Autofill storage actions implementation | ||
| storeImportedAutofill: async ( | ||
| profileId: string, | ||
| source: string, | ||
| autofillData: AutofillImportData, | ||
| ) => { | ||
| try { | ||
| const key = `autofill.import.${source}`; | ||
| await get().setSecureSetting( | ||
| profileId, | ||
| key, | ||
| JSON.stringify(autofillData), | ||
| ); | ||
|
|
||
| logger.info( | ||
| `Stored ${autofillData.count} autofill items from ${source} securely for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error(`Failed to store imported autofill from ${source}:`, error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getImportedAutofill: async ( | ||
| profileId: string, | ||
| source?: string, | ||
| ): Promise<AutofillImportData> => { | ||
| try { | ||
| if (source) { | ||
| const key = `autofill.import.${source}`; | ||
| const encryptedData = await get().getSecureSetting(profileId, key); | ||
| if (!encryptedData) | ||
| return { entries: [], profiles: [], timestamp: 0, source, count: 0 }; | ||
|
|
||
| const autofillData: AutofillImportData = JSON.parse(encryptedData); | ||
| return autofillData; | ||
| } else { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const combinedData: AutofillImportData = { | ||
| entries: [], | ||
| profiles: [], | ||
| timestamp: Date.now(), | ||
| source: "combined", | ||
| count: 0, | ||
| }; | ||
|
|
||
| for (const [key, value] of Object.entries(allSecureSettings)) { | ||
| if (key.startsWith("autofill.import.")) { | ||
| try { | ||
| const autofillData: AutofillImportData = JSON.parse(value); | ||
| combinedData.entries.push(...(autofillData.entries || [])); | ||
| combinedData.profiles.push(...(autofillData.profiles || [])); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to parse autofill data for key ${key}:`, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| combinedData.count = | ||
| combinedData.entries.length + combinedData.profiles.length; | ||
| return combinedData; | ||
| } | ||
| } catch (error) { | ||
| logger.error(`Failed to get imported autofill:`, error); | ||
| return { | ||
| entries: [], | ||
| profiles: [], | ||
| timestamp: 0, | ||
| source: source || "error", | ||
| count: 0, | ||
| }; | ||
| } | ||
| }, | ||
|
|
||
| removeImportedAutofill: async (profileId: string, source: string) => { | ||
| try { | ||
| const key = `autofill.import.${source}`; | ||
| await get().removeSecureSetting(profileId, key); | ||
| logger.info( | ||
| `Removed imported autofill from ${source} for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error(`Failed to remove imported autofill from ${source}:`, error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| clearAllImportedAutofill: async (profileId: string) => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const autofillKeys = Object.keys(allSecureSettings).filter(key => | ||
| key.startsWith("autofill.import."), | ||
| ); | ||
|
|
||
| for (const key of autofillKeys) { | ||
| const source = key.replace("autofill.import.", ""); | ||
| await get().removeImportedAutofill(profileId, source); | ||
| } | ||
|
|
||
| logger.info(`Cleared all imported autofill for profile ${profileId}`); | ||
| } catch (error) { | ||
| logger.error("Failed to clear all imported autofill:", error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getAutofillImportSources: async (profileId: string): Promise<string[]> => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const sources = Object.keys(allSecureSettings) | ||
| .filter(key => key.startsWith("autofill.import.")) | ||
| .map(key => key.replace("autofill.import.", "")); | ||
|
|
||
| return sources; | ||
| } catch (error) { | ||
| logger.error("Failed to get autofill import sources:", error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| // Search engine storage actions implementation | ||
| storeImportedSearchEngines: async ( | ||
| profileId: string, | ||
| source: string, | ||
| engines: SearchEngine[], | ||
| ) => { | ||
| try { | ||
| const searchEngineData: SearchEngineImportData = { | ||
| engines, | ||
| timestamp: Date.now(), | ||
| source, | ||
| count: engines.length, | ||
| }; | ||
|
|
||
| const key = `searchEngines.import.${source}`; | ||
| await get().setSecureSetting( | ||
| profileId, | ||
| key, | ||
| JSON.stringify(searchEngineData), | ||
| ); | ||
|
|
||
| logger.info( | ||
| `Stored ${engines.length} search engines from ${source} securely for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to store imported search engines from ${source}:`, | ||
| error, | ||
| ); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getImportedSearchEngines: async ( | ||
| profileId: string, | ||
| source?: string, | ||
| ): Promise<SearchEngine[]> => { | ||
| try { | ||
| if (source) { | ||
| const key = `searchEngines.import.${source}`; | ||
| const encryptedData = await get().getSecureSetting(profileId, key); | ||
| if (!encryptedData) return []; | ||
|
|
||
| const searchEngineData: SearchEngineImportData = | ||
| JSON.parse(encryptedData); | ||
| return searchEngineData.engines || []; | ||
| } else { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const allSearchEngines: SearchEngine[] = []; | ||
|
|
||
| for (const [key, value] of Object.entries(allSecureSettings)) { | ||
| if (key.startsWith("searchEngines.import.")) { | ||
| try { | ||
| const searchEngineData: SearchEngineImportData = | ||
| JSON.parse(value); | ||
| allSearchEngines.push(...(searchEngineData.engines || [])); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to parse search engine data for key ${key}:`, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return allSearchEngines; | ||
| } | ||
| } catch (error) { | ||
| logger.error(`Failed to get imported search engines:`, error); | ||
| return []; | ||
| } | ||
| }, | ||
|
|
||
| removeImportedSearchEngines: async (profileId: string, source: string) => { | ||
| try { | ||
| const key = `searchEngines.import.${source}`; | ||
| await get().removeSecureSetting(profileId, key); | ||
| logger.info( | ||
| `Removed imported search engines from ${source} for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error( | ||
| `Failed to remove imported search engines from ${source}:`, | ||
| error, | ||
| ); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| clearAllImportedSearchEngines: async (profileId: string) => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const searchEngineKeys = Object.keys(allSecureSettings).filter(key => | ||
| key.startsWith("searchEngines.import."), | ||
| ); | ||
|
|
||
| for (const key of searchEngineKeys) { | ||
| const source = key.replace("searchEngines.import.", ""); | ||
| await get().removeImportedSearchEngines(profileId, source); | ||
| } | ||
|
|
||
| logger.info( | ||
| `Cleared all imported search engines for profile ${profileId}`, | ||
| ); | ||
| } catch (error) { | ||
| logger.error("Failed to clear all imported search engines:", error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| getSearchEngineImportSources: async ( | ||
| profileId: string, | ||
| ): Promise<string[]> => { | ||
| try { | ||
| const allSecureSettings = await get().getAllSecureSettings(profileId); | ||
| const sources = Object.keys(allSecureSettings) | ||
| .filter(key => key.startsWith("searchEngines.import.")) | ||
| .map(key => key.replace("searchEngines.import.", "")); | ||
|
|
||
| return sources; | ||
| } catch (error) { | ||
| logger.error("Failed to get search engine import sources:", error); | ||
| return []; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication in import/export methods with a generic approach.
The import/export methods for passwords, bookmarks, history, autofill, and search engines follow identical patterns. This duplication makes the code harder to maintain and increases the chance of bugs.
Consider creating a generic import/export manager:
interface ImportExportConfig<T> {
keyPrefix: string;
serialize: (data: T) => string;
deserialize: (data: string) => T;
}
class SecureImportExportManager<T> {
constructor(
private config: ImportExportConfig<T>,
private getSecureSetting: (profileId: string, key: string) => Promise<string | null>,
private setSecureSetting: (profileId: string, key: string, value: string) => Promise<void>,
private removeSecureSetting: (profileId: string, key: string) => Promise<void>,
private getAllSecureSettings: (profileId: string) => Promise<Record<string, string>>
) {}
async store(profileId: string, source: string, data: T): Promise<void> {
const key = `${this.config.keyPrefix}.${source}`;
await this.setSecureSetting(profileId, key, this.config.serialize(data));
}
async get(profileId: string, source?: string): Promise<T | T[]> {
if (source) {
const key = `${this.config.keyPrefix}.${source}`;
const data = await this.getSecureSetting(profileId, key);
return data ? this.config.deserialize(data) : null;
}
// Get all implementation...
}
async remove(profileId: string, source: string): Promise<void> {
const key = `${this.config.keyPrefix}.${source}`;
await this.removeSecureSetting(profileId, key);
}
async getSources(profileId: string): Promise<string[]> {
const settings = await this.getAllSecureSettings(profileId);
return Object.keys(settings)
.filter(key => key.startsWith(this.config.keyPrefix))
.map(key => key.replace(`${this.config.keyPrefix}.`, ""));
}
}This would reduce the store implementation by hundreds of lines while maintaining the same functionality.
🤖 Prompt for AI Agents
In apps/electron-app/src/main/store/user-profile-store.ts around lines 1018 to
1618, the import/export methods for passwords, bookmarks, history, autofill, and
search engines are highly duplicated with nearly identical logic. To fix this,
create a generic SecureImportExportManager class that accepts configuration for
key prefixes and serialization/deserialization functions, and uses the existing
secure setting methods for storing, retrieving, removing, and listing sources.
Then refactor each specific import/export method to use this generic manager,
significantly reducing code duplication and improving maintainability.
| destroySessionForProfile: (profileId: string): void => { | ||
| const state = get(); | ||
| const profileSession = state.profileSessions.get(profileId); | ||
|
|
||
| if (!profileSession) { | ||
| logger.warn(`No session found for profile ${profileId}`); | ||
| return; | ||
| } | ||
|
|
||
| // Clear session data | ||
| profileSession.clearStorageData(); | ||
| profileSession.clearCache(); | ||
|
|
||
| // Remove from maps | ||
| set(state => { | ||
| const newSessions = new Map(state.profileSessions); | ||
| newSessions.delete(profileId); | ||
|
|
||
| const profile = state.profiles.get(profileId); | ||
| if (profile) { | ||
| delete profile.session; | ||
| } | ||
|
|
||
| return { profileSessions: newSessions }; | ||
| }); | ||
|
|
||
| logger.info(`Destroyed session for profile ${profileId}`); | ||
| }, |
There was a problem hiding this comment.
Replace delete operator with undefined assignment for better performance.
The delete operator can impact performance by changing the object's shape and causing V8 deoptimization.
const profile = state.profiles.get(profileId);
if (profile) {
- delete profile.session;
+ profile.session = undefined;
}📝 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.
| destroySessionForProfile: (profileId: string): void => { | |
| const state = get(); | |
| const profileSession = state.profileSessions.get(profileId); | |
| if (!profileSession) { | |
| logger.warn(`No session found for profile ${profileId}`); | |
| return; | |
| } | |
| // Clear session data | |
| profileSession.clearStorageData(); | |
| profileSession.clearCache(); | |
| // Remove from maps | |
| set(state => { | |
| const newSessions = new Map(state.profileSessions); | |
| newSessions.delete(profileId); | |
| const profile = state.profiles.get(profileId); | |
| if (profile) { | |
| delete profile.session; | |
| } | |
| return { profileSessions: newSessions }; | |
| }); | |
| logger.info(`Destroyed session for profile ${profileId}`); | |
| }, | |
| destroySessionForProfile: (profileId: string): void => { | |
| const state = get(); | |
| const profileSession = state.profileSessions.get(profileId); | |
| if (!profileSession) { | |
| logger.warn(`No session found for profile ${profileId}`); | |
| return; | |
| } | |
| // Clear session data | |
| profileSession.clearStorageData(); | |
| profileSession.clearCache(); | |
| // Remove from maps | |
| set(state => { | |
| const newSessions = new Map(state.profileSessions); | |
| newSessions.delete(profileId); | |
| const profile = state.profiles.get(profileId); | |
| if (profile) { | |
| profile.session = undefined; | |
| } | |
| return { profileSessions: newSessions }; | |
| }); | |
| logger.info(`Destroyed session for profile ${profileId}`); | |
| }, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 2124-2124: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🤖 Prompt for AI Agents
In apps/electron-app/src/main/store/user-profile-store.ts around lines 2104 to
2131, replace the use of the delete operator on the profile.session property
with an assignment to undefined to avoid V8 deoptimization and improve
performance. Specifically, instead of using "delete profile.session;", assign
"profile.session = undefined;" to clear the session reference.
| @@ -0,0 +1,82 @@ | |||
| on: pull_request | |||
| name: pull_request | |||
|
|
|||
There was a problem hiding this comment.
The GitHub workflow files in this PR should be removed. They already exist in main and are identical here. Keeping these duplicates adds unnecessary noise to the diff and could cause merge conflicts.
| @@ -0,0 +1,42 @@ | |||
| # Commit Changes | |||
There was a problem hiding this comment.
.claude code should be in .gitignore. Everyone has their own coding preferences, and committing these files to the repo will conflict with local setups. CI is there to make sure lint, typecheck, and build all succeed, and we use semver for commits and PRs. You can keep using your own Claude configs locally, just don’t check them in.
Summary
This PR establishes the CI/CD infrastructure with GitHub Actions workflows for automated testing and building on pull requests and pushes.
Changes
Commits
Summary by CodeRabbit
New Features
Documentation
Chores