fix: stabilize PR272 new-session UX, profiles gating, and env safety#13
fix: stabilize PR272 new-session UX, profiles gating, and env safety#13leeroybrun wants to merge 0 commit intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds Gemini model-mode selection and persistence, extensive profile and picker UIs (machine/path/profile), machine-aware environment-variable parsing and daemon-backed previews, refactors modal typing and item/group utilities, removes legacy profile sync and permission selector, and migrates translations to per-language files; includes many tests and platform/web fixes. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User (UI)
participant M as EnvironmentVariablesPreviewModal
participant H as useEnvironmentVariables (hook)
participant RPC as Daemon RPC (machinePreviewEnv)
participant DB as Storage/Cache
U->>M: Open preview (env vars, machineId)
M->>H: request var resolution (varNames, options)
H->>DB: check cache / extraEnv / sensitiveHints
alt daemon supports preview
H->>RPC: call machinePreviewEnv(machineId, request)
RPC-->>H: preview response (meta, policy)
H->>DB: persist meta/cache
H-->>M: return variables + meta + policy
else fallback
H->>RPC: run shell-based probe (or JSON path)
RPC-->>H: raw output
H->>DB: persist parsed results
H-->>M: return variables + meta
end
M->>U: render preview with masked/resolved values
Note: colored rectangles not required for this simple flow. Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
🚥 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. 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 |
Summary of ChangesHello @leeroybrun, 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 focuses on stabilizing and refining the new session experience, profile management, and environment variable handling introduced in a previous large update. The primary goal is to restore a familiar user experience while making powerful new features optional, coherent, and secure. Key areas of improvement include UI consistency, data integrity, and preventing potential security risks related to environment variable exposure. Highlights
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 is an excellent and comprehensive pull request that successfully stabilizes the new session and profiles features introduced in a previous PR. The changes are well-structured and address a wide range of issues, from restoring the original user experience to fixing critical safety and correctness bugs.
Key improvements include:
- Restoring the modal-based "New Session" flow and making the new wizard optional.
- Gating profiles and other new features behind settings, making them opt-in.
- Significantly improving the robustness of settings parsing to prevent data loss.
- Enhancing security by preventing secret-like environment variables from being fetched or logged.
- Refactoring UI into reusable components like
MachineSelector,PathSelector, andSearchHeader, which greatly improves code quality and consistency. - Improving the correctness of environment variable handling and tool result parsing.
- Adding comprehensive test coverage for new and modified logic.
The code quality is very high, and the changes demonstrate a deep understanding of the issues being addressed. I have no specific comments for code changes as the implementation is solid. This PR is a major step forward for the stability and maintainability of the application.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
sources/text/README.md (1)
220-223: Update future language file example to matchtranslations/<locale>.tslayout.The doc suggests
_spanish.ts, but earlier sections and repo conventions point totranslations/en.tsand locale files undertranslations/. This example can mislead new contributors.sources/components/CommandPalette/CommandPaletteProvider.tsx (1)
124-124: Missing dependency inuseMemo.
navigateToSessionis used inside theuseMemocallback (line 92) but is not included in the dependency array. This could cause stale closure issues where navigation uses an outdated reference.- }, [router, logout, sessions]); + }, [router, logout, sessions, navigateToSession]);sources/sync/profileUtils.ts (1)
36-227: Localize user‑visible profile documentation content.Descriptions, env var explanations, and shell examples are user‑visible. Please move them to translations and use
t(...)to keep i18n consistent. As per coding guidelines, ...🌐 Example pattern (apply broadly)
-import { AIBackendProfile } from './settings'; +import { AIBackendProfile } from './settings'; +import { t } from '@/text'; ... - description: 'Official Anthropic Claude API - uses your default Anthropic credentials', + description: t('profiles.docs.anthropic.description'), ... - shellConfigExample: `# No additional environment variables needed -# Uses ANTHROPIC_AUTH_TOKEN from your login session`, + shellConfigExample: t('profiles.docs.anthropic.shellConfigExample'),sources/text/translations/pt.ts (1)
1-71: Add missing type annotation and import to en.ts to match other language files.The English translation file is missing the
TranslationStructuretype annotation and import statement. All other language files (ru, pl, es, pt) haveimport type { TranslationStructure } from '../_types';and use the type on export (export const en: TranslationStructure = {), but en.ts only hasexport const en = {without typing. This violates TypeScript strict mode requirements and makes the reference implementation untyped.
🤖 Fix all issues with AI agents
In `@sources/app/`(app)/session/[id]/info.tsx:
- Around line 325-333: The subtitle mapping for the AI provider currently maps
flavors 'gpt' and 'openai' to t('agentInput.agent.codex') but misses the 'codex'
flavor, causing it to fall back to the raw string; update the IIFE used for
Item's subtitle to include 'codex' in the conditional (alongside 'gpt' and
'openai') when checking session.metadata.flavor so that 'codex' returns
t('agentInput.agent.codex') instead of the raw flavor.
In `@sources/components/AgentInput.tsx`:
- Around line 510-546: The permissionChipLabel computation can return an empty
string when props.permissionMode is a value not valid for the current flavor
(isCodex/isGemini), causing the chip/overlay to show no selection; update the
logic in the permissionChipLabel useMemo (and the parallel block around the
other occurrence) to first normalize/clamp props.permissionMode into the active
flavor's allowed set (e.g., map invalid values to a sensible default like
'default' or a flavor-valid equivalent) and then compute the label based on that
normalizedMode (use props.permissionMode, isCodex, isGemini and
permissionChipLabel as anchors to find the code), ensuring the normalized mode
is used consistently wherever the chip and overlay selection are derived.
In `@sources/components/ItemGroup.tsx`:
- Around line 103-115: The selectable count logic in selectableItemCount
currently only treats title as valid when it's a string, causing items with
ReactNode titles to be ignored; update the countSelectable function (used in
selectableItemCount) to consider a title present when propsAny?.title is not
null/undefined and not the empty string (special-case ""), rather than requiring
typeof === 'string', while keeping the isSelectable checks for
onPress/onLongPress and preserving Fragment recursion in countSelectable; ensure
references to ItemChildProps, title, onPress, and onLongPress remain the same.
In `@sources/components/newSession/PathSelector.tsx`:
- Line 436: Folder icon sizes are inconsistent in PathSelector.tsx (the Ionicons
instances used as leftElement in the favorites group use size={18} while other
groups use size={24}); update the Ionicons size to 24 for the favorites group
(and the other listed occurrences) so all leftElement icons use a consistent
size, or replace hardcoded sizes with a single shared constant used by the
Ionicons in PathSelector to ensure uniformity across the component.
In `@sources/components/SearchableListSelector.tsx`:
- Line 364: The fallback for allSectionTitle is brittle: instead of using
config.recentSectionTitle.replace('Recent ', 'All ') in the ItemGroup title,
change the logic in the component that renders ItemGroup (reference ItemGroup,
config.allSectionTitle, config.recentSectionTitle, and showAll) to use a safe
fallback—either (a) require config.allSectionTitle when showAll is true and
throw or surface a missing-prop error, or (b) derive a neutral default like a
localized string "All" + " " + config.recentSectionTitle (without attempting to
replace a prefix) or simply use a generic "All" label—update the title
expression to prefer config.allSectionTitle, then fall back to the safe default;
ensure no direct string replace on recentSectionTitle is used.
In `@sources/components/SearchHeader.tsx`:
- Around line 105-112: The clear icon (Ionicons rendered when value.length > 0)
is not keyboard/screen-reader accessible; wrap the Ionicons in a Pressable (or
set accessibility props on the touchable) and move the onPress handler there
(call onChangeText('') on press), add accessibilityRole="button" and an
accessibilityLabel using the localized string (e.g., t('clear')) and optionally
accessibilityHint, while preserving styles.clearIcon; update SearchHeader.tsx to
ensure the Pressable contains the Ionicons and uses the localized label for
screen readers.
In `@sources/components/tools/knownTools.tsx`:
- Around line 183-199: The code calls
resolvePath(opts.tool.input.locations[0].path, opts.metadata) without ensuring
locations[0].path is a string; add a type guard before calling resolvePath that
checks opts.tool.input.locations exists, is an array, that locations[0] exists
and typeof locations[0].path === 'string' (and non-empty if appropriate), and
only then call resolvePath; otherwise fall back to t('tools.names.readFile').
Apply the same guard fix for both the "Read" and "read" tool renderings
referenced in the comment (the other block around the 210-231 range) so neither
attempts to resolve non-string or malformed paths.
- Around line 614-649: The title extractor in the 'edit' tool can assign
non-string values to filePath from nested fields
(opts.tool.input.toolCall.content[0].path, opts.tool.input.input[0].path,
opts.tool.input.path), which can make resolvePath crash; update the title
function inside the 'edit' tool to perform explicit typeof checks (e.g., typeof
... === 'string') before assigning to filePath or returning
opts.tool.input.toolCall.title, and only call resolvePath(filePath,
opts.metadata) when filePath is a validated string; keep existing lookup order
but add these type guards to prevent non-string values from propagating to
resolvePath.
In `@sources/sync/permissionMapping.ts`:
- Around line 21-49: The mapping currently escalates 'read-only' to 'default'
instead of preserving it; update the two switch branches that translate between
Claude and Codex/Gemini (the block guarded by !fromCodexLike && toCodexLike and
the subsequent reverse switch) to include a case for 'read-only' that returns
'read-only' (use the existing mode variable and switch statements), leaving
other cases and the default unchanged.
In `@sources/sync/persistence.ts`:
- Around line 166-168: The NewSessionDraft parsing currently casts
parsed.modelMode to ModelMode if it's a string, which allows invalid values to
pass through; update the parsing in the NewSessionDraft construction to validate
parsed.modelMode using the existing guard (e.g., isSessionModelMode or
isModelMode) instead of just typeof, and only accept the value when the guard
returns true (fall back to 'default' otherwise); reference parsed.modelMode,
ModelMode, and the guard functions isSessionModelMode/isModelMode to locate and
change the logic so corrupted storage can't inject invalid modelMode values.
In `@sources/sync/profileMutations.ts`:
- Around line 17-26: The duplicateProfileForEdit function currently appends the
literal " (Copy)" to profile.name; replace that literal with a call to the i18n
helper t(...) from "@/text" (import t at top) so the suffix is localized (e.g.,
name: `${profile.name} ${t("profiles.copy")}`), keep other fields unchanged, and
add the profiles.copy key to all language files/translations.
🧹 Nitpick comments (34)
sources/components/SessionTypeSelector.tsx (1)
37-40: Remove unusedthemevariable.The
themedestructured fromuseUnistyles()is never used since styles are accessed directly from the module-levelstylesheet. This leaves a dead binding.Suggested fix
export function SessionTypeSelectorRows({ value, onChange }: Pick<SessionTypeSelectorProps, 'value' | 'onChange'>) { - const { theme } = useUnistyles(); const styles = stylesheet;sources/hooks/useCLIDetection.ts (1)
55-57: Consider using adebugLoghelper for consistency.The inline
__DEV__guards work correctly, but the PR's other files (RealtimeVoiceSession.tsx,RealtimeVoiceSession.web.tsx) use adebugLoghelper function. For consistency and reduced boilerplate, consider extracting a shared helper or using the same pattern.♻️ Optional: Extract a local helper
+function debugLog(...args: unknown[]) { + if (!__DEV__) return; + console.log(...args); +} + export function useCLIDetection(machineId: string | null): CLIAvailability {Then replace inline guards:
- if (__DEV__) { - console.log('[useCLIDetection] Starting detection for machineId:', machineId); - } + debugLog('[useCLIDetection] Starting detection for machineId:', machineId);sources/sync/typesRaw.ts (1)
211-273: Consider adding.passthrough()to ACP data types for future API compatibility.The existing schemas in this file (e.g.,
rawTextContentSchema,rawToolUseContentSchema) use.passthrough()to preserve unknown fields for future API compatibility. The ACP data types currently only have.passthrough()ontoken_count(line 271). For consistency and forward compatibility, consider adding.passthrough()to other ACP data types, especiallyfile-edit,terminal-output, andpermission-requestwhich may evolve.sources/sync/typesRaw.spec.ts (1)
1493-1545: Good coverage for ACP tool result normalization.The tests validate that both
tool-resultandtool-call-resultACP types correctly normalize Claude-style text block arrays to plain text strings. The test structure follows the established patterns in this file.Consider adding additional test cases for other content shapes handled by
toolResultContentToText:
- String output:
output: 'direct string'- JSON object output:
output: { key: 'value' }- Empty/null output:
output: nullsources/components/Switch.web.tsx (1)
26-40: Consider forwarding remaining props for future extensibility, but current usages are unaffected.While the component currently works correctly for all existing use cases, it does not forward additional props like
testID,accessibilityLabel, orstylefrom the underlyingSwitchPropstype. All 37+ current usages only passvalue,disabled, andonValueChange, so this is not an active issue. However, if future code attempts to pass these props (e.g., for E2E testing), they will silently be ignored. For defensive programming, consider spreading rest props and merging custom styles—but this is not urgent given current usage patterns.sources/app/(app)/settings/voice/language.tsx (1)
13-72: Page component should be wrapped inmemo.Per coding guidelines, page components should always be wrapped in
memofor performance optimization.-export default function LanguageSelectionScreen() { +import { memo } from 'react'; + +export default memo(function LanguageSelectionScreen() { // ... existing implementation -} +});Based on coding guidelines: "Always wrap pages in memo".
sources/-session/SessionView.tsx (1)
200-206: Consider extracting the allowed model modes to a constant.The hardcoded whitelist of allowed modes could drift out of sync with the
ModelModetype. Extracting this to a constant or deriving from the type would improve maintainability.+const CONFIGURABLE_MODEL_MODES = ['default', 'gemini-2.5-pro', 'gemini-2.5-flash', 'gemini-2.5-flash-lite'] as const; + // Function to update model mode (for Gemini sessions) const updateModelMode = React.useCallback((mode: ModelMode) => { // Only Gemini model modes are configurable from the UI today. - if (mode === 'default' || mode === 'gemini-2.5-pro' || mode === 'gemini-2.5-flash' || mode === 'gemini-2.5-flash-lite') { + if ((CONFIGURABLE_MODEL_MODES as readonly string[]).includes(mode)) { storage.getState().updateSessionModelMode(sessionId, mode); } }, [sessionId]);sources/app/(app)/settings/features.tsx (1)
10-19: Wrap page component inmemoper coding guidelines.The coding guidelines specify that pages should always be wrapped in
memo. This component is missing the memo wrapper.♻️ Proposed fix
+import { memo } from 'react'; import { Platform } from 'react-native'; // ... other imports -export default function FeaturesSettingsScreen() { +function FeaturesSettingsScreen() { // ... component body } + +export default memo(FeaturesSettingsScreen);As per coding guidelines: "Always wrap pages in memo".
sources/sync/messageMeta.test.ts (1)
4-42: Consider adding test coverage forfallbackModel.The tests cover
modelanddisplayTextinclusion, butfallbackModelis also an optional field with the same conditional inclusion logic. Adding a test for it would ensure complete coverage of the builder function's optional fields.✅ Suggested additional test
it('includes fallbackModel when explicitly provided', () => { const meta = buildOutgoingMessageMeta({ sentFrom: 'web', permissionMode: 'default', fallbackModel: 'gpt-4', appendSystemPrompt: 'PROMPT', }); expect(meta.fallbackModel).toBe('gpt-4'); expect('fallbackModel' in meta).toBe(true); });sources/components/tools/views/GeminiEditView.tsx (1)
62-73: Remove unnecessary fragment wrapper.The fragment (
<>...</>) wrapping a singleToolSectionViewchild is redundant.♻️ Proposed fix
return ( - <> - <ToolSectionView fullWidth> - <ToolDiffView - oldText={oldString} - newText={newString} - showLineNumbers={showLineNumbersInToolViews} - showPlusMinusSymbols={showLineNumbersInToolViews} - /> - </ToolSectionView> - </> + <ToolSectionView fullWidth> + <ToolDiffView + oldText={oldString} + newText={newString} + showLineNumbers={showLineNumbersInToolViews} + showPlusMinusSymbols={showLineNumbersInToolViews} + /> + </ToolSectionView> );sources/sync/profileUtils.ts (1)
3-14: Avoid returning unexpected CLI keys ingetProfilePrimaryCli.Casting arbitrary
compatibilitykeys can yield a value outside the allowed union if persisted data contains new/unknown keys. Filter against an explicit allow‑list to keep the return value stable.♻️ Suggested hardening
-export function getProfilePrimaryCli(profile: AIBackendProfile | null | undefined): ProfilePrimaryCli { - if (!profile) return 'none'; - const supported = Object.entries(profile.compatibility ?? {}) - .filter(([, isSupported]) => isSupported) - .map(([cli]) => cli as 'claude' | 'codex' | 'gemini'); +const SUPPORTED_CLIS = ['claude', 'codex', 'gemini'] as const; + +export function getProfilePrimaryCli(profile: AIBackendProfile | null | undefined): ProfilePrimaryCli { + if (!profile) return 'none'; + const supported = SUPPORTED_CLIS.filter((cli) => profile.compatibility?.[cli]); if (supported.length === 0) return 'none'; if (supported.length === 1) return supported[0]; return 'multi'; }sources/sync/storageTypes.ts (1)
73-74: Use shared PermissionMode/ModelMode types to avoid drift.These unions already exist in
permissionTypes.ts; reusing them keeps session typing aligned with the rest of the system (and future model additions).♻️ Suggested refactor
-import { z } from "zod"; +import { z } from "zod"; +import type { PermissionMode, ModelMode } from './permissionTypes'; ... - permissionMode?: 'default' | 'acceptEdits' | 'bypassPermissions' | 'plan' | 'read-only' | 'safe-yolo' | 'yolo'; // Local permission mode, not synced to server - modelMode?: 'default' | 'gemini-2.5-pro' | 'gemini-2.5-flash' | 'gemini-2.5-flash-lite'; // Local model mode, not synced to server + permissionMode?: PermissionMode; // Local permission mode, not synced to server + modelMode?: ModelMode; // Local model mode, not synced to serversources/sync/permissionMapping.test.ts (1)
4-29: Good test coverage for the core mapping paths.The tests cover the main cross-agent permission mappings well. Consider adding a few more cases for completeness:
- Identity mapping when
from === to(e.g.,mapPermissionModeAcrossAgents('plan', 'claude', 'claude')should return'plan')- Default mode mappings (e.g., Claude
'default'→ Codex/Gemini)- Unmapped modes like
'read-only'from Claude → Codex/Gemini (per the implementation, these fall through to'default')💡 Suggested additional test cases
+ it('returns the same mode when from and to agents are identical', () => { + expect(mapPermissionModeAcrossAgents('plan', 'claude', 'claude')).toBe('plan'); + expect(mapPermissionModeAcrossAgents('yolo', 'codex', 'codex')).toBe('yolo'); + }); + + it('maps Claude default to Codex/Gemini default', () => { + expect(mapPermissionModeAcrossAgents('default', 'claude', 'gemini')).toBe('default'); + }); + + it('falls back to default for unmapped Claude modes', () => { + expect(mapPermissionModeAcrossAgents('read-only', 'claude', 'codex')).toBe('default'); + });sources/sync/profileMutations.ts (1)
28-36: Consider preserving originalcreatedAtfor converted profiles.When converting a built-in profile to custom, resetting
createdAttoDate.now()loses the original creation timestamp. Typically,createdAtshould be preserved (or intentionally set to now for "new" entities), while onlyupdatedAtreflects the mutation time.If this is intentional behavior (treating the conversion as a "new" profile), please disregard.
💡 Alternative to preserve original createdAt
export function convertBuiltInProfileToCustom(profile: AIBackendProfile): AIBackendProfile { return { ...profile, id: randomUUID(), isBuiltIn: false, - createdAt: Date.now(), + // createdAt preserved from original updatedAt: Date.now(), }; }sources/components/ItemRowActions.tsx (3)
47-84: UseStyleSheet.createfor styling per coding guidelines.The inline style on Line 48 should be moved to a stylesheet. Per coding guidelines: "Use StyleSheet.create from 'react-native-unistyles' for all styling."
♻️ Suggested refactor to use StyleSheet
+import { StyleSheet } from 'react-native-unistyles'; + +const stylesheet = StyleSheet.create((theme) => ({ + container: { + flexDirection: 'row', + alignItems: 'center', + }, +})); export function ItemRowActions(props: ItemRowActionsProps) { const { theme } = useUnistyles(); + const { styles } = useUnistyles(stylesheet); // ... return ( - <View style={{ flexDirection: 'row', alignItems: 'center', gap }}> + <View style={[styles.container, { gap }]}>
54-57: Avoidanytype for event parameter.Using
e: anybypasses TypeScript's type safety. Use the proper event type from React Native.🔧 Suggested type fix
- onPress={(e: any) => { - e?.stopPropagation?.(); + onPress={(e) => { + e.stopPropagation?.(); action.onPress(); }}Note:
GestureResponderEventfromreact-nativeis the proper type, though TypeScript should infer it from thePressablecomponent'sonPressprop. The optional chaining onstopPropagationhandles cross-platform differences.
49-65: Consider adding accessibility labels for action buttons.The inline action buttons lack accessibility properties, which may impact screen reader users.
♿ Suggested accessibility improvement
<Pressable key={action.id} hitSlop={{ top: 10, bottom: 10, left: 10, right: 10 }} + accessibilityRole="button" + accessibilityLabel={action.title} onPressIn={() => props.onActionPressIn?.()} onPress={(e) => {sources/components/ItemActionsMenuModal.tsx (1)
61-64: Consider potential unmount timing with deferred action execution.The
setTimeout(..., 0)pattern defers action execution until after the modal closes. If the action triggers navigation that unmounts this component, the timeout callback will still execute, which is fine for navigation but could cause issues if the callback references component state.This is a minor concern since the callback only calls the action's
onPress, but consider usingrequestAnimationFramefor slightly more predictable timing, or document that actions should be self-contained.sources/sync/profileGrouping.ts (1)
47-51: Document the empty-string marker convention.The empty string
''is used as a special marker for "default environment" that's tracked infavoriteIdsbut doesn't correspond to a real profile object. The inline comment is helpful, but consider adding a module-level constant (e.g.,DEFAULT_ENV_MARKER = '') to make this convention explicit and prevent magic string usage elsewhere.💡 Optional: Extract constant for clarity
// At module level export const DEFAULT_ENV_MARKER = ''; // In buildProfileGroups if (favoriteProfileIds.includes(DEFAULT_ENV_MARKER)) { favoriteIds.add(DEFAULT_ENV_MARKER); }sources/sync/persistence.ts (1)
15-26: Consider deriving the whitelist from the type to prevent drift.The
SESSION_MODEL_MODESSet duplicates the values fromSession['modelMode']type defined instorageTypes.ts. If the type changes without a corresponding update to the whitelist, values will diverge. Add a comment documenting this dependency, or explore patterns to derive both from a single source of truth.sources/utils/envVarTemplate.test.ts (1)
4-30: Consider adding tests for invalid/edge-case inputs.The existing tests thoroughly cover valid templates. Consider adding tests for robustness:
it('returns null for invalid templates', () => { expect(parseEnvVarTemplate('plain text')).toBeNull(); expect(parseEnvVarTemplate('${foo}')).toBeNull(); // lowercase expect(parseEnvVarTemplate('${123BAD}')).toBeNull(); // starts with digit expect(parseEnvVarTemplate('$FOO')).toBeNull(); // missing braces });sources/components/newSession/EnvironmentVariablesPreviewModal.tsx (1)
216-227: Consider extracting the nested ternary to a lookup map.The nested ternary chain is functional but harder to read. A lookup object would improve clarity:
♻️ Optional refactor
+ const detailLabels: Record<DetailKind, string> = { + fixed: t('profiles.environmentVariables.previewModal.detail.fixed'), + machine: t('profiles.environmentVariables.previewModal.detail.machine'), + checking: t('profiles.environmentVariables.previewModal.detail.checking'), + fallback: t('profiles.environmentVariables.previewModal.detail.fallback'), + missing: t('profiles.environmentVariables.previewModal.detail.missing'), + }; + const detailLabel = (() => { if (!detailKind) return undefined; - return detailKind === 'fixed' - ? t('profiles.environmentVariables.previewModal.detail.fixed') - : detailKind === 'machine' - ? t('profiles.environmentVariables.previewModal.detail.machine') - : detailKind === 'checking' - ? t('profiles.environmentVariables.previewModal.detail.checking') - : detailKind === 'fallback' - ? t('profiles.environmentVariables.previewModal.detail.fallback') - : t('profiles.environmentVariables.previewModal.detail.missing'); + return detailLabels[detailKind]; })();sources/app/(app)/new/pick/machine.tsx (1)
32-36: Type assertionas neveris a workaround for navigation dispatch typing.The
SET_PARAMSdispatch pattern withas neverworks but is a type escape hatch. This is acceptable given the complexity of expo-router/react-navigation typing, but consider documenting why this assertion is needed.sources/components/newSession/PathSelector.tsx (1)
433-444: Inconsistent indentation in Item components.Several
Itemcomponents have extra leading tabs/spaces compared to the surrounding code structure. This appears to be a formatting inconsistency that should be cleaned up for readability.Also applies to: 474-484, 496-506, 538-548, 560-569
sources/sync/sync.ts (1)
230-233: Consolidate hardcoded Gemini model defaults.The default model
'gemini-2.5-pro'is hardcoded in multiple places across the codebase (sync.ts, SessionView.tsx, new/index.tsx). While this value is currently correct per the Gemini API stable release, the scattered hardcoding makes it harder to maintain as models evolve. Consider deriving the default from theSESSION_MODEL_MODESconstant defined in sources/sync/persistence.ts or extracting it to a single configuration point, so future model updates only require changes in one place.sources/components/ItemGroup.dividers.test.ts (1)
37-66: LGTM!Good test coverage for the core functionality. The tests validate both fragment handling and explicit override preservation, which are the main behaviors of
withItemGroupDividers.Consider adding edge case tests.
For completeness, you might want to add tests for:
- Empty children (returns unchanged)
- Single child (no divider shown)
- All items with explicit
showDivider={false}sources/app/(app)/new/pick/path.tsx (2)
14-49: Move the stylesheet to the end of the file.This page defines styles before the component; per the app guidelines, styles should be placed after the component to avoid layout shifts and keep file structure consistent. As per coding guidelines.
116-127: Avoid directSET_PARAMSdispatch; prefer expo-router APIs.
navigation.dispatch({ type: 'SET_PARAMS', ... })relies on react-navigation action types and can diverge from expo-router expectations. Please confirm the supported expo-router way to pass params back (e.g.,router.navigate/replaceto the parent route orrouter.setParamsif applicable) and adjust accordingly. As per coding guidelines.sources/components/ProfileEditForm.tsx (1)
58-94: Extract modal container/header inline styles into Unistyles.The modal’s inline style objects are re-created every render and bypass the project styling standard. Consider moving static styles into
StyleSheet.createand keep only dynamic dimensions inline (height/maxHeight). Based on learnings, align styling withreact-native-unistyles.♻️ Example extraction
function MachinePreviewModal(props: MachinePreviewModalProps) { const { theme } = useUnistyles(); + const styles = stylesheet; const { height: windowHeight } = useWindowDimensions(); @@ - <View style={{ - width: '92%', - maxWidth: 560, - height: maxHeight, - maxHeight, - backgroundColor: theme.colors.groupped.background, - borderRadius: 16, - overflow: 'hidden', - borderWidth: 1, - borderColor: theme.colors.divider, - flexShrink: 1, - }}> - <View style={{ - paddingHorizontal: 16, - paddingVertical: 12, - flexDirection: 'row', - alignItems: 'center', - justifyContent: 'space-between', - borderBottomWidth: 1, - borderBottomColor: theme.colors.divider, - }}> + <View style={[styles.machinePreviewModal, { height: maxHeight, maxHeight }]}> + <View style={styles.machinePreviewHeader}> @@ const stylesheet = StyleSheet.create((theme) => ({ + machinePreviewModal: { + width: '92%', + maxWidth: 560, + backgroundColor: theme.colors.groupped.background, + borderRadius: 16, + overflow: 'hidden', + borderWidth: 1, + borderColor: theme.colors.divider, + flexShrink: 1, + }, + machinePreviewHeader: { + paddingHorizontal: 16, + paddingVertical: 12, + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'space-between', + borderBottomWidth: 1, + borderBottomColor: theme.colors.divider, + }, inputContainer: { paddingHorizontal: 16, paddingVertical: 12, },sources/app/(app)/new/pick/profile.tsx (1)
57-68: Avoid direct react-navigation dispatch for param updates.
navigation.dispatch({ type: 'SET_PARAMS' ... })relies on react-navigation internals, while the project standard is to stick to expo-router APIs. Consider passing a return route in params and usingrouter.replace/router.navigate, or store the selection in shared state instead of dispatching raw actions. Based on learnings, stick to expo-router APIs.sources/app/(app)/settings/profiles.tsx (1)
255-272: Extract repeated row layout styles into Unistyles.The right-element layout uses inline style objects (
row+indicator slot) that are re-created each render and bypass the unistyles styling standard. Consider moving them intoStyleSheet.create(e.g., extendprofileManagerStyles) and reusing across all groups. Based on learnings, keep styling inreact-native-unistyles.♻️ Example refactor (apply similarly to other groups)
- rightElement={( - <View style={{ flexDirection: 'row', alignItems: 'center', gap: 16 }}> - <View style={{ width: 24, alignItems: 'center', justifyContent: 'center' }}> + rightElement={( + <View style={profileManagerStyles.rowRightElement}> + <View style={profileManagerStyles.indicatorSlot}> @@ const profileManagerStyles = StyleSheet.create((theme) => ({ + rowRightElement: { + flexDirection: 'row', + alignItems: 'center', + gap: 16, + }, + indicatorSlot: { + width: 24, + alignItems: 'center', + justifyContent: 'center', + }, modalOverlay: { position: 'absolute', top: 0,sources/app/(app)/new/pick/profile-edit.tsx (1)
93-111: Navigation listener cleanup pattern could be more explicit.The
beforeRemovelistener returns the subscription directly, but the subscription type may beundefinedifaddListenerdoesn't exist. The cleanup function should handle this safely.Suggested improvement
React.useEffect(() => { - const subscription = (navigation as any)?.addListener?.('beforeRemove', (e: any) => { + const unsubscribe = (navigation as any)?.addListener?.('beforeRemove', (e: any) => { if (!isDirtyRef.current) return; e.preventDefault(); void (async () => { const decision = await confirmDiscard(); if (decision === 'discard') { isDirtyRef.current = false; (navigation as any).dispatch(e.data.action); } else if (decision === 'save') { saveRef.current?.(); } })(); }); - return subscription; + return () => unsubscribe?.(); }, [confirmDiscard, navigation]);sources/components/EnvironmentVariablesList.tsx (1)
41-41: Constant regex doesn't need useMemo.
SECRET_NAME_REGEXis a static regular expression that doesn't depend on any props or state. Creating it withuseMemoadds unnecessary overhead.Suggested fix
- const SECRET_NAME_REGEX = React.useMemo(() => /TOKEN|KEY|SECRET|AUTH|PASS|PASSWORD|COOKIE/i, []); + const SECRET_NAME_REGEX = /TOKEN|KEY|SECRET|AUTH|PASS|PASSWORD|COOKIE/i;Move this outside the component or declare it as a constant at the top of the function without
useMemo.sources/components/EnvironmentVariableCard.tsx (1)
334-357: Consider extracting shared input styles.
valueInputandsourceInputshare nearly identical styles (background, border radius, padding, font, web-specific outline removal). This could be extracted to a shared base style to reduce duplication.Optional refactor
const baseInputStyle = { ...Typography.default('regular'), backgroundColor: theme.colors.input.background, borderRadius: 10, paddingHorizontal: 12, paddingVertical: Platform.select({ ios: 10, default: 12 }), fontSize: Platform.select({ ios: 17, default: 16 }), lineHeight: Platform.select({ ios: 22, default: 24 }), letterSpacing: Platform.select({ ios: -0.41, default: 0.15 }), color: theme.colors.input.text, ...(Platform.select({ web: { /* ... */ }, default: {} }) as object), }; // Then use in stylesheet: valueInput: { ...baseInputStyle, marginBottom: 4 }, sourceInput: { ...baseInputStyle, marginBottom: 6 },Also applies to: 399-422
| <Item | ||
| key={path} | ||
| title={path} | ||
| leftElement={<Ionicons name="folder-outline" size={24} color={theme.colors.textSecondary} />} |
There was a problem hiding this comment.
Inconsistent icon sizes across groups.
The folder icon in favorites group uses size={18} (line 477) while all other groups use size={24}. This creates visual inconsistency between sections.
♻️ Suggested fix
- leftElement={<Ionicons name="folder-outline" size={18} color={theme.colors.textSecondary} />}
+ leftElement={<Ionicons name="folder-outline" size={24} color={theme.colors.textSecondary} />}Also applies to: 477-477, 499-499, 541-541, 563-563
🤖 Prompt for AI Agents
In `@sources/components/newSession/PathSelector.tsx` at line 436, Folder icon
sizes are inconsistent in PathSelector.tsx (the Ionicons instances used as
leftElement in the favorites group use size={18} while other groups use
size={24}); update the Ionicons size to 24 for the favorites group (and the
other listed occurrences) so all leftElement icons use a consistent size, or
replace hardcoded sizes with a single shared constant used by the Ionicons in
PathSelector to ensure uniformity across the component.
sources/sync/profileMutations.ts
Outdated
| export function duplicateProfileForEdit(profile: AIBackendProfile): AIBackendProfile { | ||
| return { | ||
| ...profile, | ||
| id: randomUUID(), | ||
| name: `${profile.name} (Copy)`, | ||
| isBuiltIn: false, | ||
| createdAt: Date.now(), | ||
| updatedAt: Date.now(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the AIBackendProfile type definition
find . -name "settings.ts" -path "*/sync/*" | head -5Repository: leeroybrun/happy
Length of output: 86
🏁 Script executed:
# Also check the imports and structure of profileMutations.ts
head -20 sources/sync/profileMutations.tsRepository: leeroybrun/happy
Length of output: 646
🏁 Script executed:
# Search for AIBackendProfile definition
rg "interface AIBackendProfile|type AIBackendProfile" -A 15Repository: leeroybrun/happy
Length of output: 1087
🏁 Script executed:
# Search for AIBackendProfileSchema definition
rg "const AIBackendProfileSchema|export.*AIBackendProfileSchema" -A 30Repository: leeroybrun/happy
Length of output: 3960
🏁 Script executed:
# Also look for the structure of environmentVariables in the schema
rg "environmentVariables" sources/sync/settings.ts -B 2 -A 2Repository: leeroybrun/happy
Length of output: 762
🏁 Script executed:
# Search for EnvironmentVariableSchema definition
rg "EnvironmentVariableSchema" sources/sync/settings.ts -B 2 -A 5Repository: leeroybrun/happy
Length of output: 581
🏁 Script executed:
# Also check if there's any usage of environmentVariables being mutated anywhere
rg "environmentVariables" sources/ --type ts --type tsx -B 2 -A 2Repository: leeroybrun/happy
Length of output: 87
🏁 Script executed:
# Search for environmentVariables usage with correct file type flags
rg "environmentVariables" sources/ -t ts -B 2 -A 2Repository: leeroybrun/happy
Length of output: 50372
🏁 Script executed:
# Check if the " (Copy)" string appears anywhere else in the codebase
rg "Copy" sources/ -t ts -B 1 -A 1Repository: leeroybrun/happy
Length of output: 27834
🏁 Script executed:
# Check if there are any similar i18n usages in the codebase for reference
rg "t\(" sources/sync/ -t ts | head -10Repository: leeroybrun/happy
Length of output: 1042
🏁 Script executed:
# Search for existing copy/suffix translation keys in the translations
rg "copySuffix|copy.*suffix" sources/text/translations/ -iRepository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Check the full en.ts translation file for profile-related strings
rg "profiles\." sources/text/translations/en.ts | head -20Repository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Check current imports in profileMutations.ts
head -5 sources/sync/profileMutations.tsRepository: leeroybrun/happy
Length of output: 230
🏁 Script executed:
# Search for existing translation keys related to profile copy/duplication
rg "profiles\." sources/text/translations/en.ts | grep -i "copy\|duplicate\|suffix"Repository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Check all profile-related translation keys
rg "profiles\." sources/text/translations/en.tsRepository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Also verify that the file is a .ts file not .tsx
file sources/sync/profileMutations.tsRepository: leeroybrun/happy
Length of output: 104
🏁 Script executed:
# List all keys in the profiles section
rg "profiles:" sources/text/translations/en.ts -A 100 | head -50Repository: leeroybrun/happy
Length of output: 2508
🏁 Script executed:
# Check the structure of environmentVariables more carefully - are they objects with mutable nested fields?
cat sources/sync/settings.ts | grep -A 10 "EnvironmentVariableSchema"Repository: leeroybrun/happy
Length of output: 883
Use t() function for the "(Copy)" string.
Per coding guidelines, all user-visible strings must use the t(...) function from @/text for internationalization. The " (Copy)" suffix in the profile name is visible to users and should be internationalized.
Suggested fix
+import { t } from '@/text';
+
export function duplicateProfileForEdit(profile: AIBackendProfile): AIBackendProfile {
return {
...profile,
id: randomUUID(),
- name: `${profile.name} (Copy)`,
+ name: `${profile.name} ${t('profiles.copy')}`,
isBuiltIn: false,
createdAt: Date.now(),
updatedAt: Date.now(),
};
}Note: You'll need to add the profiles.copy translation key to all language files.
🤖 Prompt for AI Agents
In `@sources/sync/profileMutations.ts` around lines 17 - 26, The
duplicateProfileForEdit function currently appends the literal " (Copy)" to
profile.name; replace that literal with a call to the i18n helper t(...) from
"@/text" (import t at top) so the suffix is localized (e.g., name:
`${profile.name} ${t("profiles.copy")}`), keep other fields unchanged, and add
the profiles.copy key to all language files/translations.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@sources/app/`(app)/settings/voice/language.tsx:
- Around line 39-43: SearchHeader currently hardcodes accessibilityLabel="Clear
search"; change this so the label is localized via t(...) instead of a hardcoded
string: either (A) add a prop like clearAccessibilityLabel or clearLabel to
SearchHeader and pass t('settingsVoice.language.clearSearch') from the caller in
sources/app/(app)/settings/voice/language.tsx, or (B) inject i18n into
SearchHeader and replace the hardcoded "Clear search" with
t('settingsVoice.language.clearSearch'); update the SearchHeader prop types (if
adding a prop) and all callers accordingly to avoid regressions.
In `@sources/components/AgentInput.tsx`:
- Around line 392-395: The stylesheet entry styles.micIcon currently defines
width and height but Expo's Image sizing must be inline; remove width and height
from the micIcon style object and instead add those properties directly to the
Image component's style prop where styles.micIcon is used (the Image at/around
the current usage referenced as the mic Icon on the input UI). Keep any other
style attributes in styles.micIcon, and merge them with the inline size (e.g.,
style={[styles.micIcon, {width: 24, height: 24}]} on the Image) so width/height
are applied inline while preserving other stylesheet rules.
In `@sources/components/ProfileEditForm.tsx`:
- Around line 41-88: MachinePreviewModal currently calls props.onClose in
multiple places (the close Pressable and inside MachineSelector's onSelect) but
the parent may not pass onClose; update the component to avoid calling an
undefined callback by either making onClose optional in MachinePreviewModalProps
and guarding every call (e.g., if (props.onClose) props.onClose()), or supply a
default no-op function for onClose when destructuring props in
MachinePreviewModal; ensure all usages (Pressable onPress, the onSelect handler,
and any other props.onClose calls) are updated accordingly so the modal no
longer throws when onClose is not provided.
In `@sources/components/SearchableListSelector.tsx`:
- Line 364: The ItemGroup fallback is using a hardcoded string; update the JSX
for ItemGroup (where config.allSectionTitle is used in SearchableListSelector)
to fallback to the localized string via t('common.all') instead of 'All'; ensure
the translation function t is imported/available in this component (or use the
existing useTranslation hook) and confirm the 'common.all' key exists in the
i18n files.
In `@sources/components/tools/knownTools.tsx`:
- Around line 664-691: The code in the title and extractSubtitle handlers
assumes opts.tool.input.toolCall.title is a string and calls indexOf/match
directly; add guards to verify typeof opts.tool.input?.toolCall?.title ===
'string' before parsing (in the title function and extractSubtitle function),
and if it is not a string fall back to returning t('tools.names.terminal') for
title and null for subtitle so non-string values from Gemini won't throw.
🧹 Nitpick comments (8)
sources/text/README.md (1)
100-102: Consider expanding the_types.tsdocumentation.The new section accurately describes the purpose of
_types.ts, but could be enhanced with a brief example showing what types are exported (e.g.,TranslationParams, key type unions) or why the separation improves the architecture. This would help developers understand when they might need to reference this file.📝 Example enhancement
### `_types.ts` -Contains the TypeScript types derived from the English translation structure. +Contains the TypeScript types derived from the English translation structure: +- `TranslationParams<K>` - Extracts parameter types for each key +- Translation key unions for type-safe access +- Utility types for the translation systemsources/app/(app)/new/pick/path.tsx (2)
24-25: Guard against undefined favorites to prevent runtime errors.If
useSettingMutable('favoriteDirectories')can returnundefined,PathSelectormay crash when reading favorites. Consider defaulting to an empty array.♻️ Proposed safe defaulting
- const [favoriteDirectories, setFavoriteDirectories] = useSettingMutable('favoriteDirectories'); + const [favoriteDirectories, setFavoriteDirectories] = useSettingMutable('favoriteDirectories'); + const safeFavoriteDirectories = favoriteDirectories ?? []; ... - favoriteDirectories={favoriteDirectories} - onChangeFavoriteDirectories={setFavoriteDirectories} + favoriteDirectories={safeFavoriteDirectories} + onChangeFavoriteDirectories={(next) => setFavoriteDirectories(next ?? [])}Also applies to: 176-177
79-90: Avoid react-navigationSET_PARAMS; prefer expo-router APIs.At Line 83–90,
navigation.dispatch({ type: 'SET_PARAMS' ... })uses a react-navigation action. Please verify expo-router supports this pattern and consider an expo-router-native alternative (e.g.,router.navigate/replacewith params or a shared store) to comply with the routing guideline. As per coding guidelines, ...sources/app/(app)/session/[id]/info.tsx (1)
10-86: Localize built‑in profile display names.
profileLabelreturnsbuiltIn.namedirectly; consider mapping built‑in profile IDs to translation keys so they remain localized, while keeping custom profile names as‑is. Based on learnings, please keep user‑visible strings behindt(...).sources/app/(app)/settings/profiles.tsx (1)
233-275: Inconsistent indentation in JSX blocks.The code has mixed indentation with tabs and spaces (e.g., lines 233-275 use inconsistent leading whitespace). Per coding guidelines, use 4 spaces for indentation consistently. This affects readability and may cause merge conflicts.
sources/components/ProfileEditForm.tsx (1)
154-159: ClampdefaultPermissionModeto supported options.If a profile carries
read-only/safe-yolofrom codex/gemini flows, this list won’t show any selection. Consider normalizing to the four supported modes here.♻️ Suggested refactor
- const [defaultPermissionMode, setDefaultPermissionMode] = React.useState<PermissionMode>( - (profile.defaultPermissionMode as PermissionMode) || 'default', - ); + const allowedPermissionModes = ['default', 'acceptEdits', 'plan', 'bypassPermissions'] as const; + const initialPermissionMode = allowedPermissionModes.includes(profile.defaultPermissionMode as any) + ? (profile.defaultPermissionMode as PermissionMode) + : 'default'; + const [defaultPermissionMode, setDefaultPermissionMode] = React.useState<PermissionMode>( + initialPermissionMode, + );sources/components/ItemGroup.tsx (1)
99-126: Memoize the context value to avoid unnecessary re-renders.
ItemGroupSelectionContext.Providergets a fresh object each render, so all consumers re-render even when the count is unchanged.♻️ Suggested change
const selectableItemCount = React.useMemo(() => { return countSelectableItems(children); }, [children]); + const selectionContextValue = React.useMemo( + () => ({ selectableItemCount }), + [selectableItemCount] + ); @@ - <ItemGroupSelectionContext.Provider value={{ selectableItemCount }}> + <ItemGroupSelectionContext.Provider value={selectionContextValue}> {withItemGroupDividers(children)} </ItemGroupSelectionContext.Provider>sources/components/EnvironmentVariablesList.tsx (1)
21-69: Consider centralizing the secret-name heuristic.
SECRET_NAME_REGEX+ template extraction are now used here; moving these to a shared env‑var util would reduce drift with other secret heuristics as they evolve.
| function MachinePreviewModal(props: MachinePreviewModalProps) { | ||
| const { theme } = useUnistyles(); | ||
| const styles = stylesheet; | ||
| const { height: windowHeight } = useWindowDimensions(); | ||
|
|
||
| const selectedMachine = React.useMemo(() => { | ||
| if (!props.selectedMachineId) return null; | ||
| return props.machines.find((m) => m.id === props.selectedMachineId) ?? null; | ||
| }, [props.machines, props.selectedMachineId]); | ||
|
|
||
| const favoriteMachines = React.useMemo(() => { | ||
| const byId = new Map(props.machines.map((m) => [m.id, m] as const)); | ||
| return props.favoriteMachineIds.map((id) => byId.get(id)).filter(Boolean) as Machine[]; | ||
| }, [props.favoriteMachineIds, props.machines]); | ||
|
|
||
| const maxHeight = Math.min(720, Math.max(420, Math.floor(windowHeight * 0.85))); | ||
|
|
||
| return ( | ||
| <View style={[styles.machinePreviewModalContainer, { height: maxHeight, maxHeight }]}> | ||
| <View style={styles.machinePreviewModalHeader}> | ||
| <Text style={styles.machinePreviewModalTitle}> | ||
| {t('profiles.previewMachine.title')} | ||
| </Text> | ||
|
|
||
| <Pressable | ||
| onPress={props.onClose} | ||
| hitSlop={{ top: 10, bottom: 10, left: 10, right: 10 }} | ||
| style={({ pressed }) => ({ opacity: pressed ? 0.7 : 1 })} | ||
| > | ||
| <Ionicons name="close" size={20} color={theme.colors.textSecondary} /> | ||
| </Pressable> | ||
| </View> | ||
|
|
||
| <View style={{ flex: 1 }}> | ||
| <MachineSelector | ||
| machines={props.machines} | ||
| selectedMachine={selectedMachine} | ||
| favoriteMachines={favoriteMachines} | ||
| showRecent={false} | ||
| showFavorites={favoriteMachines.length > 0} | ||
| showSearch | ||
| searchPlacement={favoriteMachines.length > 0 ? 'favorites' : 'all'} | ||
| onSelect={(machine) => { | ||
| props.onSelect(machine.id); | ||
| props.onClose(); | ||
| }} | ||
| onToggleFavorite={(machine) => props.onToggleFavorite(machine.id)} | ||
| /> |
There was a problem hiding this comment.
Crash risk: modal close callback not wired.
MachinePreviewModal calls onClose on selection and on the close button, but the modal opener doesn’t pass it, so this will throw and leave the modal stuck.
🐛 Proposed fix
function MachinePreviewModal(props: MachinePreviewModalProps) {
@@
- <Pressable
- onPress={props.onClose}
+ <Pressable
+ onPress={() => props.onClose?.()}
@@
- onSelect={(machine) => {
- props.onSelect(machine.id);
- props.onClose();
- }}
+ onSelect={(machine) => {
+ props.onSelect(machine.id);
+ props.onClose?.();
+ }}
@@
const showMachinePreviewPicker = React.useCallback(() => {
Modal.show({
component: MachinePreviewModal,
props: {
machines,
favoriteMachineIds: favoriteMachines,
selectedMachineId: previewMachineId,
onSelect: setPreviewMachineId,
onToggleFavorite: toggleFavoriteMachineId,
+ onClose: () => Modal.hide(),
},
});
}, [favoriteMachines, machines, previewMachineId, toggleFavoriteMachineId]);Also applies to: 128-139
🤖 Prompt for AI Agents
In `@sources/components/ProfileEditForm.tsx` around lines 41 - 88,
MachinePreviewModal currently calls props.onClose in multiple places (the close
Pressable and inside MachineSelector's onSelect) but the parent may not pass
onClose; update the component to avoid calling an undefined callback by either
making onClose optional in MachinePreviewModalProps and guarding every call
(e.g., if (props.onClose) props.onClose()), or supply a default no-op function
for onClose when destructuring props in MachinePreviewModal; ensure all usages
(Pressable onPress, the onSelect handler, and any other props.onClose calls) are
updated accordingly so the modal no longer throws when onClose is not provided.
| title: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | ||
| // Gemini sends nice title in toolCall.title | ||
| if (opts.tool.input?.toolCall?.title) { | ||
| // Title is like "rm file.txt [cwd /path] (description)" | ||
| // Extract just the command part before [ | ||
| const fullTitle = opts.tool.input.toolCall.title; | ||
| const bracketIdx = fullTitle.indexOf(' ['); | ||
| if (bracketIdx > 0) { | ||
| return fullTitle.substring(0, bracketIdx); | ||
| } | ||
| return fullTitle; | ||
| } | ||
| return t('tools.names.terminal'); | ||
| }, | ||
| icon: ICON_TERMINAL, | ||
| isMutable: true, | ||
| input: z.object({}).partial().loose(), | ||
| extractSubtitle: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | ||
| // Extract description from parentheses at the end | ||
| if (opts.tool.input?.toolCall?.title) { | ||
| const title = opts.tool.input.toolCall.title; | ||
| const parenMatch = title.match(/\(([^)]+)\)$/); | ||
| if (parenMatch) { | ||
| return parenMatch[1]; | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Add string guards for toolCall.title before parsing.
execute assumes toolCall.title is a string and calls indexOf/match; if Gemini sends a non-string, this will throw.
🐛 Proposed fix
- title: (opts: { metadata: Metadata | null, tool: ToolCall }) => {
- // Gemini sends nice title in toolCall.title
- if (opts.tool.input?.toolCall?.title) {
+ title: (opts: { metadata: Metadata | null, tool: ToolCall }) => {
+ const title = opts.tool.input?.toolCall?.title;
+ // Gemini sends nice title in toolCall.title
+ if (typeof title === 'string') {
// Title is like "rm file.txt [cwd /path] (description)"
// Extract just the command part before [
- const fullTitle = opts.tool.input.toolCall.title;
- const bracketIdx = fullTitle.indexOf(' [');
+ const bracketIdx = title.indexOf(' [');
if (bracketIdx > 0) {
- return fullTitle.substring(0, bracketIdx);
+ return title.substring(0, bracketIdx);
}
- return fullTitle;
+ return title;
}
return t('tools.names.terminal');
},
@@
- extractSubtitle: (opts: { metadata: Metadata | null, tool: ToolCall }) => {
+ extractSubtitle: (opts: { metadata: Metadata | null, tool: ToolCall }) => {
// Extract description from parentheses at the end
- if (opts.tool.input?.toolCall?.title) {
- const title = opts.tool.input.toolCall.title;
+ const title = opts.tool.input?.toolCall?.title;
+ if (typeof title === 'string') {
const parenMatch = title.match(/\(([^)]+)\)$/);
if (parenMatch) {
return parenMatch[1];
}
}
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.
| title: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | |
| // Gemini sends nice title in toolCall.title | |
| if (opts.tool.input?.toolCall?.title) { | |
| // Title is like "rm file.txt [cwd /path] (description)" | |
| // Extract just the command part before [ | |
| const fullTitle = opts.tool.input.toolCall.title; | |
| const bracketIdx = fullTitle.indexOf(' ['); | |
| if (bracketIdx > 0) { | |
| return fullTitle.substring(0, bracketIdx); | |
| } | |
| return fullTitle; | |
| } | |
| return t('tools.names.terminal'); | |
| }, | |
| icon: ICON_TERMINAL, | |
| isMutable: true, | |
| input: z.object({}).partial().loose(), | |
| extractSubtitle: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | |
| // Extract description from parentheses at the end | |
| if (opts.tool.input?.toolCall?.title) { | |
| const title = opts.tool.input.toolCall.title; | |
| const parenMatch = title.match(/\(([^)]+)\)$/); | |
| if (parenMatch) { | |
| return parenMatch[1]; | |
| } | |
| } | |
| return null; | |
| } | |
| title: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | |
| const title = opts.tool.input?.toolCall?.title; | |
| // Gemini sends nice title in toolCall.title | |
| if (typeof title === 'string') { | |
| // Title is like "rm file.txt [cwd /path] (description)" | |
| // Extract just the command part before [ | |
| const bracketIdx = title.indexOf(' ['); | |
| if (bracketIdx > 0) { | |
| return title.substring(0, bracketIdx); | |
| } | |
| return title; | |
| } | |
| return t('tools.names.terminal'); | |
| }, | |
| icon: ICON_TERMINAL, | |
| isMutable: true, | |
| input: z.object({}).partial().loose(), | |
| extractSubtitle: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | |
| // Extract description from parentheses at the end | |
| const title = opts.tool.input?.toolCall?.title; | |
| if (typeof title === 'string') { | |
| const parenMatch = title.match(/\(([^)]+)\)$/); | |
| if (parenMatch) { | |
| return parenMatch[1]; | |
| } | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In `@sources/components/tools/knownTools.tsx` around lines 664 - 691, The code in
the title and extractSubtitle handlers assumes opts.tool.input.toolCall.title is
a string and calls indexOf/match directly; add guards to verify typeof
opts.tool.input?.toolCall?.title === 'string' before parsing (in the title
function and extractSubtitle function), and if it is not a string fall back to
returning t('tools.names.terminal') for title and null for subtitle so
non-string values from Gemini won't throw.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@app.config.js`:
- Around line 7-20: The current computed constants name and bundleId can become
undefined when variant (derived from APP_ENV) is unmapped and no overrides
(nameOverride/bundleIdOverride) are provided; update the logic in the name and
bundleId expressions to enforce a safe fallback or explicit error: after
attempting the override and lookup by variant (using the existing variant lookup
map), if the result is falsy then either assign a sensible default string (e.g.,
a development-safe name and bundle id) or throw a clear error that includes the
APP_ENV/variant value so Expo config cannot proceed with undefined values;
ensure you update the same expressions that compute name and bundleId so the
fallback/error is used in place of undefined.
In `@sources/auth/tokenStorage.ts`:
- Around line 46-51: The web branch in setCredentials uses localStorage.setItem
without error handling; wrap the localStorage.setItem call in a try/catch (same
pattern as the native branch) so that any exceptions (QuotaExceededError,
SecurityError, etc.) are caught, log or handle the error as appropriate, and
return false on failure instead of letting errors propagate; reference
setCredentials, getAuthKey, Platform.OS and localStorage.setItem when making the
change to mirror the native code path's behavior.
- Around line 64-69: The web-path localStorage calls can throw and currently
crash removeCredentials(); wrap the localStorage.removeItem(key) call in
removeCredentials() in a try-catch and return false on error (log or swallow as
appropriate), and do the same for the corresponding setCredentials()
localStorage.setItem(...) call (both use getAuthKey() and the Platform.OS ===
'web' branch); ensure successful operations still return true and any caught
exceptions cause a false return so logout/setting credentials fails gracefully
instead of throwing.
- Around line 23-26: The web branch of getCredentials() is unsafe:
localStorage.getItem and JSON.parse can throw and getItem is called twice;
update the code in the function that uses getAuthKey()/getCredentials() to call
localStorage.getItem(key) once, wrap the retrieval and JSON.parse in a
try/catch, return null on any error (or if item is null), and log or ignore the
caught error as appropriate; ensure you still return the parsed AuthCredentials
when parse succeeds and null for any exception or missing value.
🧹 Nitpick comments (8)
sources/sync/persistence.ts (3)
18-29: Maintenance note: SESSION_MODEL_MODES requires manual sync with Session type.The
SessionModelModetype is correctly derived fromSession['modelMode'], but theSESSION_MODEL_MODESset values are hardcoded. If new values are added to the Session type, TypeScript won't flag missing entries in the set—only invalid ones. Consider adding a comment noting this coupling.
38-38: Type mismatch:modelModefield declaresModelModebut parsing only acceptsSessionModelMode.The interface declares
modelMode: ModelMode(which includes values like'opus','sonnet','adaptiveUsage', etc.), butloadNewSessionDraftvalidates againstisSessionModelModeand rejects non-session values. This means validModelModevalues would silently fall back to'default'.Consider aligning the type with what's actually supported:
♻️ Suggested fix
export interface NewSessionDraft { input: string; selectedMachineId: string | null; selectedPath: string | null; selectedProfileId: string | null; agentType: NewSessionAgentType; permissionMode: PermissionMode; - modelMode: ModelMode; + modelMode: NonNullable<Session['modelMode']>; sessionType: NewSessionSessionType; updatedAt: number; }
166-168: Inconsistent validation:permissionModeparsing lacks type guard unlikemodelMode.The
modelModeparsing now correctly usesisSessionModelModetype guard (lines 169-171), butpermissionModestill uses a simple string type check with an unsafe cast. Invalid permission mode strings from corrupted storage could propagate.Consider adding a similar type guard for consistency:
♻️ Suggested fix
+const SESSION_PERMISSION_MODES = new Set<PermissionMode>([ + 'default', 'acceptEdits', 'bypassPermissions', 'plan', 'read-only', 'safe-yolo', 'yolo', +]); + +function isPermissionMode(value: unknown): value is PermissionMode { + return typeof value === 'string' && SESSION_PERMISSION_MODES.has(value as PermissionMode); +}Then update the parsing:
- const permissionMode: PermissionMode = typeof parsed.permissionMode === 'string' - ? (parsed.permissionMode as PermissionMode) - : 'default'; + const permissionMode: PermissionMode = isPermissionMode(parsed.permissionMode) + ? parsed.permissionMode + : 'default';sources/sync/ops.ts (1)
298-311: Consider validating individualPreviewEnvValueentries.The shape validation checks
policyand thatvaluesis a plain object, but individual entries invaluesare cast directly toPreviewEnvValuewithout validation. If a daemon returns malformed entries (e.g., missingdisplayfield), consumers could encounter runtime errors.Given that you're already defensive about mixed daemon versions, consider either:
- Filtering/validating each entry before casting, or
- Documenting that consumers should handle potentially incomplete entries
This is a minor concern since the daemon is trusted, but worth noting for robustness.
sources/hooks/useEnvironmentVariables.ts (1)
143-145: Minor: Consider hoisting SECRET_NAME_REGEX outside the effect.The regex is recreated on every effect run. Moving it to module scope (like in
EnvironmentVariablesList.tsx) would be marginally more efficient.Suggested change
+const SECRET_NAME_REGEX = /TOKEN|KEY|SECRET|AUTH|PASS|PASSWORD|COOKIE/i; + export function useEnvironmentVariables( machineId: string | null, varNames: string[], options?: UseEnvironmentVariablesOptions ): UseEnvironmentVariablesResult { // ... const fetchVars = async () => { // ... - const SECRET_NAME_REGEX = /TOKEN|KEY|SECRET|AUTH|PASS|PASSWORD|COOKIE/i; const sensitiveHints = options?.sensitiveHints ?? {};sources/components/newSession/EnvironmentVariablesPreviewModal.tsx (3)
20-36: Consider extracting shared utilities to reduce duplication.
isSecretLike,extractVarRefsFromValue, andENV_VAR_TEMPLATE_REF_REGEXare duplicated fromEnvironmentVariablesList.tsx. Consider moving these to a shared utility file (e.g.,@/utils/envVarUtils.ts) to ensure consistent behavior and easier maintenance.
38-89: StyleSheet uses function mode butruntimeparameter is unused.The stylesheet callback receives
(theme, runtime)butruntimeis never used. This is harmless but consider removing it for clarity.Minor cleanup
-const stylesheet = StyleSheet.create((theme, runtime) => ({ +const stylesheet = StyleSheet.create((theme) => ({
260-271: Consider refactoring nested ternaries for readability.The
detailLabelcomputation uses 5-level nested ternaries. While functional, a switch statement or object lookup would improve readability.Alternative approach
const detailLabelMap: Record<DetailKind, string> = { fixed: t('profiles.environmentVariables.previewModal.detail.fixed'), machine: t('profiles.environmentVariables.previewModal.detail.machine'), checking: t('profiles.environmentVariables.previewModal.detail.checking'), fallback: t('profiles.environmentVariables.previewModal.detail.fallback'), missing: t('profiles.environmentVariables.previewModal.detail.missing'), }; const detailLabel = detailKind ? detailLabelMap[detailKind] : undefined;
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
sources/text/translations/pl.ts (1)
45-82: Localize remaining English strings incommon.Line 80 and Line 82 are still in English. Please translate them for consistency.
🌍 Suggested localization
- all: 'All', - clearSearch: 'Clear search', + all: 'Wszystkie', + clearSearch: 'Wyczyść wyszukiwanie',sources/text/translations/es.ts (1)
34-71: Localize remaining English strings incommon.Line 69 and Line 71 are still in English. Please translate them for consistency.
🌍 Suggested localization
- all: 'All', - clearSearch: 'Clear search', + all: 'Todas', + clearSearch: 'Borrar búsqueda',sources/app/(app)/new/pick/path.tsx (1)
22-48: Guard against undefinedrecentMachinePathsbefore iterating.
If the setting can benull/undefined(fresh installs or failed parse),.forEachwill throw. A safe fallback keeps the picker stable.🛠️ Suggested fix
- const recentMachinePaths = useSetting('recentMachinePaths'); + const recentMachinePaths = useSetting('recentMachinePaths') ?? [];sources/app/(app)/settings/profiles.tsx (1)
35-138: Add null coalescing fallback tofavoriteProfileIdsto prevent crashes.The setting may be undefined at runtime despite type definitions and defaults. This mirrors the pattern already established in
sources/app/(app)/new/pick/path.tsxforfavoriteDirectories.🛠️ Suggested fix
- const [favoriteProfileIds, setFavoriteProfileIds] = useSettingMutable('favoriteProfiles'); + const [favoriteProfileIdsRaw, setFavoriteProfileIds] = useSettingMutable('favoriteProfiles'); + const favoriteProfileIds = favoriteProfileIdsRaw ?? [];
🤖 Fix all issues with AI agents
In `@sources/app/`(app)/new/pick/path.tsx:
- Around line 80-92: The handleSelectPath function currently uses
navigation.dispatch({... type: 'SET_PARAMS' ...}) to pass the selected path
back; replace this react-navigation internals usage with expo-router param
handling by calling router.setParams({ path: pathToUse }) (or otherwise setting
the route params via expo-router) before navigating back so the parent
(new/index.tsx which reads path via useLocalSearchParams) receives and reacts to
the updated path; update references in handleSelectPath to import/use router
from expo-router and remove the navigation.dispatch block.
In `@sources/auth/tokenStorage.test.ts`:
- Around line 9-35: The test helper installLocalStorage replaces
globalThis.localStorage but never restores it, causing leakage across Vitest
suites; modify installLocalStorage to capture the previous descriptor (e.g.,
const prev = Object.getOwnPropertyDescriptor(globalThis, 'localStorage')) and
return it, then in the test suite's afterEach restore that descriptor (or delete
localStorage if prev is undefined) so globalThis.localStorage is reset; update
the describe('TokenStorage (web)') afterEach to call this restore logic (or have
installLocalStorage expose a restore function) to ensure isolation between
tests.
In `@sources/components/tools/knownTools.tsx`:
- Around line 562-592: The status enums in the tool descriptors for 'reasoning'
(result on the reasoning tool object) and 'think' (result on the think tool
object) currently allow only ['completed','in_progress','canceled'] and will
drop Gemini's "error" responses; update both z.enum([...]) declarations to
include 'error' (i.e., ['completed','in_progress','canceled','error']) so
parsing preserves error statuses, leaving the surrounding
z.object(...).partial().loose() structure unchanged.
In `@sources/hooks/useEnvironmentVariables.ts`:
- Around line 203-237: The fallback line parser is currently skipped when JSON
parsing fails because the code checks Object.keys(results).length === 0, but
results may already contain entries for excluded sensitive vars (safeVarNames),
causing all safe vars to remain null; introduce a boolean parsedJson (initialize
false) before the JSON extraction, set parsedJson = true only after successful
JSON.parse of jsonSlice (and after populating results from parsed), and replace
the fallback condition with if (!parsedJson) so the line parser runs whenever
JSON parsing didn't succeed regardless of existing keys in results; update
references to parsed/parsedJson in the block that parses JSON and the subsequent
fallback line parser that iterates stdout, ensuring safeVarNames and results
remain used as before.
In `@sources/text/translations/ca.ts`:
- Around line 68-71: Translate the remaining English strings in the Catalan
translations: replace the value for the key all with a Catalan equivalent (e.g.,
"Tots") and replace clearSearch with a Catalan phrase (e.g., "Esborra la cerca"
or "Neteja la cerca"); update these keys in the translations object (keys: all,
clearSearch) and also make the same replacements for the duplicate occurrences
mentioned around the other locations (the entries referenced as also applies to
978-979).
In `@sources/text/translations/es.ts`:
- Around line 974-1078: The copySuffix translation is still in English; update
the copySuffix entry to the Spanish string (e.g., '(Copiar)') so the
translations object uses Spanish everywhere — locate the copySuffix key in the
translations (copySuffix: '(Copy)') and replace the value with '(Copiar)'
ensuring string quoting and commas match surrounding entries.
In `@sources/text/translations/ja.ts`:
- Around line 60-63: The listed translation entries (noMatches, all, machine,
clearSearch) contain English values and must be replaced with Japanese
equivalents; update the values for the keys noMatches, all, machine, and
clearSearch in the translations object to their proper Japanese translations
(also check and update the duplicate/related entries around the second
occurrence referenced at the other location) so the UI shows localized text
instead of English.
In `@sources/text/translations/pl.ts`:
- Around line 997-1101: The copySuffix translation is still in English—update
the copySuffix entry (the copySuffix property inside the profile translations
object) from "(Copy)" to the correct Polish term (e.g., "(Kopia)") so the UI
shows a localized suffix; locate the copySuffix key near the profile actions and
replace its value with the Polish translation.
- Line 1: Translate the untranslated English strings by replacing the values for
common.all, common.clearSearch, and copySuffix in the Polish translation object
with appropriate Polish text (e.g., common.all -> "Wszystko", common.clearSearch
-> "Wyczyść wyszukiwanie", copySuffix -> "(Kopiuj)"), and ensure the same keys
are either translated or intentionally left in English across other locale files
(ru.ts, en.ts, es.ts) for cross-locale consistency; after updating the values
for common.all, common.clearSearch, and copySuffix in pl.ts (and reconciling
others if needed), run yarn typecheck to verify type safety.
In `@sources/text/translations/pt.ts`:
- Around line 68-71: Translate the English strings left in the Portuguese
translations by updating the keys in the translations object: replace all: 'All'
with the Portuguese equivalent (e.g., 'Todos' or context-appropriate plural),
and clearSearch: 'Clear search' with 'Limpar pesquisa' (or alternative local
phrasing); ensure the existing noMatches: 'Nenhuma correspondência' and machine:
'máquina' remain unchanged and apply the same fixes for the duplicate
occurrences referenced by the same keys elsewhere in the file (the other
instances of all and clearSearch).
In `@sources/text/translations/ru.ts`:
- Around line 79-82: The translation file contains untranslated English strings:
update the values for the keys "all" and "clearSearch" (and any other
occurrences noted around the same diff, e.g., lines referenced 1010-1011) in the
ru.ts translations object to their Russian equivalents while keeping the keys
intact; for example replace "all" => "Все" and "clearSearch" => "Очистить поиск"
so the UI shows localized text.
♻️ Duplicate comments (1)
sources/components/ProfileEditForm.tsx (1)
128-146: Past issue resolved: ModalonClosecallback is now properly wired.The wrapper pattern correctly receives
onClosefrom the Modal system and forwards it toMachinePreviewModal. This addresses the previously flagged crash risk whereonClosewas called without being provided.
🧹 Nitpick comments (12)
sources/text/README.md (1)
100-106: Clarify whereTranslationKeytype is defined.Line 105 states that
TranslationKey/TranslationParams<K>are "derived fromTranslations(inindex.ts)", but this section describes_types.ts. This could confuse readers about where these types are actually defined.Consider clarifying whether:
TranslationKeyis defined in_types.tsand re-exported fromindex.ts, ORTranslationKeyis actually derived/defined inindex.ts(separate from the other types in_types.ts)📝 Suggested clarification
### `_types.ts` Contains the TypeScript types derived from the English translation structure. This keeps the canonical translation object (`translations/en.ts`) separate from the type-level API: - `Translations` / `TranslationStructure` are derived from `en` and used to type-check other locales. -- `TranslationKey` / `TranslationParams<K>` are derived from `Translations` (in `index.ts`) to type `t(...)`. +- `TranslationKey` / `TranslationParams<K>` are derived from `Translations` in `index.ts` to type the `t(...)` function.Or if they're all in
_types.ts:This keeps the canonical translation object (`translations/en.ts`) separate from the type-level API: - `Translations` / `TranslationStructure` are derived from `en` and used to type-check other locales. -- `TranslationKey` / `TranslationParams<K>` are derived from `Translations` (in `index.ts`) to type `t(...)`. +- `TranslationKey` / `TranslationParams<K>` are also defined here and used in `index.ts` to type the `t(...)` function.sources/sync/persistence.ts (2)
18-32: Consider deriving SESSION_MODEL_MODES from the Session type to prevent sync drift.The comment on lines 20-22 acknowledges that
SESSION_MODEL_MODESmust stay in sync withSession['modelMode'], but TypeScript won't catch missing entries. SinceSession['modelMode']is defined as'default' | 'gemini-2.5-pro' | 'gemini-2.5-flash' | 'gemini-2.5-flash-lite'in storageTypes.ts, consider using a type-safe pattern:🔧 Optional: Type-safe set construction
-// NOTE: -// This set must stay in sync with the configurable Session model modes. -// TypeScript will catch invalid entries here, but it won't force adding new Session modes. -const SESSION_MODEL_MODES = new Set<SessionModelMode>([ - 'default', - 'gemini-2.5-pro', - 'gemini-2.5-flash', - 'gemini-2.5-flash-lite', -]); +// Exhaustive array ensures TypeScript catches missing modes at compile time +const SESSION_MODEL_MODES_ARRAY = [ + 'default', + 'gemini-2.5-pro', + 'gemini-2.5-flash', + 'gemini-2.5-flash-lite', +] as const satisfies readonly SessionModelMode[]; + +const SESSION_MODEL_MODES = new Set<SessionModelMode>(SESSION_MODEL_MODES_ARRAY);The
satisfiesclause ensures compile-time errors if the array doesn't matchSessionModelMode.
30-32: Minor: Unnecessary cast in type guard.The cast
as SessionModelModeis unnecessary sinceSet.prototype.has()acceptsunknown. This is cosmetic but could be cleaner:🔧 Suggested simplification
function isSessionModelMode(value: unknown): value is SessionModelMode { - return typeof value === 'string' && SESSION_MODEL_MODES.has(value as SessionModelMode); + return typeof value === 'string' && SESSION_MODEL_MODES.has(value); }sources/components/AgentInput.tsx (1)
648-652: Minor: Duplicated permission mode arrays for Codex and Gemini.The keyboard navigation and overlay rendering have identical mode arrays for
isCodexandisGemini. Consider consolidating using a shared constant to reduce duplication:🔧 Optional consolidation
- const modeOrder: PermissionMode[] = isCodex - ? ['default', 'read-only', 'safe-yolo', 'yolo'] - : isGemini - ? ['default', 'read-only', 'safe-yolo', 'yolo'] - : ['default', 'acceptEdits', 'plan', 'bypassPermissions']; + const modeOrder: PermissionMode[] = (isCodex || isGemini) + ? ['default', 'read-only', 'safe-yolo', 'yolo'] + : ['default', 'acceptEdits', 'plan', 'bypassPermissions'];Alternatively, import
CODEX_LIKE_PERMISSION_MODESandCLAUDE_PERMISSION_MODESfrom@/sync/permissionTypesfor a single source of truth.sources/components/ProfileEditForm.tsx (2)
451-486: Consider moving static button styles to the stylesheet.The action buttons section uses extensive inline styles. As per coding guidelines, prefer
StyleSheet.createfromreact-native-unistylesfor all styling. The dynamicopacityfor press state can remain inline, but static properties likeborderRadius,paddingVertical, andalignItemsshould be extracted.♻️ Suggested refactor
Add to stylesheet:
actionButtonsContainer: { paddingHorizontal: Platform.select({ ios: 16, default: 12 }), paddingTop: 12, }, actionButtonsRow: { flexDirection: 'row', gap: 12, }, actionButtonWrapper: { flex: 1, }, cancelButton: { backgroundColor: theme.colors.surface, borderRadius: 10, paddingVertical: 12, alignItems: 'center', }, saveButton: { backgroundColor: theme.colors.button.primary.background, borderRadius: 10, paddingVertical: 12, alignItems: 'center', },Then use in render:
-<View style={{ paddingHorizontal: Platform.select({ ios: 16, default: 12 }), paddingTop: 12 }}> - <View style={{ flexDirection: 'row', gap: 12 }}> +<View style={styles.actionButtonsContainer}> + <View style={styles.actionButtonsRow}>
520-523: Unused styles can be cleaned up.The
selectorContainerandmultilineInputstyles are defined but not referenced anywhere in the component. Consider removing them to reduce dead code.#!/bin/bash # Verify these styles are unused in the file echo "Checking for 'selectorContainer' usage:" rg -n 'selectorContainer' sources/components/ProfileEditForm.tsx echo "Checking for 'multilineInput' usage:" rg -n 'multilineInput' sources/components/ProfileEditForm.tsxAlso applies to: 553-576
sources/components/tools/knownTools.tsx (1)
869-882: UseresolvePathfor GeminiDifffilePathto keep titles consistent.If
filePathis absolute, the current basename uses the raw path. Consider normalizing withresolvePathfor consistent UI with other tools.♻️ Suggested refactor
- if (opts.tool.input?.filePath && typeof opts.tool.input.filePath === 'string') { - const basename = opts.tool.input.filePath.split('/').pop() || opts.tool.input.filePath; - return basename; - } + if (typeof opts.tool.input?.filePath === 'string') { + const path = resolvePath(opts.tool.input.filePath, opts.metadata); + const basename = path.split('/').pop() || path; + return basename; + }sources/components/SearchableListSelector.tsx (3)
76-76: Consider typingcontextmore strictly.Using
anybypasses TypeScript's type safety. If the context structure is known (e.g.,{ homeDir?: string }), consider defining a proper type or using a generic parameter to maintain type safety across consumers.
156-179: Consider extracting static text styles to the stylesheet.The Typography and platform-specific styles could be extracted to
stylesheet, with only the dynamiccolorapplied inline. This improves maintainability and aligns with the coding guideline to useStyleSheet.createfor styling.♻️ Suggested refactor
const stylesheet = StyleSheet.create((theme) => ({ showMoreTitle: { textAlign: 'center', color: theme.colors.textLink, }, + statusText: { + ...Typography.default('regular'), + fontSize: Platform.select({ ios: 17, default: 16 }), + letterSpacing: Platform.select({ ios: -0.41, default: 0.15 }), + }, }));Then in
renderStatus:- <Text - style={[ - Typography.default('regular'), - { - fontSize: Platform.select({ ios: 17, default: 16 }), - letterSpacing: Platform.select({ ios: -0.41, default: 0.15 }), - color: status.color, - }, - ]} - > + <Text style={[styles.statusText, { color: status.color }]}>
186-187: DuplicatedselectedColorcomputation.The
selectedColorcalculation appears in bothrenderFavoriteToggle(line 186) andrenderItem(line 221). Consider computing it once at the component level to avoid repetition.♻️ Suggested refactor
const showAll = config.showAll !== false; + const selectedColor = rt.themeName === 'dark' ? theme.colors.text : theme.colors.button.primary.background; // Search query is intentionally decoupled...Then remove the duplicate declarations from both helper functions.
Also applies to: 221-221
sources/auth/tokenStorage.ts (1)
34-39: Clear cache on JSON parse failure to avoid repeated fallbacks.If cached JSON is invalid, it will repeatedly fail and still keep the bad cache. Consider clearing cache/key when parsing fails.
♻️ Proposed tweak
if (credentialsCache && credentialsCacheKey === key) { try { return JSON.parse(credentialsCache) as AuthCredentials; } catch { + credentialsCache = null; + credentialsCacheKey = null; // Ignore cache parse errors, fall through to secure store read. } }sources/auth/tokenStorage.test.ts (1)
41-69: Prefer the@/path alias for dynamic imports.*Use the project alias to keep imports consistent and resilient to file moves.
♻️ Suggested change
- const { TokenStorage } = await import('./tokenStorage'); + const { TokenStorage } = await import('@/auth/tokenStorage'); @@ - const { TokenStorage } = await import('./tokenStorage'); + const { TokenStorage } = await import('@/auth/tokenStorage'); @@ - const { TokenStorage } = await import('./tokenStorage'); + const { TokenStorage } = await import('@/auth/tokenStorage'); @@ - const { TokenStorage } = await import('./tokenStorage'); + const { TokenStorage } = await import('@/auth/tokenStorage');As per coding guidelines, use the
@/*alias for TS/JS imports.
| const handleSelectPath = React.useCallback((pathOverride?: string) => { | ||
| const rawPath = typeof pathOverride === 'string' ? pathOverride : customPath; | ||
| const pathToUse = rawPath.trim() || machine?.metadata?.homeDir || '/home'; | ||
| // Pass path back via navigation params (main's pattern, received by new/index.tsx) | ||
| const state = navigation.getState(); | ||
| const previousRoute = state?.routes?.[state.index - 1]; | ||
| if (state && state.index > 0 && previousRoute) { | ||
| navigation.dispatch({ | ||
| ...CommonActions.setParams({ path: pathToUse }), | ||
| type: 'SET_PARAMS', | ||
| payload: { params: { path: pathToUse } }, | ||
| source: previousRoute.key, | ||
| } as never); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
expo-router pass params to previous screen best practice
💡 Result:
Short best-practices for returning data to the previous screen with expo-router
-
Prefer small, serializable params only (IDs / flags). Keep large or non-serializable data out of route params. [1]
-
Update the previous screen’s params when needed:
- Use navigation.navigate or router.push targeting the previous screen with params (React Navigation treats that as “go back + set params” when a matching screen exists).
- Or use navigation.setParams / router.getParent().dispatch(CommonActions.setParams(...)) for targeted param updates. [1][2]
-
For complex or non-serializable data, use shared state or callbacks:
- Global state (Context, Zustand, Redux, React Query cache) for objects or heavy data.
- If you need a callback pattern, implement it via shared context or a promise-based helper — do not rely on passing functions through route params (params must be serializable). [2][4]
-
Consider URL/search params carefully on web:
- Updating only query params can behave unexpectedly with browser history in expo-router; prefer path-based routes or manage history explicitly. [3]
Minimal examples
- Pass minimal result via navigate (works to return data): navigation.navigate('Home', { newItemId: id }); [1]
- Use global store for big payloads: setGlobalStore(item); router.back(); then read store from previous screen. [2]
Sources:
[1] React Navigation — Passing parameters & updating params.
[2] Expo Router docs (migration / navigation guidance).
[3] expo/router GitHub issue on query-params and browser history.
[4] StackOverflow discussion (practical notes: functions not serializable in params).
🏁 Script executed:
# First, let's look at the file structure to understand the navigation pattern
find sources/app -type f -name "*.tsx" | grep -E "(new|index)" | head -20Repository: leeroybrun/happy
Length of output: 666
🏁 Script executed:
# Let's examine the new/index.tsx file mentioned in the comment to understand how it receives params
find sources -type f -name "index.tsx" | grep "new/"Repository: leeroybrun/happy
Length of output: 91
🏁 Script executed:
cat -n sources/app/\(app\)/new/index.tsxRepository: leeroybrun/happy
Length of output: 50373
🏁 Script executed:
cat -n sources/app/\(app\)/new/pick/path.tsx | head -100Repository: leeroybrun/happy
Length of output: 5037
🏁 Script executed:
# Look for other uses of router.push or setParams patterns in new/ subdirectory
rg "router\.(push|setParams|back)" sources/app/\(app\)/new/ -A 2 -B 2Repository: leeroybrun/happy
Length of output: 5959
🏁 Script executed:
# Check if there are any shared state patterns (Context, stores) used for navigation in this directory
rg "(Context|useContext|zustand|Redux)" sources/app/\(app\)/new/pick/ -lRepository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Check profile.tsx to see if it has the same pattern
sed -n '80,100p' sources/app/\(app\)/new/pick/profile.tsxRepository: leeroybrun/happy
Length of output: 796
🏁 Script executed:
# Verify if router.setParams exists as an alternative in the codebase
rg "router\.setParams|navigation\.setParams" sources/app -A 2 -B 2 | head -30Repository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Check machine.tsx for the same pattern to see if this is systemic
rg "navigation\.dispatch.*SET_PARAMS" sources/app/\(app\)/new/pick/ -B 5 -A 5Repository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Search for how other parts of the app return data from nested screens
rg "useLocalSearchParams" sources/app/\(app\)/new/pick/ -A 3Repository: leeroybrun/happy
Length of output: 3590
Replace react-navigation dispatch with expo-router param handling.
The navigation.dispatch({ type: 'SET_PARAMS' }) pattern couples this screen to react-navigation internals and violates the codebase requirement to use expo-router APIs exclusively. Instead, update the parent screen's params using router.setParams() before navigating back, or use the parent's route params via useLocalSearchParams() as new/index.tsx already does (it reads the path param in its useEffect).
The parent screen (new/index.tsx line 512) already has the logic to handle pathParam updates. Pass the path back via route params that the parent naturally receives and reacts to, eliminating the need for direct react-navigation dispatch calls.
🤖 Prompt for AI Agents
In `@sources/app/`(app)/new/pick/path.tsx around lines 80 - 92, The
handleSelectPath function currently uses navigation.dispatch({... type:
'SET_PARAMS' ...}) to pass the selected path back; replace this react-navigation
internals usage with expo-router param handling by calling router.setParams({
path: pathToUse }) (or otherwise setting the route params via expo-router)
before navigating back so the parent (new/index.tsx which reads path via
useLocalSearchParams) receives and reacts to the updated path; update references
in handleSelectPath to import/use router from expo-router and remove the
navigation.dispatch block.
| function installLocalStorage() { | ||
| const store = new Map<string, string>(); | ||
| const getItem = vi.fn((key: string) => store.get(key) ?? null); | ||
| const setItem = vi.fn((key: string, value: string) => { | ||
| store.set(key, value); | ||
| }); | ||
| const removeItem = vi.fn((key: string) => { | ||
| store.delete(key); | ||
| }); | ||
|
|
||
| Object.defineProperty(globalThis, 'localStorage', { | ||
| value: { getItem, setItem, removeItem }, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| return { store, getItem, setItem, removeItem }; | ||
| } | ||
|
|
||
| describe('TokenStorage (web)', () => { | ||
| beforeEach(() => { | ||
| vi.resetModules(); | ||
| vi.spyOn(console, 'error').mockImplementation(() => {}); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| }); |
There was a problem hiding this comment.
Restore mocked localStorage to avoid cross-suite leakage.
installLocalStorage() replaces globalThis.localStorage, but it’s never restored. In shared Vitest workers, this can leak into other suites. Consider restoring the previous descriptor in afterEach.
🔧 Suggested cleanup
+let previousLocalStorage: PropertyDescriptor | undefined;
+
function installLocalStorage() {
const store = new Map<string, string>();
+ if (!previousLocalStorage) {
+ previousLocalStorage = Object.getOwnPropertyDescriptor(globalThis, 'localStorage');
+ }
const getItem = vi.fn((key: string) => store.get(key) ?? null);
const setItem = vi.fn((key: string, value: string) => {
store.set(key, value);
});
@@
return { store, getItem, setItem, removeItem };
}
@@
afterEach(() => {
+ if (previousLocalStorage) {
+ Object.defineProperty(globalThis, 'localStorage', previousLocalStorage);
+ previousLocalStorage = undefined;
+ } else {
+ // `@ts-expect-error` removing injected localStorage
+ delete (globalThis as any).localStorage;
+ }
vi.restoreAllMocks();
});📝 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.
| function installLocalStorage() { | |
| const store = new Map<string, string>(); | |
| const getItem = vi.fn((key: string) => store.get(key) ?? null); | |
| const setItem = vi.fn((key: string, value: string) => { | |
| store.set(key, value); | |
| }); | |
| const removeItem = vi.fn((key: string) => { | |
| store.delete(key); | |
| }); | |
| Object.defineProperty(globalThis, 'localStorage', { | |
| value: { getItem, setItem, removeItem }, | |
| configurable: true, | |
| }); | |
| return { store, getItem, setItem, removeItem }; | |
| } | |
| describe('TokenStorage (web)', () => { | |
| beforeEach(() => { | |
| vi.resetModules(); | |
| vi.spyOn(console, 'error').mockImplementation(() => {}); | |
| }); | |
| afterEach(() => { | |
| vi.restoreAllMocks(); | |
| }); | |
| let previousLocalStorage: PropertyDescriptor | undefined; | |
| function installLocalStorage() { | |
| const store = new Map<string, string>(); | |
| if (!previousLocalStorage) { | |
| previousLocalStorage = Object.getOwnPropertyDescriptor(globalThis, 'localStorage'); | |
| } | |
| const getItem = vi.fn((key: string) => store.get(key) ?? null); | |
| const setItem = vi.fn((key: string, value: string) => { | |
| store.set(key, value); | |
| }); | |
| const removeItem = vi.fn((key: string) => { | |
| store.delete(key); | |
| }); | |
| Object.defineProperty(globalThis, 'localStorage', { | |
| value: { getItem, setItem, removeItem }, | |
| configurable: true, | |
| }); | |
| return { store, getItem, setItem, removeItem }; | |
| } | |
| describe('TokenStorage (web)', () => { | |
| beforeEach(() => { | |
| vi.resetModules(); | |
| vi.spyOn(console, 'error').mockImplementation(() => {}); | |
| }); | |
| afterEach(() => { | |
| if (previousLocalStorage) { | |
| Object.defineProperty(globalThis, 'localStorage', previousLocalStorage); | |
| previousLocalStorage = undefined; | |
| } else { | |
| // `@ts-expect-error` removing injected localStorage | |
| delete (globalThis as any).localStorage; | |
| } | |
| vi.restoreAllMocks(); | |
| }); |
🤖 Prompt for AI Agents
In `@sources/auth/tokenStorage.test.ts` around lines 9 - 35, The test helper
installLocalStorage replaces globalThis.localStorage but never restores it,
causing leakage across Vitest suites; modify installLocalStorage to capture the
previous descriptor (e.g., const prev =
Object.getOwnPropertyDescriptor(globalThis, 'localStorage')) and return it, then
in the test suite's afterEach restore that descriptor (or delete localStorage if
prev is undefined) so globalThis.localStorage is reset; update the
describe('TokenStorage (web)') afterEach to call this restore logic (or have
installLocalStorage expose a restore function) to ensure isolation between
tests.
| result: z.object({ | ||
| content: z.string().describe('The reasoning content'), | ||
| status: z.enum(['completed', 'in_progress', 'canceled']).optional().describe('The status of the reasoning') | ||
| }).partial().loose(), | ||
| extractDescription: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | ||
| if (opts.tool.input?.title && typeof opts.tool.input.title === 'string') { | ||
| return opts.tool.input.title; | ||
| } | ||
| return t('tools.names.reasoning'); | ||
| } | ||
| }, | ||
| 'think': { | ||
| title: (opts: { metadata: Metadata | null, tool: ToolCall }) => { | ||
| // Use the title from input if provided | ||
| if (opts.tool.input?.title && typeof opts.tool.input.title === 'string') { | ||
| return opts.tool.input.title; | ||
| } | ||
| return t('tools.names.reasoning'); | ||
| }, | ||
| icon: ICON_REASONING, | ||
| minimal: true, | ||
| input: z.object({ | ||
| title: z.string().optional().describe('The title of the thinking'), | ||
| items: z.array(z.any()).optional().describe('Items to think about'), | ||
| locations: z.array(z.any()).optional().describe('Locations to consider') | ||
| }).partial().loose(), | ||
| result: z.object({ | ||
| content: z.string().optional().describe('The reasoning content'), | ||
| text: z.string().optional().describe('The reasoning text'), | ||
| status: z.enum(['completed', 'in_progress', 'canceled']).optional().describe('The status') | ||
| }).partial().loose(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n sources/components/tools/knownTools.tsx | sed -n '560,600p'Repository: leeroybrun/happy
Length of output: 2318
🏁 Script executed:
# Search for any references to Gemini status values or error handling
rg -i "gemini.*status|error.*status" -A 3 -B 3Repository: leeroybrun/happy
Length of output: 50373
🏁 Script executed:
# Look for related documentation or comments about status enums
rg "completed.*in_progress.*canceled" -A 2 -B 2Repository: leeroybrun/happy
Length of output: 1163
🏁 Script executed:
# Check if there's any existing 'error' status usage in the codebase
rg "status.*error|error.*status" sources/components/tools/knownTools.tsx -A 2 -B 2Repository: leeroybrun/happy
Length of output: 387
🏁 Script executed:
# Search for Gemini references and tool status handling
rg -i "gemini" sources/components/tools/knownTools.tsx -A 5 -B 5Repository: leeroybrun/happy
Length of output: 4830
🏁 Script executed:
# Look for where tool results are processed or validated
rg "tool.*result|result.*status" sources/components/tools/ -l | head -20Repository: leeroybrun/happy
Length of output: 223
🏁 Script executed:
# Search for any API or documentation about Gemini response formats
rg -i "google.*gemini|gemini.*api|thinking.*status" --type ts --type tsx -A 3 -B 3 | head -50Repository: leeroybrun/happy
Length of output: 87
🏁 Script executed:
# Look for the 'reasoning' and 'think' tool definitions to understand their context
ast-grep --pattern $'thinking: {
$$$
result: z.object({
$$$
})
$$$
}'Repository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Search for where 'error' status is used or handled in reasoning/thinking tools
rg "GeminiReasoning|'thinking'|'reasoning'" sources/components/tools/knownTools.tsx -A 20 | grep -i "error\|status"Repository: leeroybrun/happy
Length of output: 179
🏁 Script executed:
# Look for tool call error handling and result validation
rg "tool.result|tool.state.*error" sources/components/tools/ -A 3 -B 3 | head -50Repository: leeroybrun/happy
Length of output: 4107
🏁 Script executed:
# Check if there's any documentation or examples showing Gemini's possible status values
rg "completed.*in_progress" sources/ -B 5 -A 5 | grep -i "error\|gemini"Repository: leeroybrun/happy
Length of output: 216
🏁 Script executed:
# Search for actual Gemini API integration or response handling
rg -i "api.*gemini|gemini.*response|thinking.*response" sources/ -lRepository: leeroybrun/happy
Length of output: 42
🏁 Script executed:
# Verify the context around the 'think' tool definition to ensure completeness
sed -n '550,600p' sources/components/tools/knownTools.tsxRepository: leeroybrun/happy
Length of output: 2441
Add 'error' to reasoning and think tool status enums to handle Gemini error responses.
If Gemini returns an "error" status in the tool result, the current enum validation will reject it and silently drop the field. Adding 'error' to the enum keeps response parsing resilient to all possible Gemini status values.
Suggested fix
- status: z.enum(['completed', 'in_progress', 'canceled']).optional().describe('The status of the reasoning')
+ status: z.enum(['completed', 'in_progress', 'canceled', 'error']).optional().describe('The status of the reasoning')
...
- status: z.enum(['completed', 'in_progress', 'canceled']).optional().describe('The status')
+ status: z.enum(['completed', 'in_progress', 'canceled', 'error']).optional().describe('The status')🤖 Prompt for AI Agents
In `@sources/components/tools/knownTools.tsx` around lines 562 - 592, The status
enums in the tool descriptors for 'reasoning' (result on the reasoning tool
object) and 'think' (result on the think tool object) currently allow only
['completed','in_progress','canceled'] and will drop Gemini's "error" responses;
update both z.enum([...]) declarations to include 'error' (i.e.,
['completed','in_progress','canceled','error']) so parsing preserves error
statuses, leaving the surrounding z.object(...).partial().loose() structure
unchanged.
| if (result.success && result.exitCode === 0) { | ||
| // Parse output: "VAR1=value1\nVAR2=value2\nVAR3=" | ||
| const lines = result.stdout.trim().split('\n'); | ||
| lines.forEach(line => { | ||
| const equalsIndex = line.indexOf('='); | ||
| if (equalsIndex !== -1) { | ||
| const name = line.substring(0, equalsIndex); | ||
| const value = line.substring(equalsIndex + 1); | ||
| results[name] = value || null; // Empty string → null (not set) | ||
| const stdout = result.stdout; | ||
|
|
||
| // JSON protocol: {"VAR":"value","MISSING":null} | ||
| // Be resilient to any stray output (log lines, warnings) by extracting the last JSON object. | ||
| const trimmed = stdout.trim(); | ||
| const firstBrace = trimmed.indexOf('{'); | ||
| const lastBrace = trimmed.lastIndexOf('}'); | ||
| if (firstBrace !== -1 && lastBrace !== -1 && lastBrace > firstBrace) { | ||
| const jsonSlice = trimmed.slice(firstBrace, lastBrace + 1); | ||
| try { | ||
| const parsed = JSON.parse(jsonSlice) as Record<string, string | null>; | ||
| safeVarNames.forEach((name) => { | ||
| results[name] = Object.prototype.hasOwnProperty.call(parsed, name) ? parsed[name] : null; | ||
| }); | ||
| } catch { | ||
| // Fall through to line parser if JSON is malformed. | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // Fallback line parser: "VAR=value" or "VAR=__HAPPY_UNSET__" | ||
| if (Object.keys(results).length === 0) { | ||
| // Do not trim each line: it can corrupt values with meaningful whitespace. | ||
| const lines = stdout.split(/\r?\n/).filter((l) => l.length > 0); | ||
| lines.forEach((line) => { | ||
| // Ignore unrelated output (warnings, prompts, etc). | ||
| if (!/^[A-Z_][A-Z0-9_]*=/.test(line)) return; | ||
| const equalsIndex = line.indexOf('='); | ||
| if (equalsIndex !== -1) { | ||
| const name = line.substring(0, equalsIndex); | ||
| const value = line.substring(equalsIndex + 1); | ||
| results[name] = value === '__HAPPY_UNSET__' ? null : value; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fallback line parser can be skipped when JSON fails and sensitive vars are excluded.
If any sensitive vars are excluded, results is non-empty before parsing. When JSON parsing fails (or node is missing), the line parser is skipped and all safe var values end up null. Use an explicit parsedJson flag instead of Object.keys(results).length === 0.
🐛 Proposed fix
const stdout = result.stdout;
// JSON protocol: {"VAR":"value","MISSING":null}
// Be resilient to any stray output (log lines, warnings) by extracting the last JSON object.
- const trimmed = stdout.trim();
+ const trimmed = stdout.trim();
+ let parsedJson = false;
const firstBrace = trimmed.indexOf('{');
const lastBrace = trimmed.lastIndexOf('}');
if (firstBrace !== -1 && lastBrace !== -1 && lastBrace > firstBrace) {
const jsonSlice = trimmed.slice(firstBrace, lastBrace + 1);
try {
const parsed = JSON.parse(jsonSlice) as Record<string, string | null>;
safeVarNames.forEach((name) => {
results[name] = Object.prototype.hasOwnProperty.call(parsed, name) ? parsed[name] : null;
});
+ parsedJson = true;
} catch {
// Fall through to line parser if JSON is malformed.
}
}
// Fallback line parser: "VAR=value" or "VAR=__HAPPY_UNSET__"
- if (Object.keys(results).length === 0) {
+ if (!parsedJson) {
// Do not trim each line: it can corrupt values with meaningful whitespace.
const lines = stdout.split(/\r?\n/).filter((l) => l.length > 0);
lines.forEach((line) => {📝 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 (result.success && result.exitCode === 0) { | |
| // Parse output: "VAR1=value1\nVAR2=value2\nVAR3=" | |
| const lines = result.stdout.trim().split('\n'); | |
| lines.forEach(line => { | |
| const equalsIndex = line.indexOf('='); | |
| if (equalsIndex !== -1) { | |
| const name = line.substring(0, equalsIndex); | |
| const value = line.substring(equalsIndex + 1); | |
| results[name] = value || null; // Empty string → null (not set) | |
| const stdout = result.stdout; | |
| // JSON protocol: {"VAR":"value","MISSING":null} | |
| // Be resilient to any stray output (log lines, warnings) by extracting the last JSON object. | |
| const trimmed = stdout.trim(); | |
| const firstBrace = trimmed.indexOf('{'); | |
| const lastBrace = trimmed.lastIndexOf('}'); | |
| if (firstBrace !== -1 && lastBrace !== -1 && lastBrace > firstBrace) { | |
| const jsonSlice = trimmed.slice(firstBrace, lastBrace + 1); | |
| try { | |
| const parsed = JSON.parse(jsonSlice) as Record<string, string | null>; | |
| safeVarNames.forEach((name) => { | |
| results[name] = Object.prototype.hasOwnProperty.call(parsed, name) ? parsed[name] : null; | |
| }); | |
| } catch { | |
| // Fall through to line parser if JSON is malformed. | |
| } | |
| }); | |
| } | |
| // Fallback line parser: "VAR=value" or "VAR=__HAPPY_UNSET__" | |
| if (Object.keys(results).length === 0) { | |
| // Do not trim each line: it can corrupt values with meaningful whitespace. | |
| const lines = stdout.split(/\r?\n/).filter((l) => l.length > 0); | |
| lines.forEach((line) => { | |
| // Ignore unrelated output (warnings, prompts, etc). | |
| if (!/^[A-Z_][A-Z0-9_]*=/.test(line)) return; | |
| const equalsIndex = line.indexOf('='); | |
| if (equalsIndex !== -1) { | |
| const name = line.substring(0, equalsIndex); | |
| const value = line.substring(equalsIndex + 1); | |
| results[name] = value === '__HAPPY_UNSET__' ? null : value; | |
| } | |
| }); | |
| } | |
| if (result.success && result.exitCode === 0) { | |
| const stdout = result.stdout; | |
| // JSON protocol: {"VAR":"value","MISSING":null} | |
| // Be resilient to any stray output (log lines, warnings) by extracting the last JSON object. | |
| const trimmed = stdout.trim(); | |
| let parsedJson = false; | |
| const firstBrace = trimmed.indexOf('{'); | |
| const lastBrace = trimmed.lastIndexOf('}'); | |
| if (firstBrace !== -1 && lastBrace !== -1 && lastBrace > firstBrace) { | |
| const jsonSlice = trimmed.slice(firstBrace, lastBrace + 1); | |
| try { | |
| const parsed = JSON.parse(jsonSlice) as Record<string, string | null>; | |
| safeVarNames.forEach((name) => { | |
| results[name] = Object.prototype.hasOwnProperty.call(parsed, name) ? parsed[name] : null; | |
| }); | |
| parsedJson = true; | |
| } catch { | |
| // Fall through to line parser if JSON is malformed. | |
| } | |
| } | |
| // Fallback line parser: "VAR=value" or "VAR=__HAPPY_UNSET__" | |
| if (!parsedJson) { | |
| // Do not trim each line: it can corrupt values with meaningful whitespace. | |
| const lines = stdout.split(/\r?\n/).filter((l) => l.length > 0); | |
| lines.forEach((line) => { | |
| // Ignore unrelated output (warnings, prompts, etc). | |
| if (!/^[A-Z_][A-Z0-9_]*=/.test(line)) return; | |
| const equalsIndex = line.indexOf('='); | |
| if (equalsIndex !== -1) { | |
| const name = line.substring(0, equalsIndex); | |
| const value = line.substring(equalsIndex + 1); | |
| results[name] = value === '__HAPPY_UNSET__' ? null : value; | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@sources/hooks/useEnvironmentVariables.ts` around lines 203 - 237, The
fallback line parser is currently skipped when JSON parsing fails because the
code checks Object.keys(results).length === 0, but results may already contain
entries for excluded sensitive vars (safeVarNames), causing all safe vars to
remain null; introduce a boolean parsedJson (initialize false) before the JSON
extraction, set parsedJson = true only after successful JSON.parse of jsonSlice
(and after populating results from parsed), and replace the fallback condition
with if (!parsedJson) so the line parser runs whenever JSON parsing didn't
succeed regardless of existing keys in results; update references to
parsed/parsedJson in the block that parses JSON and the subsequent fallback line
parser that iterates stdout, ensuring safeVarNames and results remain used as
before.
| noMatches: 'Sense coincidències', | ||
| all: 'All', | ||
| machine: 'màquina', | ||
| clearSearch: 'Clear search', |
There was a problem hiding this comment.
Localize newly added strings left in English.
A few new UI strings are still English and will appear untranslated in Catalan.
📝 Suggested localization
@@
- all: 'All',
- clearSearch: 'Clear search',
+ all: 'Tots',
+ clearSearch: 'Neteja la cerca',
@@
- copySuffix: '(Copy)',
+ copySuffix: '(Còpia)',Also applies to: 978-979
🤖 Prompt for AI Agents
In `@sources/text/translations/ca.ts` around lines 68 - 71, Translate the
remaining English strings in the Catalan translations: replace the value for the
key all with a Catalan equivalent (e.g., "Tots") and replace clearSearch with a
Catalan phrase (e.g., "Esborra la cerca" or "Neteja la cerca"); update these
keys in the translations object (keys: all, clearSearch) and also make the same
replacements for the duplicate occurrences mentioned around the other locations
(the entries referenced as also applies to 978-979).
| noMatches: '一致するものがありません', | ||
| all: 'All', | ||
| machine: 'マシン', | ||
| clearSearch: 'Clear search', |
There was a problem hiding this comment.
Localize newly added strings left in English.
A few new UI strings are still English and will appear untranslated in Japanese.
📝 Suggested localization
@@
- all: 'All',
- clearSearch: 'Clear search',
+ all: 'すべて',
+ clearSearch: '検索をクリア',
@@
- copySuffix: '(Copy)',
+ copySuffix: '(コピー)',Also applies to: 112-113
🤖 Prompt for AI Agents
In `@sources/text/translations/ja.ts` around lines 60 - 63, The listed translation
entries (noMatches, all, machine, clearSearch) contain English values and must
be replaced with Japanese equivalents; update the values for the keys noMatches,
all, machine, and clearSearch in the translations object to their proper
Japanese translations (also check and update the duplicate/related entries
around the second occurrence referenced at the other location) so the UI shows
localized text instead of English.
| @@ -1,4 +1,4 @@ | |||
| import type { TranslationStructure } from '../_default'; | |||
| import type { TranslationStructure } from '../_types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# List translation files
ls -la sources/text/translations/Repository: leeroybrun/happy
Length of output: 683
🏁 Script executed:
# Check the pl.ts file to see actual changes
wc -l sources/text/translations/pl.ts
head -50 sources/text/translations/pl.tsRepository: leeroybrun/happy
Length of output: 1527
🏁 Script executed:
# Get file sizes for all locale files
wc -l sources/text/translations/{en,ru,pl,es}.ts 2>/dev/null || echo "Some files may be missing"Repository: leeroybrun/happy
Length of output: 228
🏁 Script executed:
# Search for the specific keys mentioned in the review comment
echo "=== Searching for mentioned keys ==="
rg -n "(clearSearch|machinePicker|pathPicker|geminiModel|environmentVariables|copySuffix)" sources/text/translations/Repository: leeroybrun/happy
Length of output: 7130
🏁 Script executed:
# Check for English strings (potential leftovers) in pl.ts
echo "=== Checking pl.ts for potential English text ==="
cat sources/text/translations/pl.ts | grep -E '(all|clearSearch|copySuffix)' | head -20Repository: leeroybrun/happy
Length of output: 1412
🏁 Script executed:
# Verify if these English strings are actually translations or placeholders
# Check en.ts for what the English versions should be
grep -n "all:" sources/text/translations/en.ts | head -5
grep -n "clearSearch:" sources/text/translations/en.ts | head -5
grep -n "copySuffix:" sources/text/translations/en.ts | head -5Repository: leeroybrun/happy
Length of output: 157
🏁 Script executed:
# Spot-check if these keys should have been translated
# by looking at the exact context in pl.ts vs en.ts
echo "=== pl.ts context for 'all' ==="
sed -n '70,90p' sources/text/translations/pl.ts
echo ""
echo "=== en.ts context for 'all' ==="
sed -n '70,90p' sources/text/translations/en.tsRepository: leeroybrun/happy
Length of output: 1285
🏁 Script executed:
# Check if these untranslated strings are the same in ru.ts as a comparison
echo "=== ru.ts lines for 'all', 'clearSearch' ==="
sed -n '70,90p' sources/text/translations/ru.tsRepository: leeroybrun/happy
Length of output: 783
🏁 Script executed:
# Check git history to see if these are recent additions or long-standing
git log --oneline -n 5 sources/text/translations/pl.tsRepository: leeroybrun/happy
Length of output: 98
Fix untranslated English strings and confirm cross-locale consistency.
The untranslated English strings in common.all ('All'), common.clearSearch ('Clear search'), and copySuffix ('(Copy)') are present in pl.ts but also appear untranslated in ru.ts and other locale files. These should be translated to Polish or verified as intentionally untranslated across all locales. Locale key structure parity is intact across en.ts, ru.ts, pl.ts, and es.ts for the newly added keys (machinePicker, pathPicker, geminiModel, environmentVariables). Run yarn typecheck to ensure type safety after completing translation updates.
🤖 Prompt for AI Agents
In `@sources/text/translations/pl.ts` at line 1, Translate the untranslated
English strings by replacing the values for common.all, common.clearSearch, and
copySuffix in the Polish translation object with appropriate Polish text (e.g.,
common.all -> "Wszystko", common.clearSearch -> "Wyczyść wyszukiwanie",
copySuffix -> "(Kopiuj)"), and ensure the same keys are either translated or
intentionally left in English across other locale files (ru.ts, en.ts, es.ts)
for cross-locale consistency; after updating the values for common.all,
common.clearSearch, and copySuffix in pl.ts (and reconciling others if needed),
run yarn typecheck to verify type safety.
| builtIn: 'Wbudowane', | ||
| groups: { | ||
| favorites: 'Ulubione', | ||
| custom: 'Twoje profile', | ||
| builtIn: 'Profile wbudowane', | ||
| }, | ||
| actions: { | ||
| viewEnvironmentVariables: 'Zmienne środowiskowe', | ||
| addToFavorites: 'Dodaj do ulubionych', | ||
| removeFromFavorites: 'Usuń z ulubionych', | ||
| editProfile: 'Edytuj profil', | ||
| duplicateProfile: 'Duplikuj profil', | ||
| deleteProfile: 'Usuń profil', | ||
| }, | ||
| copySuffix: '(Copy)', | ||
| duplicateName: 'Profil o tej nazwie już istnieje', | ||
| setupInstructions: { | ||
| title: 'Instrukcje konfiguracji', | ||
| viewOfficialGuide: 'Zobacz oficjalny przewodnik konfiguracji', | ||
| }, | ||
| defaultSessionType: 'Domyślny typ sesji', | ||
| defaultPermissionMode: { | ||
| title: 'Domyślny tryb uprawnień', | ||
| descriptions: { | ||
| default: 'Pytaj o uprawnienia', | ||
| acceptEdits: 'Automatycznie zatwierdzaj edycje', | ||
| plan: 'Zaplanuj przed wykonaniem', | ||
| bypassPermissions: 'Pomiń wszystkie uprawnienia', | ||
| }, | ||
| }, | ||
| aiBackend: { | ||
| title: 'Backend AI', | ||
| selectAtLeastOneError: 'Wybierz co najmniej jeden backend AI.', | ||
| claudeSubtitle: 'CLI Claude', | ||
| codexSubtitle: 'CLI Codex', | ||
| geminiSubtitleExperimental: 'CLI Gemini (eksperymentalne)', | ||
| }, | ||
| tmux: { | ||
| title: 'Tmux', | ||
| spawnSessionsTitle: 'Uruchamiaj sesje w Tmux', | ||
| spawnSessionsEnabledSubtitle: 'Sesje uruchamiają się w nowych oknach tmux.', | ||
| spawnSessionsDisabledSubtitle: 'Sesje uruchamiają się w zwykłej powłoce (bez integracji z tmux)', | ||
| sessionNamePlaceholder: 'Puste = bieżąca/najnowsza sesja', | ||
| tempDirPlaceholder: '/tmp (opcjonalne)', | ||
| }, | ||
| previewMachine: { | ||
| title: 'Podgląd maszyny', | ||
| selectMachine: 'Wybierz maszynę', | ||
| resolveSubtitle: 'Rozwiąż zmienne środowiskowe maszyny dla tego profilu.', | ||
| selectSubtitle: 'Wybierz maszynę, aby podejrzeć rozwiązane wartości.', | ||
| }, | ||
| environmentVariables: { | ||
| title: 'Zmienne środowiskowe', | ||
| addVariable: 'Dodaj zmienną', | ||
| namePlaceholder: 'Nazwa zmiennej (np. MY_CUSTOM_VAR)', | ||
| valuePlaceholder: 'Wartość (np. my-value lub ${MY_VAR})', | ||
| validation: { | ||
| nameRequired: 'Wprowadź nazwę zmiennej.', | ||
| invalidNameFormat: 'Nazwy zmiennych muszą zawierać wielkie litery, cyfry i podkreślenia oraz nie mogą zaczynać się od cyfry.', | ||
| duplicateName: 'Taka zmienna już istnieje.', | ||
| }, | ||
| card: { | ||
| valueLabel: 'Wartość:', | ||
| fallbackValueLabel: 'Wartość fallback:', | ||
| valueInputPlaceholder: 'Wartość', | ||
| defaultValueInputPlaceholder: 'Wartość domyślna', | ||
| secretNotRetrieved: 'Wartość sekretna - nie jest pobierana ze względów bezpieczeństwa', | ||
| overridingDefault: ({ expectedValue }: { expectedValue: string }) => | ||
| `Nadpisywanie udokumentowanej wartości domyślnej: ${expectedValue}`, | ||
| useMachineEnvToggle: 'Użyj wartości ze środowiska maszyny', | ||
| resolvedOnSessionStart: 'Rozwiązywane podczas uruchamiania sesji na wybranej maszynie.', | ||
| sourceVariableLabel: 'Zmienna źródłowa', | ||
| sourceVariablePlaceholder: 'Nazwa zmiennej źródłowej (np. Z_AI_MODEL)', | ||
| checkingMachine: ({ machine }: { machine: string }) => `Sprawdzanie ${machine}...`, | ||
| emptyOnMachine: ({ machine }: { machine: string }) => `Pusto na ${machine}`, | ||
| emptyOnMachineUsingFallback: ({ machine }: { machine: string }) => `Pusto na ${machine} (używam fallback)`, | ||
| notFoundOnMachine: ({ machine }: { machine: string }) => `Nie znaleziono na ${machine}`, | ||
| notFoundOnMachineUsingFallback: ({ machine }: { machine: string }) => `Nie znaleziono na ${machine} (używam fallback)`, | ||
| valueFoundOnMachine: ({ machine }: { machine: string }) => `Znaleziono wartość na ${machine}`, | ||
| differsFromDocumented: ({ expectedValue }: { expectedValue: string }) => | ||
| `Różni się od udokumentowanej wartości: ${expectedValue}`, | ||
| }, | ||
| preview: { | ||
| secretValueHidden: ({ value }: { value: string }) => `${value} - ukryte ze względów bezpieczeństwa`, | ||
| hiddenValue: '***ukryte***', | ||
| emptyValue: '(puste)', | ||
| sessionWillReceive: ({ name, value }: { name: string; value: string }) => | ||
| `Sesja otrzyma: ${name} = ${value}`, | ||
| }, | ||
| previewModal: { | ||
| titleWithProfile: ({ profileName }: { profileName: string }) => `Zmienne środowiskowe · ${profileName}`, | ||
| descriptionPrefix: 'Te zmienne środowiskowe są wysyłane podczas uruchamiania sesji. Wartości są rozwiązywane przez daemon na', | ||
| descriptionFallbackMachine: 'wybranej maszynie', | ||
| descriptionSuffix: '.', | ||
| emptyMessage: 'Dla tego profilu nie ustawiono zmiennych środowiskowych.', | ||
| checkingSuffix: '(sprawdzanie…)', | ||
| detail: { | ||
| fixed: 'Stała', | ||
| machine: 'Maszyna', | ||
| checking: 'Sprawdzanie', | ||
| fallback: 'Fallback', | ||
| missing: 'Brak', | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Translate copySuffix.
Line 1011 still uses English.
🌍 Suggested localization
- copySuffix: '(Copy)',
+ copySuffix: '(Kopia)',📝 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.
| builtIn: 'Wbudowane', | |
| groups: { | |
| favorites: 'Ulubione', | |
| custom: 'Twoje profile', | |
| builtIn: 'Profile wbudowane', | |
| }, | |
| actions: { | |
| viewEnvironmentVariables: 'Zmienne środowiskowe', | |
| addToFavorites: 'Dodaj do ulubionych', | |
| removeFromFavorites: 'Usuń z ulubionych', | |
| editProfile: 'Edytuj profil', | |
| duplicateProfile: 'Duplikuj profil', | |
| deleteProfile: 'Usuń profil', | |
| }, | |
| copySuffix: '(Copy)', | |
| duplicateName: 'Profil o tej nazwie już istnieje', | |
| setupInstructions: { | |
| title: 'Instrukcje konfiguracji', | |
| viewOfficialGuide: 'Zobacz oficjalny przewodnik konfiguracji', | |
| }, | |
| defaultSessionType: 'Domyślny typ sesji', | |
| defaultPermissionMode: { | |
| title: 'Domyślny tryb uprawnień', | |
| descriptions: { | |
| default: 'Pytaj o uprawnienia', | |
| acceptEdits: 'Automatycznie zatwierdzaj edycje', | |
| plan: 'Zaplanuj przed wykonaniem', | |
| bypassPermissions: 'Pomiń wszystkie uprawnienia', | |
| }, | |
| }, | |
| aiBackend: { | |
| title: 'Backend AI', | |
| selectAtLeastOneError: 'Wybierz co najmniej jeden backend AI.', | |
| claudeSubtitle: 'CLI Claude', | |
| codexSubtitle: 'CLI Codex', | |
| geminiSubtitleExperimental: 'CLI Gemini (eksperymentalne)', | |
| }, | |
| tmux: { | |
| title: 'Tmux', | |
| spawnSessionsTitle: 'Uruchamiaj sesje w Tmux', | |
| spawnSessionsEnabledSubtitle: 'Sesje uruchamiają się w nowych oknach tmux.', | |
| spawnSessionsDisabledSubtitle: 'Sesje uruchamiają się w zwykłej powłoce (bez integracji z tmux)', | |
| sessionNamePlaceholder: 'Puste = bieżąca/najnowsza sesja', | |
| tempDirPlaceholder: '/tmp (opcjonalne)', | |
| }, | |
| previewMachine: { | |
| title: 'Podgląd maszyny', | |
| selectMachine: 'Wybierz maszynę', | |
| resolveSubtitle: 'Rozwiąż zmienne środowiskowe maszyny dla tego profilu.', | |
| selectSubtitle: 'Wybierz maszynę, aby podejrzeć rozwiązane wartości.', | |
| }, | |
| environmentVariables: { | |
| title: 'Zmienne środowiskowe', | |
| addVariable: 'Dodaj zmienną', | |
| namePlaceholder: 'Nazwa zmiennej (np. MY_CUSTOM_VAR)', | |
| valuePlaceholder: 'Wartość (np. my-value lub ${MY_VAR})', | |
| validation: { | |
| nameRequired: 'Wprowadź nazwę zmiennej.', | |
| invalidNameFormat: 'Nazwy zmiennych muszą zawierać wielkie litery, cyfry i podkreślenia oraz nie mogą zaczynać się od cyfry.', | |
| duplicateName: 'Taka zmienna już istnieje.', | |
| }, | |
| card: { | |
| valueLabel: 'Wartość:', | |
| fallbackValueLabel: 'Wartość fallback:', | |
| valueInputPlaceholder: 'Wartość', | |
| defaultValueInputPlaceholder: 'Wartość domyślna', | |
| secretNotRetrieved: 'Wartość sekretna - nie jest pobierana ze względów bezpieczeństwa', | |
| overridingDefault: ({ expectedValue }: { expectedValue: string }) => | |
| `Nadpisywanie udokumentowanej wartości domyślnej: ${expectedValue}`, | |
| useMachineEnvToggle: 'Użyj wartości ze środowiska maszyny', | |
| resolvedOnSessionStart: 'Rozwiązywane podczas uruchamiania sesji na wybranej maszynie.', | |
| sourceVariableLabel: 'Zmienna źródłowa', | |
| sourceVariablePlaceholder: 'Nazwa zmiennej źródłowej (np. Z_AI_MODEL)', | |
| checkingMachine: ({ machine }: { machine: string }) => `Sprawdzanie ${machine}...`, | |
| emptyOnMachine: ({ machine }: { machine: string }) => `Pusto na ${machine}`, | |
| emptyOnMachineUsingFallback: ({ machine }: { machine: string }) => `Pusto na ${machine} (używam fallback)`, | |
| notFoundOnMachine: ({ machine }: { machine: string }) => `Nie znaleziono na ${machine}`, | |
| notFoundOnMachineUsingFallback: ({ machine }: { machine: string }) => `Nie znaleziono na ${machine} (używam fallback)`, | |
| valueFoundOnMachine: ({ machine }: { machine: string }) => `Znaleziono wartość na ${machine}`, | |
| differsFromDocumented: ({ expectedValue }: { expectedValue: string }) => | |
| `Różni się od udokumentowanej wartości: ${expectedValue}`, | |
| }, | |
| preview: { | |
| secretValueHidden: ({ value }: { value: string }) => `${value} - ukryte ze względów bezpieczeństwa`, | |
| hiddenValue: '***ukryte***', | |
| emptyValue: '(puste)', | |
| sessionWillReceive: ({ name, value }: { name: string; value: string }) => | |
| `Sesja otrzyma: ${name} = ${value}`, | |
| }, | |
| previewModal: { | |
| titleWithProfile: ({ profileName }: { profileName: string }) => `Zmienne środowiskowe · ${profileName}`, | |
| descriptionPrefix: 'Te zmienne środowiskowe są wysyłane podczas uruchamiania sesji. Wartości są rozwiązywane przez daemon na', | |
| descriptionFallbackMachine: 'wybranej maszynie', | |
| descriptionSuffix: '.', | |
| emptyMessage: 'Dla tego profilu nie ustawiono zmiennych środowiskowych.', | |
| checkingSuffix: '(sprawdzanie…)', | |
| detail: { | |
| fixed: 'Stała', | |
| machine: 'Maszyna', | |
| checking: 'Sprawdzanie', | |
| fallback: 'Fallback', | |
| missing: 'Brak', | |
| }, | |
| }, | |
| }, | |
| builtIn: 'Wbudowane', | |
| groups: { | |
| favorites: 'Ulubione', | |
| custom: 'Twoje profile', | |
| builtIn: 'Profile wbudowane', | |
| }, | |
| actions: { | |
| viewEnvironmentVariables: 'Zmienne środowiskowe', | |
| addToFavorites: 'Dodaj do ulubionych', | |
| removeFromFavorites: 'Usuń z ulubionych', | |
| editProfile: 'Edytuj profil', | |
| duplicateProfile: 'Duplikuj profil', | |
| deleteProfile: 'Usuń profil', | |
| }, | |
| copySuffix: '(Kopia)', | |
| duplicateName: 'Profil o tej nazwie już istnieje', | |
| setupInstructions: { | |
| title: 'Instrukcje konfiguracji', | |
| viewOfficialGuide: 'Zobacz oficjalny przewodnik konfiguracji', | |
| }, | |
| defaultSessionType: 'Domyślny typ sesji', | |
| defaultPermissionMode: { | |
| title: 'Domyślny tryb uprawnień', | |
| descriptions: { | |
| default: 'Pytaj o uprawnienia', | |
| acceptEdits: 'Automatycznie zatwierdzaj edycje', | |
| plan: 'Zaplanuj przed wykonaniem', | |
| bypassPermissions: 'Pomiń wszystkie uprawnienia', | |
| }, | |
| }, | |
| aiBackend: { | |
| title: 'Backend AI', | |
| selectAtLeastOneError: 'Wybierz co najmniej jeden backend AI.', | |
| claudeSubtitle: 'CLI Claude', | |
| codexSubtitle: 'CLI Codex', | |
| geminiSubtitleExperimental: 'CLI Gemini (eksperymentalne)', | |
| }, | |
| tmux: { | |
| title: 'Tmux', | |
| spawnSessionsTitle: 'Uruchamiaj sesje w Tmux', | |
| spawnSessionsEnabledSubtitle: 'Sesje uruchamiają się w nowych oknach tmux.', | |
| spawnSessionsDisabledSubtitle: 'Sesje uruchamiają się w zwykłej powłoce (bez integracji z tmux)', | |
| sessionNamePlaceholder: 'Puste = bieżąca/najnowsza sesja', | |
| tempDirPlaceholder: '/tmp (opcjonalne)', | |
| }, | |
| previewMachine: { | |
| title: 'Podgląd maszyny', | |
| selectMachine: 'Wybierz maszynę', | |
| resolveSubtitle: 'Rozwiąż zmienne środowiskowe maszyny dla tego profilu.', | |
| selectSubtitle: 'Wybierz maszynę, aby podejrzeć rozwiązane wartości.', | |
| }, | |
| environmentVariables: { | |
| title: 'Zmienne środowiskowe', | |
| addVariable: 'Dodaj zmienną', | |
| namePlaceholder: 'Nazwa zmiennej (np. MY_CUSTOM_VAR)', | |
| valuePlaceholder: 'Wartość (np. my-value lub ${MY_VAR})', | |
| validation: { | |
| nameRequired: 'Wprowadź nazwę zmiennej.', | |
| invalidNameFormat: 'Nazwy zmiennych muszą zawierać wielkie litery, cyfry i podkreślenia oraz nie mogą zaczynać się od cyfry.', | |
| duplicateName: 'Taka zmienna już istnieje.', | |
| }, | |
| card: { | |
| valueLabel: 'Wartość:', | |
| fallbackValueLabel: 'Wartość fallback:', | |
| valueInputPlaceholder: 'Wartość', | |
| defaultValueInputPlaceholder: 'Wartość domyślna', | |
| secretNotRetrieved: 'Wartość sekretna - nie jest pobierana ze względów bezpieczeństwa', | |
| overridingDefault: ({ expectedValue }: { expectedValue: string }) => | |
| `Nadpisywanie udokumentowanej wartości domyślnej: ${expectedValue}`, | |
| useMachineEnvToggle: 'Użyj wartości ze środowiska maszyny', | |
| resolvedOnSessionStart: 'Rozwiązywane podczas uruchamiania sesji na wybranej maszynie.', | |
| sourceVariableLabel: 'Zmienna źródłowa', | |
| sourceVariablePlaceholder: 'Nazwa zmiennej źródłowej (np. Z_AI_MODEL)', | |
| checkingMachine: ({ machine }: { machine: string }) => `Sprawdzanie ${machine}...`, | |
| emptyOnMachine: ({ machine }: { machine: string }) => `Pusto na ${machine}`, | |
| emptyOnMachineUsingFallback: ({ machine }: { machine: string }) => `Pusto na ${machine} (używam fallback)`, | |
| notFoundOnMachine: ({ machine }: { machine: string }) => `Nie znaleziono na ${machine}`, | |
| notFoundOnMachineUsingFallback: ({ machine }: { machine: string }) => `Nie znaleziono na ${machine} (używam fallback)`, | |
| valueFoundOnMachine: ({ machine }: { machine: string }) => `Znaleziono wartość na ${machine}`, | |
| differsFromDocumented: ({ expectedValue }: { expectedValue: string }) => | |
| `Różni się od udokumentowanej wartości: ${expectedValue}`, | |
| }, | |
| preview: { | |
| secretValueHidden: ({ value }: { value: string }) => `${value} - ukryte ze względów bezpieczeństwa`, | |
| hiddenValue: '***ukryte***', | |
| emptyValue: '(puste)', | |
| sessionWillReceive: ({ name, value }: { name: string; value: string }) => | |
| `Sesja otrzyma: ${name} = ${value}`, | |
| }, | |
| previewModal: { | |
| titleWithProfile: ({ profileName }: { profileName: string }) => `Zmienne środowiskowe · ${profileName}`, | |
| descriptionPrefix: 'Te zmienne środowiskowe są wysyłane podczas uruchamiania sesji. Wartości są rozwiązywane przez daemon na', | |
| descriptionFallbackMachine: 'wybranej maszynie', | |
| descriptionSuffix: '.', | |
| emptyMessage: 'Dla tego profilu nie ustawiono zmiennych środowiskowych.', | |
| checkingSuffix: '(sprawdzanie…)', | |
| detail: { | |
| fixed: 'Stała', | |
| machine: 'Maszyna', | |
| checking: 'Sprawdzanie', | |
| fallback: 'Fallback', | |
| missing: 'Brak', | |
| }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In `@sources/text/translations/pl.ts` around lines 997 - 1101, The copySuffix
translation is still in English—update the copySuffix entry (the copySuffix
property inside the profile translations object) from "(Copy)" to the correct
Polish term (e.g., "(Kopia)") so the UI shows a localized suffix; locate the
copySuffix key near the profile actions and replace its value with the Polish
translation.
| noMatches: 'Nenhuma correspondência', | ||
| all: 'All', | ||
| machine: 'máquina', | ||
| clearSearch: 'Clear search', |
There was a problem hiding this comment.
Localize newly added strings left in English.
A few new UI strings are still English and will appear untranslated in Portuguese.
📝 Suggested localization
@@
- all: 'All',
- clearSearch: 'Clear search',
+ all: 'Todos',
+ clearSearch: 'Limpar busca',
@@
- copySuffix: '(Copy)',
+ copySuffix: '(Cópia)',Also applies to: 978-979
🤖 Prompt for AI Agents
In `@sources/text/translations/pt.ts` around lines 68 - 71, Translate the English
strings left in the Portuguese translations by updating the keys in the
translations object: replace all: 'All' with the Portuguese equivalent (e.g.,
'Todos' or context-appropriate plural), and clearSearch: 'Clear search' with
'Limpar pesquisa' (or alternative local phrasing); ensure the existing
noMatches: 'Nenhuma correspondência' and machine: 'máquina' remain unchanged and
apply the same fixes for the duplicate occurrences referenced by the same keys
elsewhere in the file (the other instances of all and clearSearch).
| noMatches: 'Нет совпадений', | ||
| all: 'All', | ||
| machine: 'машина', | ||
| clearSearch: 'Clear search', |
There was a problem hiding this comment.
Localize newly added strings left in English.
A few new UI strings are still English and will appear untranslated in Russian.
📝 Suggested localization
@@
- all: 'All',
- clearSearch: 'Clear search',
+ all: 'Все',
+ clearSearch: 'Очистить поиск',
@@
- copySuffix: '(Copy)',
+ copySuffix: '(Копия)',Also applies to: 1010-1011
🤖 Prompt for AI Agents
In `@sources/text/translations/ru.ts` around lines 79 - 82, The translation file
contains untranslated English strings: update the values for the keys "all" and
"clearSearch" (and any other occurrences noted around the same diff, e.g., lines
referenced 1010-1011) in the ru.ts translations object to their Russian
equivalents while keeping the keys intact; for example replace "all" => "Все"
and "clearSearch" => "Очистить поиск" so the UI shows localized text.
1 participant
PR 272
fix: stabilize PR272 new-session UX, profiles gating, and env safety
Context / Goal
This PR is a follow-up to
slopus/happy#272, which introduced a large new-session + profiles + env-var/settings/sync surface.The goal here is to keep the feature work, but restore the pre-#272 “standard New Session” UX and make the new profiles/env work coherent + safe.
I’m trying to keep this factual (no blame): this is mostly stabilization work after a very large merge.
Summary (what this PR does)
What #272 introduced (and what we’re fixing here)
Profiles/editor correctness
id: ''.Navigation coherence (web + native + unmount safety)
Global daemon env fetch (and secret exposure risk)
envVarRefs/daemonEnvfetch in/newthat queried the daemon for all${VAR}references across all profiles (including secrets).New Session (standard flow) regressions
Wizard + pickers inconsistent with existing UI
Profiles “leaked” into default behavior
Non-UI safety/reliability issues (PR272 logic surface)
profileSync.ts, `NewSessionWizard) that wasn’t actually wired.Even if Codex was detected on the machines, the Codex profiles stayed disabled
IMAGE
What we implemented to fix/improve it (what’s in this branch)
1) New Session UX restored (pre-PR272 feel, still compatible with new features)
t('session.inputPlaceholder')) and legacy keyboard behavior.2) Wizard + pickers refactored to reduce duplication and improve coherence
3) Profiles made truly optional (separate feature flag) + coherent selection UX
useProfilesflag (separate fromuseEnhancedSessionWizard).4) Profile identity persisted/displayed safely
profileIdand show a read-only chip in session UI.Non-UI “logic hardening” improvements tied to PR272’s new surface area
These changes are limited to files added/modified by PR272, and fix real issues introduced or made risky by PR272’s new data/features.
A) Prevent leaking secrets via logs
__DEV__.B) Fix tool-result robustness (don’t drop messages; don’t break on structured outputs)
C) Make settings parsing tolerant (avoid “one bad profile nukes all settings”)
settingsParse()to parse fields individually and filter invalid profile entries rather than failing the entire settings object.D) Unify env-var template parsing + preserve correctness in remote env querying
${VAR}and${VAR:-default}consistently.nodewhen available (fallback remains safe)E) Remove unused placeholder sync service & component added by PR272
profileSync.ts&components/NewSessionWizard.tsxwere added but not referenced; removed to prevent future confusion.F) Conservative security: avoid querying remote values for secret-like vars
PASS|PASSWORD|COOKIEin addition to token/key/secret/auth.Links to the detailled issues in slopus/happy#272 (links go to the PR diff)
https://github.com/slopus/happy/pull/272/files#diff-30e51abcbe846d95abc7b9de0aee916aa6a7489e72486ec06388b9ac612f7e6bR322
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R1147
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R1897
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R1007
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R1012
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R481
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R491
https://github.com/slopus/happy/pull/272/files#diff-03a554e2004f08654732692f1487a20f748f4ed90e7e187d90678e7d24abb232R401
https://github.com/slopus/happy/pull/272/files#diff-388510a51fa84edaf57a36644f1a084aeb18ee8bd265a798133f48dc5cee281cR257
https://github.com/slopus/happy/pull/272/files#diff-388510a51fa84edaf57a36644f1a084aeb18ee8bd265a798133f48dc5cee281cR270
https://github.com/slopus/happy/pull/272/files#diff-0275aef060275b84215b2726a2b6c016e929a644b469df6b77915fbf3fa8de48R32
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R780
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R43
https://github.com/slopus/happy/pull/272/files#diff-0275aef060275b84215b2726a2b6c016e929a644b469df6b77915fbf3fa8de48R25
https://github.com/slopus/happy/pull/272/files#diff-0275aef060275b84215b2726a2b6c016e929a644b469df6b77915fbf3fa8de48R46
https://github.com/slopus/happy/pull/272/files#diff-92a4e9f2312662ce954da5d3c480da187552b897c564bc19e45db5e2b5c5cdabR110
https://github.com/slopus/happy/pull/272/files#diff-cbde5d803bb1053919f1167c4eb4f7a56b48dd93f77e75d0ebef73e4fe1f3f92R92
https://github.com/slopus/happy/pull/272/files#diff-15dcee897d504b4753e80c45148ed21f31394550cf02721bd2641929b4be06b1R136
https://github.com/slopus/happy/pull/272/files#diff-01c0b6096ea1c61ee49415456c4d662f513a21a3e3295d7882fde9b94305137aR62
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R563
https://github.com/slopus/happy/pull/272/files#diff-f4b041fa0826ef7d66e7e872b4e23bd490d2dc755c0c879551045a005731f401R1346
https://github.com/slopus/happy/pull/272/files#diff-21ea1e7b314af287b486e284fe47fa9681dc76a38cb14c0e2395d7467a7556e0R48
https://github.com/slopus/happy/pull/272/files#diff-21ea1e7b314af287b486e284fe47fa9681dc76a38cb14c0e2395d7467a7556e0R148
https://github.com/slopus/happy/pull/272/files#diff-92a4e9f2312662ce954da5d3c480da187552b897c564bc19e45db5e2b5c5cdabR372
https://github.com/slopus/happy/pull/272/files#diff-7077cf5c55232633bf3a22097c51d59b3eb25db37a642f78e128197f3425e8c2R86
https://github.com/slopus/happy/pull/272/files#diff-7077cf5c55232633bf3a22097c51d59b3eb25db37a642f78e128197f3425e8c2R92
https://github.com/slopus/happy/pull/272/files#diff-9b2dcf6707736bef944b8a005ff7ff6b4f5856defbe446710027e1ebc25b17d1R143
https://github.com/slopus/happy/pull/272/files#diff-cbde5d803bb1053919f1167c4eb4f7a56b48dd93f77e75d0ebef73e4fe1f3f92R84
(This is relevant because PR107’s tmux util does not implement “current/most recent” when name is empty—see below.)
https://github.com/slopus/happy/pull/272/files#diff-a2f78b18918804b89391c64e0942948bfbd600834e789295a93fec0328365e38R14
https://github.com/slopus/happy/pull/272/files#diff-f754c8a677da1cc9a08309fa83d216afe0b6d2c47561cb89b32fc97c6c970153R12
(Runtime import of English from _default.ts is in sources/text/index.ts, but that file wasn’t changed in #272 so it can’t be linked via the PR /files view.)
newpage):https://github.com/slopus/happy/pull/272/files#diff-51775e520e07fb30ce518a991ca21278568a253304b102071fce5fb4c6e41effR1
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.