feat(config): add automatic project type detection#20
Conversation
Implement .clix/config.jsonc for per-project settings: - Add ProjectConfigManager with JSONC support - Create SetupUI for first-run project setup flow - Display project info in ChatHeader (name, org, ID, user) - Store project public_key in local config - Remove global workspaces from config schema - Fix formatPath edge case for similar path prefixes
Add project type detection system that identifies framework (native, react-native, expo, flutter) and target platform (ios, android, both). Detection runs on login and saves to .clix/config.jsonc. Refactor Firebase detection to reuse already-detected ProjectType, eliminating redundant file system checks for platform detection.
WalkthroughAdds per-project config (.clix/config.jsonc) with schema and manager, first-run/setup flow and UI, project-type detection, makes Firebase services project-type aware, bumps global config version to 5, updates UIs/commands, and appends Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 078135a7a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/lib/config/project-config-manager.ts`:
- Around line 122-127: Replace the generic error code with the project-specific
one in the ConfigError throws: locate the ConfigError instantiations inside
project-config-manager.ts (where the code currently uses
ERROR_CODES.CONFIG_INVALID) and change them to use
ERROR_CODES.PROJECT_CONFIG_INVALID so project configuration errors are
categorized correctly; update both occurrences (the one near the first throw and
the second throw later in the file).
In `@src/lib/config/project-config-schema.ts`:
- Around line 60-62: The docstring and constants in project-config-schema.ts
currently point to ".clix/config.jsonc" in the repo root; update the docstring
and the config path constants (e.g., symbols like PROJECT_CONFIG_PATH,
CONFIG_FILENAME, SESSIONS_DIR or any getProjectConfigPath/getConfigPath helpers)
to resolve to the XDG locations instead: use $XDG_CONFIG_HOME/clix/config.json
for the main config filename and $XDG_STATE_HOME/clix/sessions/ for session
storage, and ensure the config manager code that resolves paths uses
process.env.XDG_CONFIG_HOME and process.env.XDG_STATE_HOME with sensible
fallbacks (~/.config and ~/.local/state) so no user data is written into
repositories.
In `@src/lib/services/first-run-service.ts`:
- Around line 20-32: SETUP_EXEMPT_COMMANDS currently omits the 'setup' command
which can cause re-entry/infinite-loop issues when setupCommand() is invoked;
update the SETUP_EXEMPT_COMMANDS array to include 'setup' (and optionally
'doctor' if you want diagnostics exempt) so first-run setup logic will skip
these commands, ensuring the first-run guard checks against 'setup' and 'doctor'
when deciding whether to run the setup flow.
In `@src/ui/LoginUI.tsx`:
- Around line 169-173: In handleProjectSelect's catch block, call the onError
prop with the caught error (or a constructed Error using the message) in
addition to setErrorMessage and setPhase('error') so failures propagate the same
way as the useEffect error path; locate the catch block inside the
handleProjectSelect function and invoke onError(error) (or onError(new
Error(message)) if error is not an Error) before or after setting state.
🧹 Nitpick comments (17)
src/lib/services/firebase/downloader.ts (1)
249-265: Consider adding a comment for the 'ios-android' case.When
frameworkis'native'andtargetis'ios-android', this returns'unknown'. While this works correctly (lines 239-240 fall back to serving both platforms for'unknown'), a brief comment would clarify this is intentional.📝 Suggested clarification
private projectTypeToPlatform(projectType: ProjectType): Platform { if (projectType.framework === 'flutter') { return 'flutter'; } if (projectType.framework === 'react-native' || projectType.framework === 'expo') { return 'react-native'; } if (projectType.framework === 'native') { if (projectType.target === 'ios') return 'ios'; if (projectType.target === 'android') return 'android'; + // 'ios-android' or 'unknown' target: return 'unknown' to serve both platforms return 'unknown'; } return 'unknown'; }src/lib/services/project-detector.ts (1)
155-178: Minor inefficiency with redundant filesystem checks.
hasIosProjectandhasAndroidProjectare called here (lines 170-171), and then again indetectTargetPlatform(called fromdetectProjectType). For projects detected as native, this results in 4 extra filesystem operations.This is a minor concern since detection typically runs once during setup, but if performance becomes noticeable, consider caching these results.
src/lib/config/project-config-manager.ts (3)
175-183: Inconsistent import style forunlink.The
delete()method dynamically importsunlinkwhilereadFile,writeFile,stat, andmkdirare statically imported at the top. Consider importingunlinkstatically for consistency and to avoid the async import overhead.♻️ Proposed fix
At the top of the file:
-import { mkdir, readFile, stat, writeFile } from 'node:fs/promises'; +import { mkdir, readFile, stat, unlink, writeFile } from 'node:fs/promises';In the
delete()method:async delete(): Promise<void> { - const { unlink } = await import('node:fs/promises'); try { await unlink(this.configFilePath);
91-97: Simplify directory creation logic.The
ensureConfigDirmethod checks if the directory exists before creating it, butmkdirwith{ recursive: true }already handles this gracefully and is a no-op if the directory exists.♻️ Proposed fix
private async ensureConfigDir(): Promise<void> { - try { - await stat(this.configDirPath); - } catch { - await mkdir(this.configDirPath, { recursive: true, mode: 0o755 }); - } + await mkdir(this.configDirPath, { recursive: true, mode: 0o755 }); }
246-248: Edge case: Escaped backslashes before quotes.The
isUnescapedQuotefunction only checks if the immediately preceding character is a backslash. This doesn't correctly handle sequences like\\"where the backslash itself is escaped, making the quote unescaped. For typical JSONC config files this is unlikely to be an issue, but worth noting.♻️ Proposed fix for robustness
/** Check if current position is an unescaped quote */ function isUnescapedQuote(content: string, index: number): boolean { - return content[index] === '"' && (index === 0 || content[index - 1] !== '\\'); + if (content[index] !== '"') return false; + // Count preceding backslashes + let backslashCount = 0; + let i = index - 1; + while (i >= 0 && content[i] === '\\') { + backslashCount++; + i--; + } + // Quote is unescaped if preceded by an even number of backslashes + return backslashCount % 2 === 0; }src/ui/chat/ChatApp.tsx (1)
52-60: Add error handling for config loading.The async
loadProjectConfigfunction doesn't handle potential errors. WhileprojectConfigremainingnullis a valid fallback, unhandled promise rejections can cause issues. Consider adding a try-catch or.catch().🛡️ Proposed fix
// Load project config on mount useEffect(() => { const loadProjectConfig = async () => { - const configManager = getProjectConfigManager(); - const config = await configManager.load(); - setProjectConfig(config); + try { + const configManager = getProjectConfigManager(); + const config = await configManager.load(); + setProjectConfig(config); + } catch { + // Config load failed - leave as null, ChatHeader handles this gracefully + } }; loadProjectConfig(); }, []);src/commands/setup.tsx (1)
14-30: Promise may resolve twice due to overlapping completion paths.Both
onComplete(line 19) andwaitUntilExit().then()(line 28) callresolve(). While callingresolve()multiple times is harmless (the Promise ignores subsequent calls), this creates unclear control flow. Consider removing the redundantwaitUntilExit().then()block sinceonCompleteandonErroralready handle all completion scenarios.♻️ Proposed simplification
export async function setupCommand(options?: SetupCommandOptions): Promise<void> { return new Promise((resolve, reject) => { - const { waitUntilExit } = safeRender( + safeRender( <SetupUI projectPath={options?.projectPath} onComplete={() => { resolve(); }} onError={(error) => { reject(error); }} />, ); - - waitUntilExit().then(() => { - resolve(); - }); }); }src/ui/chat/components/ChatHeader.tsx (1)
62-68: Minor: Inconsistent prefix spacing forproject_idlabel.The
idPrefixvalue' project_id:'lacks a trailing space, unlike other prefixes (e.g.,' project: ',' user: '). This may cause slight visual misalignment if the intent was consistent column alignment.💅 Proposed fix for consistency
- const idPrefix = ' project_id:'; + const idPrefix = ' project_id: ';src/ui/SetupUI.tsx (5)
166-169: EmptyhandleProjectSkipcallback may cause UX confusion.The callback is passed to
ProjectSelectorwithshowSkip={false}, but if the skip behavior changes in the future orshowSkipis accidentally enabled, pressing Esc would do nothing with no feedback. Consider removing the callback entirely or adding a comment explaining why it's intentionally empty.
281-311: MissinguseCancelInputhook for cancellation handling.The "waiting for auth" phase shows "Press Ctrl+C to cancel" but doesn't use the
useCancelInputhook from@/ui/hooksfor proper cancellation handling. This could leave the component in an inconsistent state if the user cancels.Based on learnings: "Import and use
useCancelInputhook from@/ui/hooksfor ESC/Ctrl+C cancellation handling in new UI components."♻️ Proposed fix to add cancellation handling
import { useCallback, useEffect, useRef, useState } from 'react'; +import { useCancelInput } from '@/ui/hooks';Then add in the component:
+ useCancelInput(() => { + if (phase === 'waiting_for_auth' || phase === 'starting_auth') { + pkceServiceRef.current?.abort(); + if (onError) { + onError(new Error('Setup cancelled by user')); + } else { + exit(); + } + } + });
50-90: Code duplication with LoginUI.tsx.The helper functions
fetchMember,fetchUserName, andfetchOrganizationsWithProjectsare nearly identical to those inLoginUI.tsx. Consider extracting these into a shared utility module to reduce duplication and ensure consistent behavior.
82-85: Sequential API calls could be parallelized.Fetching projects for each organization is done sequentially in a loop. For users with many organizations, this could be slow. Consider using
Promise.allfor parallel fetching.♻️ Proposed optimization
async function fetchOrganizationsWithProjects(): Promise<OrgWithProjects[]> { - const orgsWithProjects: OrgWithProjects[] = []; try { const apiClient = getInternalApiClient(); const orgs = await apiClient.listOrganizations(); - for (const org of orgs) { - const projects = await apiClient.listProjects(org.id); - orgsWithProjects.push({ org, projects }); - } + const orgsWithProjects = await Promise.all( + orgs.map(async (org) => ({ + org, + projects: await apiClient.listProjects(org.id), + })) + ); + return orgsWithProjects; } catch { // Silently ignore org/project fetch errors + return []; } - return orgsWithProjects; }
147-153: Magic number 1500ms timeout should be extracted as a constant.The 1500ms delay before exit/completion and 2000ms delay on error are magic numbers. Consider extracting these as named constants for clarity and consistency.
Also applies to: 256-257
src/lib/services/firebase/detector.ts (1)
57-70: DuplicateprojectTypeToPlatformfunction across multiple files.This function is duplicated in
firebase-service.ts(lines 25-38) anddownloader.ts(lines 251-264). Consider extracting it to a shared utility in the firebase module to maintain DRY principles.♻️ Suggested extraction
Create a shared utility in
src/lib/services/firebase/utils.ts:import type { ProjectType } from '@/lib/config'; import type { Platform } from './types'; export function projectTypeToPlatform(projectType: ProjectType): Platform { if (projectType.framework === 'flutter') { return 'flutter'; } if (projectType.framework === 'react-native' || projectType.framework === 'expo') { return 'react-native'; } if (projectType.framework === 'native') { if (projectType.target === 'ios') return 'ios'; if (projectType.target === 'android') return 'android'; return 'unknown'; } return 'unknown'; }Then import and use it in detector.ts, firebase-service.ts, and downloader.ts.
src/ui/LoginUI.tsx (1)
48-52: Code duplication:fetchMemberhelper is identical to SetupUI.tsx.This function and
fetchUserName,fetchOrganizationsWithProjectsare duplicated betweenLoginUI.tsxandSetupUI.tsx. Consider extracting to a shared module like@/lib/api/helpers.tsor@/ui/utils/auth-helpers.ts.src/lib/services/firebase/firebase-service.ts (1)
22-38: DuplicateprojectTypeToPlatformfunction.As noted in the detector.ts review, this function is duplicated across three files. Extract to a shared utility.
src/lib/config/project-config-schema.ts (1)
65-66: Avoid version drift between schema literal and constant.Use the exported constant in the schema to keep versioning single-sourced.
♻️ Suggested refactor
-export const ProjectConfigSchema = z.object({ - /** Configuration schema version */ - version: z.literal(1), +export const CURRENT_PROJECT_CONFIG_VERSION = 1; + +export const ProjectConfigSchema = z.object({ + /** Configuration schema version */ + version: z.literal(CURRENT_PROJECT_CONFIG_VERSION), @@ -export const CURRENT_PROJECT_CONFIG_VERSION = 1;Also applies to: 89-90
- Add explicit 'setup' case to CLI switch statement (P1) - Add 'setup' to SETUP_EXEMPT_COMMANDS to prevent double execution - Persist detected project type in SetupUI (P2) - Use PROJECT_CONFIG_INVALID error code for better categorization - Add onError callback in LoginUI handleProjectSelect catch block Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/cli.tsx`:
- Around line 210-212: The 'setup' case in the CLI currently no-ops when
shouldRunSetup() returns false (because needsSetup is false), leaving users with
no feedback; update the case handler for 'setup' in src/cli.tsx to detect
shouldRunSetup()/needsSetup and if false print a clear user-facing message like
"Setup already complete — nothing to do" (or alternatively call the existing
setup routine unconditionally), referencing the existing shouldRunSetup and
needsSetup logic so you either invoke the setup function regardless or emit the
message before breaking out of the case.
In `@src/lib/config/project-config-manager.ts`:
- Around line 246-248: The isUnescapedQuote function incorrectly treats a quote
as escaped if the immediate previous character is a backslash; update
isUnescapedQuote to count consecutive backslashes immediately before
content[index] and treat the quote as unescaped when the count is even
(including zero) and escaped when odd; keep the check that content[index] ===
'"' and implement parity-based logic on the backslash run to decide escaping for
isUnescapedQuote.
🧹 Nitpick comments (3)
src/lib/config/project-config-manager.ts (3)
14-16: Consider using English for the config header comment.The config header uses Korean text. For consistency and broader accessibility in an open-source CLI tool, consider using English or making it configurable/localizable.
-const CONFIG_HEADER = `// Clix CLI 프로젝트 설정 -// 자동 생성됨 - 수동 수정 시 덮어쓰기될 수 있음 +const CONFIG_HEADER = `// Clix CLI project configuration +// Auto-generated - manual edits may be overwritten `;
91-97: SimplifyensureConfigDir- the stat check is redundant.With
recursive: true,mkdirsilently succeeds if the directory already exists. The stat-then-mkdir pattern adds unnecessary I/O and a minor TOCTOU window.♻️ Proposed simplification
private async ensureConfigDir(): Promise<void> { - try { - await stat(this.configDirPath); - } catch { - await mkdir(this.configDirPath, { recursive: true, mode: 0o755 }); - } + await mkdir(this.configDirPath, { recursive: true, mode: 0o755 }); }
175-183: Inconsistent import style forunlink.
unlinkis dynamically imported here, whilereadFile,writeFile,mkdir, andstatare statically imported at the top. This is inconsistent and adds unnecessary overhead.♻️ Proposed fix
Add
unlinkto the static imports at line 1:-import { mkdir, readFile, stat, writeFile } from 'node:fs/promises'; +import { mkdir, readFile, stat, unlink, writeFile } from 'node:fs/promises';Then simplify the
deletemethod:async delete(): Promise<void> { - const { unlink } = await import('node:fs/promises'); try { await unlink(this.configFilePath); this.cachedConfig = null; } catch { // File doesn't exist, ignore } }
- Add user feedback when `clix setup` is run on already configured project - Fix isUnescapedQuote to correctly handle escaped backslashes (count consecutive backslashes to determine parity) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/cli.tsx`:
- Around line 210-220: Replace the dynamic import of checkFirstRun with a static
import alongside shouldRunSetup from './lib/services/first-run-service' (so both
are imported at module top), then simplify the 'setup' case to call the
statically imported checkFirstRun and, if not needing setup, update the message
to correctly tell users how to reconfigure (e.g., "Project already configured.
Run `clix setup` to reconfigure." or another appropriate reconfigure command)
instead of suggesting `clix login`; keep the existing call to setupCommand when
status.needsSetup is true.
- Replace dynamic import with static import for consistency - Simplify setup already configured message Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Adds automatic project type detection system to Clix CLI. The system detects mobile projects (Expo, React Native, Flutter, native iOS/Android) and their target platforms, enabling smarter setup workflows and project-specific configurations.
Details
This PR introduces three main components:
Project Type Detection (): Analyzes project structure to identify framework (Expo, React Native, Flutter, native) and target platform (iOS, Android, or both).
Project-Local Configuration (, ): Stores project-specific settings in
.clix/config.jsonwithin each project, enabling persistent configurations without polluting user-level config.First-Run Service (): Handles initial setup workflows, including project detection and configuration initialization.
The system integrates with existing configuration management and provides an interactive setup UI for users.
Related Issues
Related to #17 (iOS APNS and Firebase setup integration) and #18-19 (UI improvements)
How to Validate
bun run devand test interactive modebun run dev setupto trigger project detection.clix/config.jsonis created with correct project typebun run dev doctorcommand to verify project settingsbun run check && bun test && bun run build && bun test tests/e2e/Pre-Merge Checklist
Code Quality
bun run build)bun run typecheck)bun run lint)bun test)Documentation
Commit Standards
Platform Validation
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Style