feature/custom terminal configs #717
Conversation
…tions - Add TerminalConfig types to settings schema (global & project-specific) - Create RC generator with hex-to-xterm-256 color mapping - Create RC file manager for .automaker/terminal/ directory - Add terminal theme color data (40 themes) to platform package - Integrate terminal config injection into TerminalService - Support bash, zsh, and sh with proper env var injection (BASH_ENV, ZDOTDIR, ENV) - Add onThemeChange hook for theme synchronization Part of custom terminal configurations feature implementation. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Pass SettingsService to TerminalService constructor - Initialize terminal service with settings service dependency - Enable terminal config injection to work with actual settings This completes Steps 1-4 of the terminal configuration plan: - RC Generator (color mapping, prompt formats) - RC File Manager (file I/O, atomic writes) - Settings Schema (GlobalSettings + ProjectSettings) - Terminal Service Integration (env var injection) Next steps: Settings UI and theme change hooks. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Complete Steps 5 & 6 of terminal configuration implementation: Settings UI Components: - Add PromptPreview component with live theme-aware rendering - Add TerminalConfigSection with comprehensive controls: * Enable/disable toggle with confirmation dialog * Custom prompt toggle * Prompt format selector (4 formats) * Git branch/status toggles * Custom aliases textarea * Custom env vars key-value editor with validation * Info box explaining behavior - Integrate into existing TerminalSection Theme Change Hook: - Add theme detection in update-global settings route - Regenerate RC files for all projects when theme changes - Skip projects with terminal config disabled - Error handling with per-project logging - Inject terminal service with settings service dependency This completes the full terminal configuration feature: ✓ RC Generator (color mapping, prompts) ✓ RC File Manager (file I/O, versioning) ✓ Settings Schema (types, defaults) ✓ Terminal Service Integration (env vars, PTY spawn) ✓ Settings UI (comprehensive controls, preview) ✓ Theme Synchronization (automatic RC regeneration) New terminals will use custom prompts matching app theme. Existing terminals unaffected. User RC files preserved. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…inal config - Add try-catch block to handleToggleEnabled - Explicitly set all required terminalConfig fields - Add console logging for debugging - Show error toast if update fails - Include rcFileVersion: 1 in config object This should fix the issue where the toggle doesn't enable after clicking OK in the confirmation dialog. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The issue was that `updateGlobalSettings` doesn't exist in the app store. The correct pattern is to use the `useUpdateGlobalSettings` hook from use-settings-mutations.ts, which is a React Query mutation. Changes: - Import useUpdateGlobalSettings from mutations hook - Use mutation.mutate() instead of direct function call - Add proper onSuccess/onError callbacks - Remove async/await pattern (React Query handles this) This fixes the toggle not enabling after clicking OK in the confirmation dialog. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The root cause: Component was reading globalSettings from the app store, which doesn't update reactively when the mutation completes. Solution: Use useGlobalSettings() React Query hook which: - Automatically refetches when the mutation invalidates the cache - Triggers re-render with updated data - Makes the toggle reflect the new state Now the flow is: 1. User clicks toggle → confirmation dialog 2. Click OK → mutation.mutate() called 3. Mutation succeeds → invalidates queryKeys.settings.global() 4. Query refetches → component re-renders with new globalSettings 5. Toggle shows enabled state ✓ Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add logging to track: - When terminal config check happens - CWD being used - Global and project enabled states - Effective enabled state This will help diagnose why RC files aren't being generated when opening terminals in Automaker. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary of ChangesHello @DhanushSantosh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant feature enabling users to customize their terminal experience within the application. It provides an opt-in mechanism for generating theme-aware shell prompts, custom aliases, and environment variables, all managed through a new, intuitive settings interface. The changes ensure seamless integration with existing shell setups by creating configuration files in a dedicated directory and dynamically updating them based on user preferences and theme changes. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature for custom terminal configurations, including themed prompts, aliases, and environment variables. The implementation spans both the server and UI, with new platform-level libraries for generating and managing shell configuration files. The changes are well-structured and follow a detailed implementation plan.
I've found a high-severity bug in the theme change handling logic that could lead to data loss for project-specific terminal settings. I've also identified a medium-severity issue in the new settings UI that could cause stale state. My review includes suggestions to address both of these issues. Overall, this is a great addition to the application.
| const effectiveConfig: TerminalConfig = { | ||
| enabled: true, | ||
| customPrompt: terminalConfig.customPrompt, | ||
| promptFormat: terminalConfig.promptFormat, | ||
| showGitBranch: terminalConfig.showGitBranch, | ||
| showGitStatus: terminalConfig.showGitStatus, | ||
| customAliases: terminalConfig.customAliases, | ||
| customEnvVars: terminalConfig.customEnvVars, | ||
| rcFileVersion: terminalConfig.rcFileVersion, | ||
| }; |
There was a problem hiding this comment.
The onThemeChange method incorrectly regenerates RC files using only global terminal settings. It doesn't account for project-specific overrides for customAliases and customEnvVars. This will cause project-specific configurations to be lost whenever the global theme is changed.
To fix this, you should fetch and merge the project-specific settings within onThemeChange, similar to how it's done in the createSession method.
You should add the following lines before fetching theme colors (around line 804):
const projectSettings = await this.settingsService.getProjectSettings(projectPath);
const projectTerminalConfig = projectSettings?.terminalConfig;Then, update the effectiveConfig object to use the project-specific settings, like this:
const effectiveConfig: TerminalConfig = {
enabled: true,
customPrompt: terminalConfig.customPrompt,
promptFormat: terminalConfig.promptFormat,
showGitBranch: terminalConfig.showGitBranch,
showGitStatus: terminalConfig.showGitStatus,
customAliases: projectTerminalConfig?.customAliases || terminalConfig.customAliases,
customEnvVars: {
...terminalConfig.customEnvVars,
...projectTerminalConfig?.customEnvVars,
},
rcFileVersion: terminalConfig.rcFileVersion,
};| const [localEnvVars, setLocalEnvVars] = useState<Array<{ key: string; value: string }>>( | ||
| Object.entries(globalSettings?.terminalConfig?.customEnvVars || {}).map(([key, value]) => ({ | ||
| key, | ||
| value, | ||
| })) | ||
| ); |
There was a problem hiding this comment.
The localEnvVars state is initialized from globalSettings but is not updated if globalSettings changes after the initial render (e.g., due to an update from another tab). This can lead to a stale UI.
To keep the component in sync with the global state, you should use a useEffect hook to update localEnvVars whenever globalSettings.terminalConfig.customEnvVars changes. You'll also need to add useEffect to your import from react.
const [localEnvVars, setLocalEnvVars] = useState<Array<{ key: string; value: string }>>([]);
useEffect(() => {
setLocalEnvVars(
Object.entries(globalSettings?.terminalConfig?.customEnvVars || {}).map(([key, value]) => ({
key,
value,
}))
);
}, [globalSettings?.terminalConfig?.customEnvVars]);
📝 WalkthroughWalkthroughAdds theme-synced, opt-in terminal configurations: RC generator and file manager in platform, 40 terminal themes, server TerminalService integration (SettingsService injection, RC regeneration on theme change, session env injection), UI for terminal config and live preview, types/settings additions, tests, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User/UI
participant SettingsRoute as Settings Route
participant ProjectStore as Project Store
participant TerminalService as Terminal Service
participant RCManager as RC File Manager
UI->>SettingsRoute: Update global theme
SettingsRoute->>SettingsRoute: Detect theme change
loop per project
SettingsRoute->>ProjectStore: Fetch project settings
ProjectStore-->>SettingsRoute: Return project settings
alt terminalConfig enabled
SettingsRoute->>TerminalService: onThemeChange(projectPath, newTheme)
TerminalService->>ProjectStore: Read terminalConfig (via SettingsService)
TerminalService->>RCManager: ensureRcFilesUpToDate(projectPath, newTheme, config, themeColors, allThemes)
RCManager->>RCManager: Generate theme files & RCs (atomic write, version/signature)
RCManager-->>TerminalService: Done
end
end
SettingsRoute-->>UI: Respond success/error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@apps/server/src/services/terminal-service.ts`:
- Around line 355-358: The code currently sets shellArgs from detectedShellArgs
even when options.shell overrides detectedShell; change it so shellArgs are
computed from the selected shell variable (shell) instead of the originally
detected one: after computing const shell = options.shell || detectedShell, call
the routine that derives args for a given shell (or map the shell to its args)
and assign shellArgs from that result (replace use of detectedShellArgs). Update
references in detectShell() usage and ensure any helper/mapping used (e.g.,
detectShell, detectShellArgs, or a new getShellArgsFor(shell) helper) returns
args for the chosen shell.
In
`@apps/ui/src/components/views/settings-view/terminal/terminal-config-section.tsx`:
- Around line 34-39: The local state localEnvVars initialized from
globalSettings.terminalConfig.customEnvVars only runs once (useState
initializer) so updates to globalSettings won't update the UI; add a useEffect
that listens for changes to globalSettings (or
globalSettings?.terminalConfig?.customEnvVars) and calls
setLocalEnvVars(Object.entries(...).map(({key,value}) => ({key,value}))) to
rehydrate localEnvVars whenever the settings load or change, preserving the same
shape as the initial state.
In `@apps/ui/src/components/views/settings-view/terminal/terminal-section.tsx`:
- Around line 304-309: The TerminalConfigSection is being rendered inside the
main TerminalSection card, causing a nested-card visual; update the JSX in the
TerminalSection component so TerminalConfigSection is moved out of the main card
container and rendered as a sibling (e.g., place <TerminalConfigSection /> after
the closing div of the main card that contains TerminalSection content),
preserving any spacing (mt-6) as needed so it appears consistent with other
settings sections.
In `@docs/terminal-custom-configs-plan.md`:
- Around line 210-219: The default rcFileVersion in the docs is outdated; update
the DEFAULT_TERMINAL_CONFIG object's rcFileVersion value from 1 to match the
current RC_FILE_VERSION (4) used in code so the doc and code are
consistent—locate DEFAULT_TERMINAL_CONFIG and set rcFileVersion: 4 to mirror
RC_FILE_VERSION.
- Around line 43-45: The docs list uses the filename "zshrc.zsh" but the
implementation writes ".zshrc" into ZDOTDIR; update the plan entry to match the
implementation by replacing "zshrc.zsh" with ".zshrc" (keep bashrc.sh and
common.sh unchanged) and ensure the comment notes that .zshrc is placed in the
ZDOTDIR directory so the doc and the code (ZDOTDIR/.zshrc) are consistent.
- Around line 41-53: Several fenced code blocks in the terminal configs plan are
missing language identifiers (MD040); update each triple-backtick block
containing the ASCII tree and shell prompt examples (the `.automaker/terminal/`
directory tree block and the various prompt examples like `[user@host]
~/path/to/project (main*) $`, `~/project (main*) $`, the
`┌─[user@host]─[~/path]─[main*]` block, and `user@host in ~/path on main* ❯`) to
include a language tag (use "text" or "bash" as appropriate) immediately after
the opening ``` so every fenced block has a language identifier.
In `@libs/platform/src/rc-generator.ts`:
- Around line 559-597: The generated zshrc in generateZshrc currently emits a
prompt that uses command substitution (via $(automaker_git_prompt)) but doesn’t
enable PROMPT_SUBST; update generateZshrc to emit the zsh option enabling prompt
substitution by inserting a `setopt PROMPT_SUBST` line into the generated output
(e.g., before the block that conditionally sets the prompt where promptLine is
injected) so that the promptLine (which contains $(automaker_git_prompt)) is
executed rather than shown literally.
In `@libs/platform/src/terminal-theme-colors.ts`:
- Around line 413-460: The header comments for the theme groups are inaccurate:
update the "Dark themes (16)" and "Light themes (16)" comments in the
terminalThemeColors mapping to reflect the actual counts (20 dark and 20 light)
to match the ThemeMode list and the file's "all 40 themes" remark; locate the
terminalThemeColors constant and replace those two comment strings accordingly
so they read "Dark themes (20)" and "Light themes (20)" (leave the "Special"
comment as-is).
🧹 Nitpick comments (3)
libs/platform/tests/rc-generator.test.ts (1)
30-46: Consider adding edge case tests for better coverage.The current tests verify happy-path scenarios. Consider adding tests for:
- Disabled configs (
enabled: false)- Different prompt formats
- Configs with
showGitBranch: falseorshowGitStatus: falseWould you like me to generate additional test cases for edge scenarios?
libs/types/src/settings.ts (1)
853-871: Consider extracting a shared type for terminal config.The
terminalConfigstructure here duplicates theTerminalConfiginterface defined inlibs/platform/src/rc-generator.ts. This creates a maintenance burden if fields change.♻️ Suggested approach
Define the type once (either here in settings.ts or import from rc-generator) and reuse it:
// Option 1: Import from platform (if platform can be a dependency) import type { TerminalConfig } from '@automaker/platform'; export interface GlobalSettings { // ... terminalConfig?: TerminalConfig; } // Option 2: Define here and export for platform to use export interface TerminalConfig { enabled: boolean; customPrompt: boolean; promptFormat: 'standard' | 'minimal' | 'powerline' | 'starship'; showGitBranch: boolean; showGitStatus: boolean; customAliases: string; customEnvVars: Record<string, string>; rcFileVersion?: number; }apps/server/src/routes/settings/routes/update-global.ts (1)
72-101: Theme change handling is well-implemented with good error resilience.The sequential per-project regeneration with individual error handling is appropriate. A few observations:
Logic is correct: The terminalConfigEnabled check properly handles both global and project-level settings.
Sequential execution is acceptable: While parallel execution via
Promise.allSettledcould be faster for many projects, sequential execution provides clearer logging and the operation is infrequent.Good error isolation: Per-project try/catch prevents one project's failure from affecting others.
One minor consideration for the future: if users have many projects, you could parallelize with:
await Promise.allSettled(projects.map(async (project) => { /* ... */ }));
apps/ui/src/components/views/settings-view/terminal/terminal-config-section.tsx
Outdated
Show resolved
Hide resolved
apps/ui/src/components/views/settings-view/terminal/terminal-section.tsx
Show resolved
Hide resolved
| ``` | ||
| .automaker/terminal/ | ||
| ├── bashrc.sh # Bash config (sourced via BASH_ENV) | ||
| ├── zshrc.zsh # Zsh config (via ZDOTDIR) | ||
| ├── common.sh # Shared functions (git prompt, etc.) | ||
| ├── themes/ | ||
| │ ├── dark.sh # Theme-specific color exports (40 files) | ||
| │ ├── dracula.sh | ||
| │ ├── nord.sh | ||
| │ └── ... (38 more) | ||
| ├── version.txt # RC file format version (for migrations) | ||
| └── user-custom.sh # User's additional customizations (optional) | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
markdownlint flags these blocks; please add a language (e.g., text, bash, typescript) to silence MD040.
📝 Suggested fix
-```
+```text
.automaker/terminal/
├── bashrc.sh
...
-```
+```
-```
+```text
[user@host] ~/path/to/project (main*) $
-```
+```
-```
+```text
~/project (main*) $
-```
+```
-```
+```text
┌─[user@host]─[~/path]─[main*]
└─$
-```
+```
-```
+```text
user@host in ~/path on main*
❯
-```
+```Also applies to: 390-393, 396-399, 402-405, 409-412
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/terminal-custom-configs-plan.md` around lines 41 - 53, Several fenced
code blocks in the terminal configs plan are missing language identifiers
(MD040); update each triple-backtick block containing the ASCII tree and shell
prompt examples (the `.automaker/terminal/` directory tree block and the various
prompt examples like `[user@host] ~/path/to/project (main*) $`, `~/project
(main*) $`, the `┌─[user@host]─[~/path]─[main*]` block, and `user@host in ~/path
on main* ❯`) to include a language tag (use "text" or "bash" as appropriate)
immediately after the opening ``` so every fenced block has a language
identifier.
| ├── bashrc.sh # Bash config (sourced via BASH_ENV) | ||
| ├── zshrc.zsh # Zsh config (via ZDOTDIR) | ||
| ├── common.sh # Shared functions (git prompt, etc.) |
There was a problem hiding this comment.
Doc mismatch: zsh RC filename differs from implementation.
The plan lists zshrc.zsh, but the implementation writes .zshrc in the ZDOTDIR directory. Please update this section to match the actual filename.
🤖 Prompt for AI Agents
In `@docs/terminal-custom-configs-plan.md` around lines 43 - 45, The docs list
uses the filename "zshrc.zsh" but the implementation writes ".zshrc" into
ZDOTDIR; update the plan entry to match the implementation by replacing
"zshrc.zsh" with ".zshrc" (keep bashrc.sh and common.sh unchanged) and ensure
the comment notes that .zshrc is placed in the ZDOTDIR directory so the doc and
the code (ZDOTDIR/.zshrc) are consistent.
| const DEFAULT_TERMINAL_CONFIG = { | ||
| enabled: false, | ||
| customPrompt: true, | ||
| promptFormat: 'standard' as const, | ||
| showGitBranch: true, | ||
| showGitStatus: true, | ||
| customAliases: '', | ||
| customEnvVars: {}, | ||
| rcFileVersion: 1, | ||
| }; |
There was a problem hiding this comment.
Doc mismatch: rcFileVersion default is outdated.
The plan states rcFileVersion: 1, but the code uses RC_FILE_VERSION = 4. Please update the docs to reflect the current version.
🤖 Prompt for AI Agents
In `@docs/terminal-custom-configs-plan.md` around lines 210 - 219, The default
rcFileVersion in the docs is outdated; update the DEFAULT_TERMINAL_CONFIG
object's rcFileVersion value from 1 to match the current RC_FILE_VERSION (4)
used in code so the doc and code are consistent—locate DEFAULT_TERMINAL_CONFIG
and set rcFileVersion: 4 to mirror RC_FILE_VERSION.
| /** | ||
| * Theme color mapping for all 40 themes | ||
| */ | ||
| export const terminalThemeColors: Record<ThemeMode, TerminalTheme> = { | ||
| // Special | ||
| system: darkTheme, // Resolved at runtime based on OS preference | ||
| // Dark themes (16) | ||
| dark: darkTheme, | ||
| retro: retroTheme, | ||
| dracula: draculaTheme, | ||
| nord: nordTheme, | ||
| monokai: monokaiTheme, | ||
| tokyonight: tokyonightTheme, | ||
| solarized: solarizedTheme, | ||
| gruvbox: gruvboxTheme, | ||
| catppuccin: catppuccinTheme, | ||
| onedark: onedarkTheme, | ||
| synthwave: synthwaveTheme, | ||
| red: redTheme, | ||
| sunset: sunsetTheme, | ||
| gray: grayTheme, | ||
| forest: gruvboxTheme, // Green-ish theme | ||
| ocean: nordTheme, // Blue-ish theme | ||
| ember: monokaiTheme, // Warm orange theme | ||
| 'ayu-dark': darkTheme, | ||
| 'ayu-mirage': darkTheme, | ||
| matcha: nordTheme, | ||
| // Light themes (16) | ||
| light: lightTheme, | ||
| cream: creamTheme, | ||
| solarizedlight: lightTheme, | ||
| github: lightTheme, | ||
| paper: lightTheme, | ||
| rose: lightTheme, | ||
| mint: lightTheme, | ||
| lavender: lightTheme, | ||
| sand: creamTheme, | ||
| sky: lightTheme, | ||
| peach: creamTheme, | ||
| snow: lightTheme, | ||
| sepia: creamTheme, | ||
| gruvboxlight: creamTheme, | ||
| nordlight: lightTheme, | ||
| blossom: lightTheme, | ||
| 'ayu-light': lightTheme, | ||
| onelight: lightTheme, | ||
| bluloco: lightTheme, | ||
| feather: lightTheme, |
There was a problem hiding this comment.
Update theme count comments to match the ThemeMode list.
The mapping has 20 dark and 20 light themes, but the comments still say 16 each.
📝 Suggested comment fix
- // Dark themes (16)
+ // Dark themes (20)
@@
- // Light themes (16)
+ // Light themes (20)📝 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.
| /** | |
| * Theme color mapping for all 40 themes | |
| */ | |
| export const terminalThemeColors: Record<ThemeMode, TerminalTheme> = { | |
| // Special | |
| system: darkTheme, // Resolved at runtime based on OS preference | |
| // Dark themes (16) | |
| dark: darkTheme, | |
| retro: retroTheme, | |
| dracula: draculaTheme, | |
| nord: nordTheme, | |
| monokai: monokaiTheme, | |
| tokyonight: tokyonightTheme, | |
| solarized: solarizedTheme, | |
| gruvbox: gruvboxTheme, | |
| catppuccin: catppuccinTheme, | |
| onedark: onedarkTheme, | |
| synthwave: synthwaveTheme, | |
| red: redTheme, | |
| sunset: sunsetTheme, | |
| gray: grayTheme, | |
| forest: gruvboxTheme, // Green-ish theme | |
| ocean: nordTheme, // Blue-ish theme | |
| ember: monokaiTheme, // Warm orange theme | |
| 'ayu-dark': darkTheme, | |
| 'ayu-mirage': darkTheme, | |
| matcha: nordTheme, | |
| // Light themes (16) | |
| light: lightTheme, | |
| cream: creamTheme, | |
| solarizedlight: lightTheme, | |
| github: lightTheme, | |
| paper: lightTheme, | |
| rose: lightTheme, | |
| mint: lightTheme, | |
| lavender: lightTheme, | |
| sand: creamTheme, | |
| sky: lightTheme, | |
| peach: creamTheme, | |
| snow: lightTheme, | |
| sepia: creamTheme, | |
| gruvboxlight: creamTheme, | |
| nordlight: lightTheme, | |
| blossom: lightTheme, | |
| 'ayu-light': lightTheme, | |
| onelight: lightTheme, | |
| bluloco: lightTheme, | |
| feather: lightTheme, | |
| /** | |
| * Theme color mapping for all 40 themes | |
| */ | |
| export const terminalThemeColors: Record<ThemeMode, TerminalTheme> = { | |
| // Special | |
| system: darkTheme, // Resolved at runtime based on OS preference | |
| // Dark themes (20) | |
| dark: darkTheme, | |
| retro: retroTheme, | |
| dracula: draculaTheme, | |
| nord: nordTheme, | |
| monokai: monokaiTheme, | |
| tokyonight: tokyonightTheme, | |
| solarized: solarizedTheme, | |
| gruvbox: gruvboxTheme, | |
| catppuccin: catppuccinTheme, | |
| onedark: onedarkTheme, | |
| synthwave: synthwaveTheme, | |
| red: redTheme, | |
| sunset: sunsetTheme, | |
| gray: grayTheme, | |
| forest: gruvboxTheme, // Green-ish theme | |
| ocean: nordTheme, // Blue-ish theme | |
| ember: monokaiTheme, // Warm orange theme | |
| 'ayu-dark': darkTheme, | |
| 'ayu-mirage': darkTheme, | |
| matcha: nordTheme, | |
| // Light themes (20) | |
| light: lightTheme, | |
| cream: creamTheme, | |
| solarizedlight: lightTheme, | |
| github: lightTheme, | |
| paper: lightTheme, | |
| rose: lightTheme, | |
| mint: lightTheme, | |
| lavender: lightTheme, | |
| sand: creamTheme, | |
| sky: lightTheme, | |
| peach: creamTheme, | |
| snow: lightTheme, | |
| sepia: creamTheme, | |
| gruvboxlight: creamTheme, | |
| nordlight: lightTheme, | |
| blossom: lightTheme, | |
| 'ayu-light': lightTheme, | |
| onelight: lightTheme, | |
| bluloco: lightTheme, | |
| feather: lightTheme, |
🤖 Prompt for AI Agents
In `@libs/platform/src/terminal-theme-colors.ts` around lines 413 - 460, The
header comments for the theme groups are inaccurate: update the "Dark themes
(16)" and "Light themes (16)" comments in the terminalThemeColors mapping to
reflect the actual counts (20 dark and 20 light) to match the ThemeMode list and
the file's "all 40 themes" remark; locate the terminalThemeColors constant and
replace those two comment strings accordingly so they read "Dark themes (20)"
and "Light themes (20)" (leave the "Special" comment as-is).
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/server/src/services/terminal-service.ts`:
- Around line 825-866: onThemeChange currently builds effectiveConfig only from
globalSettings. Fetch the project-specific settings and merge its terminalConfig
into the global terminalConfig before constructing effectiveConfig so project
overrides (customAliases, customEnvVars, pathStyle, pathDepth, etc.) are
preserved; for example, call the project settings method on this.settingsService
(e.g., this.settingsService.getProjectSettings(projectPath) or similar) to get
projectSettings?.terminalConfig, merge project terminalConfig over global
terminalConfig, then use the merged terminalConfig when creating effectiveConfig
and when calling ensureRcFilesUpToDate.
- Around line 433-452: The effectiveConfig is always taking globalTerminalConfig
values and uses || for customAliases which prevents a project from clearing
aliases; update effectiveConfig to apply per-project overrides by using
projectTerminalConfig?.<field> ?? globalTerminalConfig.<field> for each
overridable property (e.g., showUserHost, showPath, pathStyle (use
normalizePathStyle(projectTerminalConfig?.pathStyle ??
globalTerminalConfig.pathStyle)), pathDepth (use
normalizePathDepth(projectTerminalConfig?.pathDepth ??
globalTerminalConfig.pathDepth)), showTime, showExitStatus, and enabled if
applicable) and change customAliases to projectTerminalConfig?.customAliases ??
globalTerminalConfig.customAliases so an explicit empty value from the project
clears the global value; keep customEnvVars merge order but ensure it uses
projectTerminalConfig?.customEnvVars ?? {} on the right side.
In `@apps/ui/src/components/views/settings-view/terminal/prompt-preview.tsx`:
- Around line 109-173: The declarations inside the switch case 'powerline'
(powerlineSegments, powerlineCore, powerlineExtras, powerlineLine) must be
enclosed in a new block to avoid scope leakage across cases; wrap the entire
case 'powerline' body in braces { ... } so all const/let bindings (and the JSX
return) are block-scoped, leaving the switch labels and other cases unchanged.
In
`@apps/ui/src/components/views/settings-view/terminal/terminal-config-section.tsx`:
- Around line 126-133: The handler handleUpdateConfig currently calls
updateGlobalSettings.mutate without error handling; add an options object to
that mutate call with an onError callback (matching the pattern used in
applyEnabledUpdate) that surfaces the error to the user (e.g., show the existing
UI toast/error UI and/or console.error) so failures produce visible feedback;
keep the payload construction the same but pass { onError: (err) => { /* show
user-facing error and log err */ } } to updateGlobalSettings.mutate.
In `@start-automaker.sh`:
- Around line 12-16: The web mode still runs an unguarded export of the .env
file causing failures if .env is missing; locate the web) case block and wrap
the export command (the export $(grep -v '^#' .env | xargs) invocation) with a
file-existence check [ -f .env ] or remove it and rely on the top-level loader
(the earlier if [ -f .env ] ... set -a ... set +a block) so the export only runs
when .env exists.
🧹 Nitpick comments (7)
libs/platform/src/rc-file-manager.ts (1)
11-19: Prefer shared package entrypoints over relative imports here.Consider importing these generators/types via the
@automaker/platformbarrel (or exporting them there) instead of./rc-generator.js.As per coding guidelines:
**/*.{ts,tsx,js,jsx}: Always import from shared packages (@automaker/*), never from old paths or relative imports to other modules.apps/server/src/services/terminal-service.ts (1)
16-23: Route new dependencies through shared package entrypoints.These relative imports (e.g., SettingsService and terminal-themes-data) should come from an
@automaker/* entrypoint or a shared barrel export.As per coding guidelines:
**/*.{ts,tsx,js,jsx}: Always import from shared packages (@automaker/*), never from old paths or relative imports to other modules.libs/platform/tests/rc-generator.test.ts (1)
1-5: Prefer package entrypoint imports in tests.Use the
@automaker/platformexports rather than../src/*paths to align with the shared import guideline.As per coding guidelines:
**/*.{ts,tsx,js,jsx}: Always import from shared packages (@automaker/*), never from old paths or relative imports to other modules.apps/ui/src/components/views/settings-view/terminal/terminal-config-section.tsx (3)
92-92: Consider removing or conditionalizing debug log.This
console.logwill output to production browser console. Consider removing it or gating behind a debug flag.♻️ Suggested fix
- console.log('[TerminalConfig] Updating settings with:', updatedConfig); + // Debug logging removed for production
155-173: Consider debouncing settings updates for env var edits.
updateEnvVarcallshandleUpdateConfigon every keystroke, which may trigger excessive API mutations. Consider debouncing or updating settings only on blur.♻️ Suggested approach
// Option 1: Update on blur instead of onChange // Option 2: Debounce the handleUpdateConfig call import { useDebouncedCallback } from 'use-debounce'; const debouncedUpdateConfig = useDebouncedCallback( (updates) => handleUpdateConfig(updates), 500 );
451-452: Avoid using array index as React key for mutable lists.Using
indexaskeycan cause rendering issues when items are added/removed/reordered. Consider using a stable identifier.♻️ Suggested fix
+ const [localEnvVars, setLocalEnvVars] = useState<Array<{ id: string; key: string; value: string }>>( + Object.entries(globalSettings?.terminalConfig?.customEnvVars || {}).map(([key, value]) => ({ + id: crypto.randomUUID(), + key, + value, + })) + ); // In addEnvVar: - setLocalEnvVars([...localEnvVars, { key: '', value: '' }]); + setLocalEnvVars([...localEnvVars, { id: crypto.randomUUID(), key: '', value: '' }]); // In JSX: - <div key={index} className="flex gap-2 items-start"> + <div key={envVar.id} className="flex gap-2 items-start">apps/ui/src/components/views/settings-view/terminal/prompt-preview.tsx (1)
89-107: Wrapcase 'minimal'in a block for consistency.While this case doesn't have declarations that leak, wrapping all cases in blocks improves consistency and prevents future issues.
♻️ Suggested pattern for all cases
switch (format) { case 'minimal': { return (/* ... */); } case 'powerline': { const powerlineSegments: ReactNode[] = []; // ... return (/* ... */); } // etc. }
apps/ui/src/components/views/settings-view/terminal/prompt-preview.tsx
Outdated
Show resolved
Hide resolved
apps/ui/src/components/views/settings-view/terminal/terminal-config-section.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@start-automaker.sh`:
- Around line 1161-1163: The export invocation using command substitution
(export $(grep -v '^#' .env | xargs)) must be quoted to prevent word splitting
and preserve values with spaces/special chars; replace it with a quoted command
substitution (export "..." around the grep|xargs output) or, even safer, iterate
lines from .env with a while read -r loop and export each key=value to avoid
xargs splitting—update the export line referencing the .env parsing (grep -v
'^#' .env | xargs) accordingly.
🧹 Nitpick comments (5)
apps/ui/tests/features/edit-feature.spec.ts (1)
83-83: Avoid hardcodedwaitForTimeoutin favor of condition-based waits.
waitForTimeout(2000)is a Playwright anti-pattern that can cause flaky tests (if 2s is insufficient) or unnecessarily slow execution (if the condition is met sooner). Consider waiting for a specific condition instead, such as a network request completing or the feature card becoming visible.♻️ Suggested alternative
- await page.waitForTimeout(2000); + // Wait for feature creation API to complete + await page.waitForResponse( + (response) => response.url().includes('/features') && response.status() === 200, + { timeout: 10000 } + );Or rely solely on the
.toPass()assertion below which already polls for the expected state.apps/ui/src/components/views/settings-view/terminal/terminal-config-section.tsx (1)
50-60: Minor: Remove constant from dependency array.
ENV_VAR_ID_PREFIXis a constant defined outside the component, so including it in the dependency array is unnecessary. This is a very minor nit.♻️ Optional fix
const createEnvVarEntry = useCallback( (key = '', value = '') => { envVarIdRef.current += 1; return { id: `${ENV_VAR_ID_PREFIX}-${envVarIdRef.current}`, key, value, }; }, - [ENV_VAR_ID_PREFIX] + [] );libs/platform/src/rc-file-manager.ts (3)
71-79: Consider cleaning up temp file on rename failure.If
fs.renamefails (e.g., cross-device move on some systems), the.tmpfile will be left behind. Adding cleanup in a catch block would improve robustness.♻️ Optional improvement
async function atomicWriteFile( filePath: string, content: string, mode: number = 0o644 ): Promise<void> { const tempPath = `${filePath}.tmp`; await fs.writeFile(tempPath, content, { encoding: 'utf8', mode }); - await fs.rename(tempPath, filePath); + try { + await fs.rename(tempPath, filePath); + } catch (error) { + // Clean up temp file on failure + await fs.unlink(tempPath).catch(() => {}); + throw error; + } }
125-134: Remove unused error variable.The
errorvariable in the catch block is unused. Either use it for logging or remove it.♻️ Suggested fix
} catch (error) { - return null; // File doesn't exist or can't be read + } catch { + return null; // File doesn't exist or can't be read }
277-284: Consider logging unexpected errors in deleteTerminalDir.The empty catch block silently ignores all errors, including unexpected ones. While ignoring "file not found" is intentional, other errors (permission denied, etc.) might warrant logging.
♻️ Optional improvement
export async function deleteTerminalDir(projectPath: string): Promise<void> { const terminalDir = getTerminalDir(projectPath); try { await fs.rm(terminalDir, { recursive: true, force: true }); - } catch (error) { - // Ignore errors if directory doesn't exist + } catch (error: unknown) { + // Ignore ENOENT (directory doesn't exist), log others + if (error instanceof Error && 'code' in error && error.code !== 'ENOENT') { + console.warn(`[rc-file-manager] Failed to delete terminal dir: ${error.message}`); + } } }
| if [ -f .env ]; then | ||
| export $(grep -v '^#' .env | xargs) | ||
| fi |
There was a problem hiding this comment.
Quote command substitution to prevent word splitting.
The static analysis tool correctly flags that $(grep ... | xargs) should be quoted to handle values containing spaces or special characters.
🔧 Suggested fix
if [ -f .env ]; then
- export $(grep -v '^#' .env | xargs)
+ set -a
+ . ./.env
+ set +a
fiAlternatively, if you prefer export:
if [ -f .env ]; then
- export $(grep -v '^#' .env | xargs)
+ while IFS='=' read -r key value; do
+ [ -n "$key" ] && export "$key=$value"
+ done < <(grep -v '^#' .env | grep -v '^$')
fi📝 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.
| if [ -f .env ]; then | |
| export $(grep -v '^#' .env | xargs) | |
| fi | |
| if [ -f .env ]; then | |
| set -a | |
| . ./.env | |
| set +a | |
| fi |
| if [ -f .env ]; then | |
| export $(grep -v '^#' .env | xargs) | |
| fi | |
| if [ -f .env ]; then | |
| while IFS='=' read -r key value; do | |
| [ -n "$key" ] && export "$key=$value" | |
| done < <(grep -v '^#' .env | grep -v '^$') | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 1162-1162: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
In `@start-automaker.sh` around lines 1161 - 1163, The export invocation using
command substitution (export $(grep -v '^#' .env | xargs)) must be quoted to
prevent word splitting and preserve values with spaces/special chars; replace it
with a quoted command substitution (export "..." around the grep|xargs output)
or, even safer, iterate lines from .env with a while read -r loop and export
each key=value to avoid xargs splitting—update the export line referencing the
.env parsing (grep -v '^#' .env | xargs) accordingly.
DhanushSantosh
added
Enhancement
- Remove macOS-only platform restriction for Warp - Add Linux CLI alias 'warp-terminal' (primary on Linux) - Add CLI launch handler using --cwd flag - Fixes issue where Warp was not detected on Linux systems Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/platform/src/terminal.ts (1)
449-451: Misleading comment: no AppleScript is used here.The comment states "use AppleScript to cd" but the implementation simply passes
targetPathas an argument toopen -a. This approach relies on Warp's native handling of directory arguments rather than AppleScript.📝 Proposed fix to correct the comment
} else if (id === 'warp') { - // Warp: Open app and use AppleScript to cd + // Warp: Open app with target directory as argument await execFileAsync('open', ['-a', appPath, targetPath]);
🤖 Fix all issues with AI agents
In `@libs/platform/src/terminal.ts`:
- Around line 143-146: Update the Warp CLI config and launch logic: remove the
unsupported 'warp' alias from cliAliases and keep only 'warp-cli' and
'warp-terminal' (cliAliases) and ensure cliCommand remains 'warp-cli'; then fix
launch behavior that tries to use the '--cwd' flag directly—either invoke the
documented "warp agent run --cwd <path>" flow when you need to set working dir
with cliCommand, or if launching the GUI on macOS (macAppName) implement a
proper AppleScript that opens Warp and changes directory instead of relying on
'--cwd' or claiming "use AppleScript to cd"; finally remove or correct the
misleading comment that says "use AppleScript to cd" where the code just runs
open -a Warp targetPath.
| cliCommand: 'warp-cli', | ||
| cliAliases: ['warp-terminal', 'warp'], | ||
| macAppName: 'Warp', | ||
| platform: 'darwin', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the relevant sections
if [ -f "libs/platform/src/terminal.ts" ]; then
echo "=== Lines 140-150 (Warp definition) ==="
sed -n '140,150p' libs/platform/src/terminal.ts
echo ""
echo "=== Lines 445-455 (AppleScript comment) ==="
sed -n '445,455p' libs/platform/src/terminal.ts
echo ""
echo "=== Lines 475-485 (Warp CLI case) ==="
sed -n '475,485p' libs/platform/src/terminal.ts
echo ""
echo "=== Imports at top of file ==="
head -20 libs/platform/src/terminal.ts
else
echo "File not found"
fiRepository: AutoMaker-Org/automaker
Length of output: 1808
🌐 Web query:
Warp terminal CLI command warp-cli Linux installation
💡 Result:
On Linux, the standalone Warp CLI package installs via your distro package manager (after adding Warp’s repo):
# after adding the Warp apt repo/signing key
sudo apt update
sudo apt install warp-cli
# (preview)
sudo apt install warp-cli-previewIf you haven’t added Warp’s apt repo yet, follow Warp’s Linux repo setup steps (it installs the signing key + repo list file) and then install the package. [1][2]
CLI command names on Linux:
- Standalone CLI:
warp-cli(orwarp-cli-preview) - If you installed the full Warp app:
warp-terminal(orwarp-terminal-preview) [1]
Sources:
[1] Warp CLI docs (install + command names): https://docs.warp.dev/platform/cli
[2] Warp Linux installation (repo setup commands): https://docs.warp.dev/getting-started/readme/installation-and-setup
🌐 Web query:
Warp terminal --cwd flag documentation
💡 Result:
Warp documents --cwd (and its short form -C) as a flag on the Warp CLI command warp agent run:
--cwd <PATH>/-C <PATH>: run the agent from a different working directory than the current one. [1]warp agent run-ambientdoes not support--cwd(the selected environment determines the working directory). [1]
Example:
warp agent run -C /path/to/repo --prompt "Summarize this repo"[1]
Sources
- [1] Warp Docs — “Warp CLI” (Running agents /
warp agent runflags): https://docs.warp.dev/platform/cli
The Warp CLI command usage needs correction; --cwd flag and warp alias may not work as intended.
The web search confirms warp-cli and warp-terminal are valid Warp CLI commands on Linux, enabling cross-platform support. However, two issues require attention:
- The
warpalias (line 144) is not documented as a valid Warp terminal command—onlywarp-cliandwarp-terminalappear in official Warp CLI documentation. - The
--cwdflag (line 480) is documented only forwarp agent runcommands, not for directly launching the Warp terminal. The current code may fail to set the working directory when usingwarp-clidirectly. - The comment at line 449 ("use AppleScript to cd") is misleading—the code runs
open -a Warp targetPathwithout any AppleScript execution.
Verify the correct approach for launching Warp with a specific working directory, and either remove the unsupported warp alias or confirm it as a valid fallback.
🤖 Prompt for AI Agents
In `@libs/platform/src/terminal.ts` around lines 143 - 146, Update the Warp CLI
config and launch logic: remove the unsupported 'warp' alias from cliAliases and
keep only 'warp-cli' and 'warp-terminal' (cliAliases) and ensure cliCommand
remains 'warp-cli'; then fix launch behavior that tries to use the '--cwd' flag
directly—either invoke the documented "warp agent run --cwd <path>" flow when
you need to set working dir with cliCommand, or if launching the GUI on macOS
(macAppName) implement a proper AppleScript that opens Warp and changes
directory instead of relying on '--cwd' or claiming "use AppleScript to cd";
finally remove or correct the misleading comment that says "use AppleScript to
cd" where the code just runs open -a Warp targetPath.
1 participant
Summary
Introduces real Oh My Posh theme rendering for Automaker terminals, with opt-in custom RC generation, theme-synced prompts, and richer prompt customization controls.
Changes
POSH_THEMES_PATH.warp-terminalCLI alias for cross-platform support.Demo
Testing
npx vitest run libs/platform/tests/rc-file-manager.test.tsnpx vitest run libs/platform/tests/rc-generator.test.tsnpm run build -w @automaker/typesnpm run build -w @automaker/platformnpm run build -w @automaker/serverSummary by CodeRabbit
New Features
Documentation
Tests
Chore
✏️ Tip: You can customize this high-level summary in your review settings.