Open
Conversation
- Implemented LocaleProvider to manage app localization settings, including loading and setting locale codes. - Created StoragePermissionService to handle storage permission requests with user dialogs. - Updated pubspec.lock with new dependencies.
- Created app_strings_zh_hans.dart for Simplified Chinese translations. - Created app_strings_zh_hant.dart for Traditional Chinese translations. - Implemented _expand_l10n.dart script to generate localized string maps from English and Japanese sources.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds full i18n: new AppLocalizations and locale bundles (en, zh-Hans, zh-Hant, ja), LocaleProvider with persistence, MaterialApp wired for runtime locale resolution and localized titles, many screens/widgets converted to l10n keys, provider baseUrl support, theme property adjustments, and l10n tooling script. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (main)
participant LP as LocaleProvider
participant Pref as PreferencesService
participant Device as Device Locale
participant AL as AppLocalizations
participant UI as Widgets
App->>LP: Provide LocaleProvider via Provider
LP->>Pref: load() -> read localeCode
Pref-->>LP: stored localeCode|null
alt stored explicit locale
LP-->>App: locale (e.g., zh-Hant)
else system/default
App->>Device: query device locale
Device-->>LP: device locale
LP-->>App: resolved locale or null
end
App->>AL: LocalizationsDelegate.load(resolved locale)
AL-->>UI: localized strings via context.l10n
UI->>AL: widgets call l10n.t(key) to render text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
TIANLI0
changed the title
TIANLI0
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 24 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="flutter_app/lib/l10n/app_strings_ja.dart">
<violation number="1" location="flutter_app/lib/l10n/app_strings_ja.dart:43">
P2: Roughly half of the entries in this Japanese localization file are left untranslated in English. User-facing strings like `'API key cannot be empty'`, `'Save & Activate'`, `'This will download Ubuntu, Node.js, and OpenClaw into a self-contained environment.'`, `'Auto-pair with gateway on this device'`, and many others should be translated to Japanese for a consistent user experience. The file currently has a mix of translated keys (e.g., gateway, logs, settings titles) and untranslated keys (e.g., provider details, setup wizard statuses, node configuration, settings subtitles).</violation>
</file>
<file name="flutter_app/lib/screens/configure_screen.dart">
<violation number="1" location="flutter_app/lib/screens/configure_screen.dart:149">
P2: Add a `mounted` check before this `setState` call. `_startConfigure()` is async with multiple `await`s above this catch block; if the widget is disposed mid-flight, accessing `context.l10n` on a deactivated element can throw in release mode. The `_takeScreenshot` method nearby already follows the correct pattern (`if (!mounted) return;`).</violation>
</file>
<file name="flutter_app/lib/screens/onboarding_screen.dart">
<violation number="1" location="flutter_app/lib/screens/onboarding_screen.dart:202">
P2: Accessing `context.l10n` inside `setState` after async gaps without a `mounted` check risks a crash if the widget is disposed during the awaited operations. Other callbacks in this file already guard with `if (mounted)`.</violation>
</file>
<file name="flutter_app/lib/app.dart">
<violation number="1" location="flutter_app/lib/app.dart:69">
P2: Custom locale resolution misroutes `zh_TW` and `zh_HK` (no scriptCode) to Simplified Chinese instead of Traditional Chinese. The second matching pass compares `scriptCode` directly, so `null == null` matches `Locale('zh')` (Simplified) before the Traditional entry is ever considered.
Add country-code inference for Chinese locales to the callback, similar to what `AppLocalizations._localeToKey` already does. For example, insert a Chinese-specific check before the third (language-only) pass:</violation>
</file>
<file name="flutter_app/lib/screens/node_screen.dart">
<violation number="1" location="flutter_app/lib/screens/node_screen.dart:225">
P2: The "Serial" capability tile was missed during localization. All other 7 capability tiles in this section were localized with `l10n.t()`, but this one still uses hardcoded English strings `'Serial'` and `'Bluetooth and USB serial communication'`. Corresponding l10n keys (`nodeCapabilitySerialTitle` / `nodeCapabilitySerialSubtitle`) also need to be added to each locale file.</violation>
</file>
<file name="flutter_app/lib/screens/setup_wizard_screen.dart">
<violation number="1" location="flutter_app/lib/screens/setup_wizard_screen.dart:24">
P2: `_permissionError` is never set to a non-null value anywhere in this file—it is only declared and cleared to `null`. The condition `_permissionError != null` is always false, and `_permissionError ??` always falls through. This looks like an incomplete implementation of permission error handling in `_beginSetup` (which is `async` but doesn't `await`/catch `runSetup()`).</violation>
<violation number="2" location="flutter_app/lib/screens/setup_wizard_screen.dart:45">
P1: `provider.runSetup()` is not awaited. Since `_beginSetup` is `async`, the returned `Future` from `runSetup()` is silently discarded, meaning any thrown exception becomes an unhandled async error rather than being catchable. Either `await` the call (and add error handling as the `_permissionError` field suggests was intended), or remove the `async` keyword if fire-and-forget is deliberate.</violation>
</file>
<file name="flutter_app/lib/l10n/app_strings_zh_hant.dart">
<violation number="1" location="flutter_app/lib/l10n/app_strings_zh_hant.dart:146">
P2: Incorrect Traditional Chinese character: `修複` should be `修復`. The character `複` (complex/duplicate) is wrong here; the correct character for "repair" is `復` (restore/recover). This error also appears in `settingsRerunSetupSubtitle`.</violation>
<violation number="2" location="flutter_app/lib/l10n/app_strings_zh_hant.dart:190">
P2: Incorrect Traditional Chinese character: `錄制` should be `錄製`. In Traditional Chinese, the character for "produce/record" is `製`, not `制` (which means "restrain/system"). This appears to be an incomplete Simplified→Traditional conversion.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Owner
|
@TIANLI0 please resolve the build issues. |
There was a problem hiding this comment.
Pull request overview
Adds a custom localization/i18n layer (EN, zh-Hans, zh-Hant, JA) and wires much of the UI to use localized strings, plus introduces locale persistence and some storage-permission UX improvements in Settings.
Changes:
- Introduces
AppLocalizations+ localized string maps and hooks them intoMaterialApp. - Adds locale persistence via
PreferencesService+LocaleProvider, and a language selector in Settings. - Refactors multiple screens/widgets to use localized strings; extends provider configuration UI to support custom base URLs.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| flutter_app/scripts/_expand_l10n.dart | Script to generate/expand localized string-map files. |
| flutter_app/pubspec.yaml | Adds flutter_localizations SDK dependency. |
| flutter_app/lib/widgets/node_controls.dart | Localizes Node controls UI and status badge labels. |
| flutter_app/lib/widgets/gateway_controls.dart | Localizes Gateway controls UI and status badge labels. |
| flutter_app/lib/services/provider_config_service.dart | Adds optional baseUrl override to provider config saving; doc tweak. |
| flutter_app/lib/services/preferences_service.dart | Persists selected locale code in shared preferences. |
| flutter_app/lib/screens/setup_wizard_screen.dart | Localizes setup wizard UI and adds message-localization mapping. |
| flutter_app/lib/screens/settings_screen.dart | Adds language dropdown and storage permission confirmation dialog; localizes UI. |
| flutter_app/lib/screens/providers_screen.dart | Localizes provider list UI; updates status labeling and passes baseUrl/model to detail screen. |
| flutter_app/lib/screens/provider_detail_screen.dart | Adds base URL field (when supported) and localizes copy; validation + save/remove messages. |
| flutter_app/lib/screens/packages_screen.dart | Localizes optional packages screen and descriptions. |
| flutter_app/lib/screens/onboarding_screen.dart | Localizes onboarding UI strings and snackbars/tooltips. |
| flutter_app/lib/screens/node_screen.dart | Localizes node configuration screen strings and refactors gateway connection radio UI. |
| flutter_app/lib/screens/logs_screen.dart | Localizes logs screen UI strings and snackbars/tooltips. |
| flutter_app/lib/screens/dashboard_screen.dart | Localizes most dashboard card labels and adds localized node-status mapping. |
| flutter_app/lib/screens/configure_screen.dart | Localizes configure screen UI strings and snackbars/tooltips. |
| flutter_app/lib/providers/locale_provider.dart | New provider to load/save locale selection and expose Locale?. |
| flutter_app/lib/models/ai_provider.dart | Converts provider display strings to localization keys; adds custom base URL support + new providers. |
| flutter_app/lib/l10n/app_strings_zh_hant.dart | Traditional Chinese string map. |
| flutter_app/lib/l10n/app_strings_zh_hans.dart | Simplified Chinese string map. |
| flutter_app/lib/l10n/app_strings_ja.dart | Japanese string map. |
| flutter_app/lib/l10n/app_strings_en.dart | English string map (base/fallback). |
| flutter_app/lib/l10n/app_localizations.dart | New localizations implementation + delegate + context.l10n extension. |
| flutter_app/lib/app.dart | Wires localization + locale resolution into MaterialApp; theme refactors. |
Comments suppressed due to low confidence (2)
flutter_app/lib/app.dart:218
- ThemeData.dialogTheme expects a DialogTheme, but this code uses DialogThemeData. This is likely a compile-time type error; use the correct theme data type for the Flutter SDK in use (typically DialogTheme).
dialogTheme: DialogThemeData(
backgroundColor: AppColors.darkSurface,
shape: RoundedRectangleBorder(
borderRadius: BorderRadius.circular(16),
side: const BorderSide(color: AppColors.darkBorder),
),
),
flutter_app/lib/app.dart:150
- ThemeData.cardTheme expects a CardTheme, but this code uses CardThemeData. Unless the project defines a custom type (it doesn't appear to), this will fail to compile; use CardTheme here (or verify the correct Flutter API type for your pinned Flutter SDK).
cardTheme: CardThemeData(
elevation: 0,
color: AppColors.darkSurface,
shape: RoundedRectangleBorder(
borderRadius: BorderRadius.circular(12),
side: const BorderSide(color: AppColors.darkBorder),
),
margin: const EdgeInsets.symmetric(vertical: 4),
),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
147
to
164
| switch (status) { | ||
| case GatewayStatus.running: | ||
| color = AppColors.statusGreen; | ||
| label = 'Running'; | ||
| label = l10n.t('gatewayStatusRunning'); | ||
| icon = Icons.check_circle_outline; | ||
| case GatewayStatus.starting: | ||
| color = AppColors.statusAmber; | ||
| label = 'Starting'; | ||
| label = l10n.t('gatewayStatusStarting'); | ||
| icon = Icons.hourglass_top; | ||
| case GatewayStatus.error: | ||
| color = AppColors.statusRed; | ||
| label = 'Error'; | ||
| label = l10n.t('gatewayStatusError'); | ||
| icon = Icons.error_outline; | ||
| case GatewayStatus.stopped: | ||
| color = AppColors.statusGrey; | ||
| label = 'Stopped'; | ||
| label = l10n.t('gatewayStatusStopped'); | ||
| icon = Icons.circle_outlined; | ||
| } |
There was a problem hiding this comment.
This switch assigns color/label/icon but has no break/return statements. In Dart this is either a compile error (most likely) or will fall through and end up using the last case's values, producing incorrect badges. Add break after each case (or rewrite as a switch expression returning a tuple/object).
Comment on lines
297
to
309
| DropdownButtonFormField<String>( | ||
| value: _selectedModel, | ||
| initialValue: _selectedModel, | ||
| isExpanded: true, | ||
| decoration: const InputDecoration(), | ||
| items: [ | ||
| ...widget.provider.defaultModels | ||
| .map((m) => DropdownMenuItem(value: m, child: Text(m))), | ||
| const DropdownMenuItem( | ||
| DropdownMenuItem( | ||
| value: _customModelSentinel, | ||
| child: Text('Custom...'), | ||
| child: Text(l10n.t('providerDetailCustomModelAction')), | ||
| ), | ||
| ], | ||
| onChanged: (value) { |
There was a problem hiding this comment.
DropdownButtonFormField doesn't take an initialValue parameter in the stable Flutter API (it uses value). As written this is likely a compile-time error; even if it compiles in your SDK, initialValue won't update when _selectedModel changes programmatically. Prefer binding the selection via value: _selectedModel so the widget reliably reflects state.
| Padding( | ||
| padding: const EdgeInsets.fromLTRB(16, 0, 16, 8), | ||
| child: DropdownButtonFormField<String>( | ||
| initialValue: localeProvider.localeCode, |
There was a problem hiding this comment.
DropdownButtonFormField typically uses value rather than initialValue. Using initialValue here is likely a compile-time error on stable Flutter, and it can also prevent the dropdown from reflecting updates from LocaleProvider on rebuilds. Bind the selected option with value: localeProvider.localeCode instead.
Suggested change
| initialValue: localeProvider.localeCode, | |
| value: localeProvider.localeCode, |
| Future<void> _beginSetup(SetupProvider provider) async { | ||
| setState(() { | ||
| _started = true; | ||
| _permissionError = null; |
There was a problem hiding this comment.
_permissionError is introduced, cleared, and checked for display, but it's never set to a non-null value anywhere in this screen. This adds dead state and UI branching; either wire it up (e.g., set it when storage permission is missing/denied before starting setup) or remove it and rely on SetupProvider.state.error.
Suggested change
| _permissionError = null; | |
| _permissionError = provider.state.error; |
| /// Read the current config and return a map with: | ||
| /// - `activeModel`: the current primary model string (or null) | ||
| /// - `providers`: Map<providerId, {apiKey, model}> for configured providers | ||
| /// - `providers`: `Map<providerId, {apiKey, model}>` for configured providers |
There was a problem hiding this comment.
The readConfig() doc comment says the returned providers map contains {apiKey, model}, but provider entries include baseUrl and models (list) rather than a model field. Update the comment to match the actual shape so callers don't read the wrong keys (as happens in ProvidersScreen).
Suggested change
| /// - `providers`: `Map<providerId, {apiKey, model}>` for configured providers | |
| /// - `providers`: `Map<providerId, {apiKey, baseUrl, models}>` for configured providers, | |
| /// where `models` is a list of model identifiers. |
Comment on lines
39
to
+66
| final providerConfig = _providers[provider.id] as Map<String, dynamic>?; | ||
| final result = await Navigator.of(context).push<bool>( | ||
| MaterialPageRoute( | ||
| builder: (_) => ProviderDetailScreen( | ||
| provider: provider, | ||
| existingApiKey: providerConfig?['apiKey'] as String?, | ||
| existingModel: _activeModel, | ||
| existingBaseUrl: providerConfig?['baseUrl'] as String?, | ||
| existingModel: providerConfig?['model'] as String? ?? _activeModel, | ||
| ), | ||
| ), | ||
| ); | ||
| if (result == true) { | ||
| _refresh(); | ||
| } | ||
| } | ||
|
|
||
| String _statusLabel(AiProvider provider) { | ||
| ({String label, bool isActive}) _statusInfo(AiProvider provider) { | ||
| final l10n = context.l10n; | ||
| final isConfigured = _providers.containsKey(provider.id); | ||
| if (!isConfigured) return ''; | ||
| // Check if the active model belongs to this provider | ||
| if (!isConfigured) { | ||
| return (label: '', isActive: false); | ||
| } | ||
|
|
||
| final providerConfig = _providers[provider.id] as Map<String, dynamic>?; | ||
| final configuredModel = providerConfig?['model'] as String?; | ||
| if (_activeModel != null) { | ||
| final isActive = provider.defaultModels.any((m) => _activeModel!.contains(m)) || | ||
| _activeModel!.contains(provider.id); | ||
| if (isActive) return 'Active'; | ||
| final isActive = configuredModel == _activeModel || | ||
| provider.matchesModel(_activeModel!); |
There was a problem hiding this comment.
Provider config entries written by ProviderConfigService use the key 'models' (a list), not 'model'. Reading providerConfig['model'] will always be null, so the detail screen won't preselect the configured model and the active/configured badge logic may be wrong. Read the first element of providerConfig['models'] (or whatever is appropriate) instead of providerConfig['model'].
| l10n.t('nodeCapabilityCanvasTitle'), | ||
| l10n.t('nodeCapabilityCanvasSubtitle'), | ||
| Icons.web, | ||
| available: false, |
There was a problem hiding this comment.
The Canvas capability is marked available: false, but the localized subtitle key used here describes normal functionality (e.g. “Navigate and interact with web pages”). This is misleading for users and translators. Either change the subtitle key/value to indicate it's unavailable on mobile, or mark the capability as available.
Suggested change
| available: false, | |
| available: true, |
Comment on lines
75
to
96
| final subtitle = provider.state.isRunning | ||
| ? (token != null | ||
| ? 'Token: ${token.substring(0, (token.length > 8 ? 8 : token.length))}...' | ||
| : 'Open OpenClaw dashboard in browser') | ||
| : 'Start gateway first'; | ||
| return StatusCard( | ||
| title: 'Web Dashboard', | ||
| title: l10n.t('dashboardWebDashboardTitle'), | ||
| subtitle: subtitle, | ||
| icon: Icons.dashboard, | ||
| trailing: Row( | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| if (token != null) | ||
| IconButton( | ||
| icon: const Icon(Icons.copy, size: 18), | ||
| tooltip: 'Copy dashboard URL', | ||
| onPressed: () { | ||
| Clipboard.setData(ClipboardData(text: url!)); | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| const SnackBar(content: Text('Dashboard URL copied')), | ||
| const SnackBar( | ||
| content: Text('Dashboard URL copied')), | ||
| ); |
There was a problem hiding this comment.
This section still contains user-facing strings that bypass the new localization system (e.g., token subtitle text, “Open OpenClaw dashboard in browser”, “Start gateway first”, tooltip, and SnackBar text). Since this PR is wiring up i18n, these should be moved to l10n keys (with params for the token prefix) so the Web Dashboard card is fully translated.
Comment on lines
+63
to
+92
| localeListResolutionCallback: (deviceLocales, supportedLocales) { | ||
| if (localeProvider.locale != null) { | ||
| return localeProvider.locale; | ||
| } | ||
|
|
||
| for (final deviceLocale in deviceLocales ?? const <Locale>[]) { | ||
| for (final supportedLocale in supportedLocales) { | ||
| if (supportedLocale.languageCode == deviceLocale.languageCode && | ||
| supportedLocale.scriptCode == deviceLocale.scriptCode && | ||
| supportedLocale.countryCode == deviceLocale.countryCode) { | ||
| return supportedLocale; | ||
| } | ||
| } | ||
|
|
||
| for (final supportedLocale in supportedLocales) { | ||
| if (supportedLocale.languageCode == deviceLocale.languageCode && | ||
| supportedLocale.scriptCode == deviceLocale.scriptCode) { | ||
| return supportedLocale; | ||
| } | ||
| } | ||
|
|
||
| for (final supportedLocale in supportedLocales) { | ||
| if (supportedLocale.languageCode == deviceLocale.languageCode) { | ||
| return supportedLocale; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return supportedLocales.first; | ||
| }, |
There was a problem hiding this comment.
The localeListResolutionCallback returns a supported Locale without preserving zh-HK/zh-TW/etc. country info; this means Traditional Chinese users can resolve to Locale('zh') (Simplified) even though AppLocalizations can map HK/TW to Hant when the Locale contains countryCode. Consider explicitly mapping zh locales with countryCode in {TW, HK, MO} to the supported Hant locale (Locale.fromSubtags(languageCode: 'zh', scriptCode: 'Hant')) before falling back to language-only matching.
Comment on lines
129
to
152
| switch (status) { | ||
| case NodeStatus.paired: | ||
| color = AppColors.statusGreen; | ||
| label = 'Paired'; | ||
| label = l10n.t('nodeStatusPaired'); | ||
| icon = Icons.check_circle_outline; | ||
| case NodeStatus.connecting: | ||
| case NodeStatus.challenging: | ||
| case NodeStatus.pairing: | ||
| color = AppColors.statusAmber; | ||
| label = 'Connecting'; | ||
| label = l10n.t('nodeStatusConnecting'); | ||
| icon = Icons.hourglass_top; | ||
| case NodeStatus.error: | ||
| color = AppColors.statusRed; | ||
| label = 'Error'; | ||
| label = l10n.t('nodeStatusError'); | ||
| icon = Icons.error_outline; | ||
| case NodeStatus.disabled: | ||
| color = AppColors.statusGrey; | ||
| label = 'Disabled'; | ||
| label = l10n.t('nodeStatusDisabled'); | ||
| icon = Icons.circle_outlined; | ||
| case NodeStatus.disconnected: | ||
| color = AppColors.statusGrey; | ||
| label = 'Disconnected'; | ||
| label = l10n.t('nodeStatusDisconnected'); | ||
| icon = Icons.link_off; | ||
| } |
There was a problem hiding this comment.
This switch assigns color/label/icon but has no break/return statements. In Dart this is either a compile error (most likely) or will fall through and end up using the last case's values, producing incorrect badges. Add break after each case (or rewrite as a switch expression returning a tuple/object).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
flutter_app/lib/screens/dashboard_screen.dart (1)
69-95:⚠️ Potential issue | 🟡 MinorLocalize the remaining literals in the Web Dashboard card.
This block still ships English subtitle/copy text (
Token: …,Open OpenClaw dashboard in browser,Start gateway first,Copy dashboard URL,Dashboard URL copied) even though the rest of the screen moved tol10n, so non-English locales will still see a mixed card. Reuse the existing dashboard/gateway copy keys here and add a token-preview key for the truncated token branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/dashboard_screen.dart` around lines 69 - 95, The StatusCard block uses hard-coded English strings for subtitle, tooltip and SnackBar — update it to use the l10n strings instead: replace the inline subtitle branches (the 'Token: …' truncated preview, 'Open OpenClaw dashboard in browser', and 'Start gateway first') with l10n lookups (add a new token-preview key for the truncated token branch), and replace the IconButton tooltip ('Copy dashboard URL') and the SnackBar text ('Dashboard URL copied') to use l10n as well; modify the code around the token variable, StatusCard.subtitle, IconButton.tooltip and the ScaffoldMessenger.of(context).showSnackBar call to fetch and interpolate the proper l10n keys.flutter_app/lib/screens/node_screen.dart (1)
184-233:⚠️ Potential issue | 🟡 MinorLocalize the
Serialcapability tile too.Every other capability in this section now goes through
l10n, but the last card is still hard-coded English, so localized builds will flip back to English here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/node_screen.dart` around lines 184 - 233, The Serial capability tile is hard-coded instead of localized; update the _capabilityTile call that currently passes 'Serial' and 'Bluetooth and USB serial communication' to use l10n.t(...) keys (e.g., l10n.t('nodeCapabilitySerialTitle') and l10n.t('nodeCapabilitySerialSubtitle')) and ensure corresponding localization keys are added to the app's l10n resource files so the tile displays translated text.flutter_app/lib/screens/configure_screen.dart (1)
145-150:⚠️ Potential issue | 🟠 MajorGuard the async completion path with
mounted.
_startConfigure()awaits multiple futures before these state updates at lines 145 and 147-150. If the user backs out during startup, both the success and error paths can hitsetState()after dispose, and the error path also readscontext.l10nafter that async gap.🛡️ Minimal guard
- setState(() => _loading = false); + if (!mounted) return; + setState(() => _loading = false); } catch (e) { - setState(() { + if (!mounted) return; + setState(() { _loading = false; _error = context.l10n.t('configureStartFailed', {'error': '$e'}); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/configure_screen.dart` around lines 145 - 150, The async completion paths in _startConfigure() call setState() and read context.l10n after awaiting, which can occur after the widget is disposed; fix by checking mounted before calling setState() and avoid using context after dispose by capturing the localized string into a local variable before awaiting or by checking mounted again and using a stored message. Specifically, in both the success path (where you set _loading = false) and the catch path (where you set _loading = false and _error), wrap setState calls with if (!mounted) return or if (!mounted) skip setState, and for the error message obtain context.l10n.t('configureStartFailed', {'error': '$e'}) into a local String before any await or only use it after confirming mounted.flutter_app/lib/screens/settings_screen.dart (1)
421-445:⚠️ Potential issue | 🟡 MinorSnapshot export/import should include the new locale preference.
This PR adds a persisted language setting, but the snapshot payload still omits it, so backup/restore silently drops the user's locale choice. Export
localeCodewith the other prefs and updateLocaleProviderafter import so the UI switches immediately.Also applies to: 480-518
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/settings_screen.dart` around lines 421 - 445, The snapshot export currently omits the persisted language; add the locale to the exported snapshot by including _prefs.localeCode in the snapshot object created in the export flow (the block building snapshot in settings_screen.dart) and write it out with the rest of the prefs. In the import flow (the corresponding snapshot import code around the import handler at lines ~480-518), read snapshot['localeCode'] and immediately apply it to the app by invoking the LocaleProvider API (e.g., call LocaleProvider.of(context).setLocale(Locale(localeCode)) or the project’s equivalent method) so the UI switches to the restored locale right after import.
🧹 Nitpick comments (4)
flutter_app/lib/screens/logs_screen.dart (1)
115-121: Post-frame callback registered inside builder may accumulate on rapid rebuilds.
addPostFrameCallbackis invoked inside theConsumerbuilder, which runs on every state change. When logs update frequently, multiple callbacks could queue before the frame completes, potentially causing redundant scroll operations.Consider moving the auto-scroll logic to a
didUpdateWidgetor using a listener pattern, or guard with a flag to avoid scheduling multiple callbacks per frame.♻️ Possible refactor using a simple guard
+ bool _scrollScheduled = false; + `@override` Widget build(BuildContext context) { // ... if (filtered.isEmpty) { // ... } - WidgetsBinding.instance.addPostFrameCallback((_) { - if (_autoScroll && _scrollController.hasClients) { - _scrollController.jumpTo( - _scrollController.position.maxScrollExtent, - ); - } - }); + if (_autoScroll && !_scrollScheduled) { + _scrollScheduled = true; + WidgetsBinding.instance.addPostFrameCallback((_) { + _scrollScheduled = false; + if (_autoScroll && _scrollController.hasClients) { + _scrollController.jumpTo( + _scrollController.position.maxScrollExtent, + ); + } + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/logs_screen.dart` around lines 115 - 121, The post-frame callback is being scheduled inside the Consumer builder which can run many times and enqueue duplicate callbacks; change the auto-scroll scheduling so it only schedules once per frame by introducing a guard flag (e.g., _autoScrollScheduled) next to existing _autoScroll and _scrollController or move the logic out of the Consumer into a lifecycle method like didUpdateWidget / initState or a scroll/listener handler; ensure WidgetsBinding.instance.addPostFrameCallback(...) is only called when !_autoScrollScheduled and clear the guard after the callback runs so duplicate callbacks are not queued.flutter_app/lib/l10n/app_strings_en.dart (1)
1-9: Minor: Inconsistent indentation throughout the file.The map entries have inconsistent indentation — some use 6 spaces (lines 3-7, 10+), while others use 4 spaces (lines 8-9). Consider running a formatter for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/l10n/app_strings_en.dart` around lines 1 - 9, The appStringsEn Map has inconsistent indentation for its entries (e.g., 'appName', 'languageEnglish', 'languageChinese', 'languageTraditionalChinese', 'languageJapanese' within appStringsEn); fix by normalizing whitespace so all key/value lines use the same indentation style (run dart format or adjust spaces to match the project's standard) ensuring consistent alignment inside the appStringsEn map.flutter_app/scripts/_expand_l10n.dart (1)
24-29: Minor: Redundant fallback in key iteration.On line 26,
base[key] ?? keyis used, but since we're iterating overbase.keys,base[key]will never be null. The?? keyfallback is unreachable.🧹 Simplified version
for (final key in base.keys) { - final value = localized[key] ?? base[key] ?? key; + final value = localized[key] ?? base[key]!; buffer.writeln(" '$key': '${_escape(value)}',"); seen.add(key); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/scripts/_expand_l10n.dart` around lines 24 - 29, The loop over base.keys contains an unreachable fallback: change the value assignment in the loop (where variables base, localized, value, buffer, seen and _escape are used) to remove the redundant "?? key" so it becomes value = localized[key] ?? base[key];; update the buffer.writeln call to use that value unchanged and keep seen.add(key) as-is.flutter_app/lib/l10n/app_strings_zh_hans.dart (1)
1-9: Minor: Same indentation inconsistency as English file.Lines 8-9 use 4-space indentation while others use 6 spaces. Consider running a formatter for consistency across all localization files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/l10n/app_strings_zh_hans.dart` around lines 1 - 9, The appStringsZhHans map has inconsistent indentation for the keys 'languageTraditionalChinese' and 'languageJapanese' (4 spaces) compared to the other entries (6 spaces); update those lines to match the existing 6-space indentation used throughout appStringsZhHans and run the Dart/Flutter formatter (dart format) on the localization files to ensure consistent formatting across all localization maps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter_app/lib/app.dart`:
- Around line 63-91: The locale resolution misses Traditional Chinese variants
because device Locales like Locale('zh','TW'/'HK'/'MO') often lack scriptCode
and fall through to the generic zh match; update localeListResolutionCallback to
add a special-case before the language-only match that normalizes Traditional
Chinese: when deviceLocale.languageCode == 'zh' and (deviceLocale.scriptCode ==
'Hant' OR deviceLocale.countryCode in {'TW','HK','MO'}), search supportedLocales
for a supportedLocale with scriptCode == 'Hant' (or matching countryCode if
present) and return it; keep this check before the generic language-only loop so
Traditional Chinese resolves to Hant/TW/HK/MO instead of Simplified zh.
In `@flutter_app/lib/l10n/app_strings_ja.dart`:
- Around line 42-60: The Japanese localization bundle contains many untranslated
English strings (e.g., providerDetailApiKeyEmpty, providerDetailApiKey,
providerDetailEndpointHelper, providerDetailModelEmpty, packagesUninstallTitle,
setupWizardIntroIdle, onboardingStartFailed, settingsBatteryOptimized and others
in the 117-166, 168-178, 216-263 ranges); please translate all remaining English
values into Japanese (or mark them intentionally withheld) so the ja bundle is
complete and consistent, updating the corresponding keys in app_strings_ja.dart
(providerDetail* keys and the other listed keys) and verify there are no English
fallbacks before enabling/exposing the ja locale.
- Around line 89-92: The translation for the key dashboardNodeDisabled is
incorrect for its current use as the disabled-state subtitle; update the value
of dashboardNodeDisabled in app_strings_ja.dart so it conveys the "node is
disabled" subtitle (e.g., a concise Japanese phrase indicating the node is
disabled) instead of the generic device capabilities description, ensuring
DashboardScreen shows the correct disabled-state message.
In `@flutter_app/lib/screens/configure_screen.dart`:
- Around line 64-69: The current empty catches around NativeBridge.setupDirs()
and NativeBridge.writeResolv() swallow startup errors; change them to surface
failures: catch the exception (and stack) in the try blocks around
NativeBridge.setupDirs and NativeBridge.writeResolv, log the error with context
via your app logger or Flutter's debug/logging utilities, and rethrow or
return/fail so _startConfigure() does not continue in a partially prepared state
(alternatively propagate a specific SetupException so callers can handle/display
the error).
In `@flutter_app/lib/screens/node_screen.dart`:
- Around line 76-145: The radio selection only toggles _isLocal but never
persists or establishes a local connection; update the RadioGroup<bool>
onChanged handler (and/or create provider.connectLocal()) so when value == true
you call the provider's local-connect/persist method (e.g.,
provider.connectLocal() or provider.connectLocal(save: true)) after setState,
mirroring how provider.connectRemote(host, port, token: ...) is used for remote;
ensure any previous remote settings are cleared/updated so reopening the screen
reflects the local choice (use _hostController/_portController/_tokenController
only for remote).
In `@flutter_app/lib/screens/onboarding_screen.dart`:
- Around line 122-129: The shell pipeline in onboarding_screen.dart currently
runs 'openclaw onboard;' and always prints the success banner, which can falsely
match _completionPattern; change the command element so onboarding success is
gated on the process exit status (e.g., replace 'openclaw onboard; echo "" &&
echo "Onboarding complete!..."' with a conditional such as 'if openclaw onboard;
then echo "" && echo "Onboarding complete! You can close this screen."; else
echo "" && echo "Onboarding failed" >&2; fi'), so the success banner is only
emitted when the openclaw command exits with status 0 and thus only matches
_completionPattern on real success. Ensure this updated string is placed in the
same command array element passed in onboarding_screen.dart.
- Line 202: The catch block updates state with setState setting _error using
context.l10n.t('onboardingStartFailed', {'error': '$e'}), but it lacks a mounted
check; before calling setState, add a guard like if (!mounted) return; to avoid
updating disposed widgets (keep the existing setState call and _error assignment
intact).
In `@flutter_app/lib/screens/providers_screen.dart`:
- Around line 42-46: The existingModel fallback incorrectly uses the global
_activeModel for any provider; change it so the fallback is only applied if the
active model actually matches the provider by using
provider.matchesModel(_activeModel). In other words, when constructing
ProviderDetailScreen (symbols: ProviderDetailScreen, existingModel,
providerConfig, provider.matchesModel, _activeModel), pass
providerConfig?['model'] as String? if present, otherwise pass _activeModel only
when provider.matchesModel(_activeModel) is true; otherwise pass null so
ProviderDetailScreen can use the provider's own default.
In `@flutter_app/lib/screens/settings_screen.dart`:
- Around line 107-139: The DropdownButtonFormField in SettingsScreen uses
localeProvider.localeCode as initialValue but can remain stuck on the default
'system' because LocaleProvider.load() is async; fix by forcing the
DropdownButtonFormField to recreate its FormField state when
localeProvider.localeCode changes—for example, give the DropdownButtonFormField
a key constructed from localeProvider.localeCode (use
ValueKey(localeProvider.localeCode)) so the widget is rebuilt with the updated
initialValue when LocaleProvider finishes loading and notifies listeners; update
the widget that builds the DropdownButtonFormField in SettingsScreen to use this
key and ensure localeProvider.load() remains async elsewhere
(LocaleProvider.load()).
In `@flutter_app/lib/screens/setup_wizard_screen.dart`:
- Around line 209-211: Active step label becomes blank because
_localizedSetupMessage(l10n, state.message) returns an empty string for
null/empty messages; change the conditional so that when state.step == step you
use the localized runtime message only if it is non-empty, otherwise fall back
to the original static label. Locate the expressions using state.step, step,
label and _localizedSetupMessage (appearing around the current snippet and the
similar block at lines ~311-314) and replace the ternary to check for a
non-empty result (e.g., compute msg = _localizedSetupMessage(...); use
msg.isNotEmpty ? msg : label) so the static label remains until a runtime
message is present.
---
Outside diff comments:
In `@flutter_app/lib/screens/configure_screen.dart`:
- Around line 145-150: The async completion paths in _startConfigure() call
setState() and read context.l10n after awaiting, which can occur after the
widget is disposed; fix by checking mounted before calling setState() and avoid
using context after dispose by capturing the localized string into a local
variable before awaiting or by checking mounted again and using a stored
message. Specifically, in both the success path (where you set _loading = false)
and the catch path (where you set _loading = false and _error), wrap setState
calls with if (!mounted) return or if (!mounted) skip setState, and for the
error message obtain context.l10n.t('configureStartFailed', {'error': '$e'})
into a local String before any await or only use it after confirming mounted.
In `@flutter_app/lib/screens/dashboard_screen.dart`:
- Around line 69-95: The StatusCard block uses hard-coded English strings for
subtitle, tooltip and SnackBar — update it to use the l10n strings instead:
replace the inline subtitle branches (the 'Token: …' truncated preview, 'Open
OpenClaw dashboard in browser', and 'Start gateway first') with l10n lookups
(add a new token-preview key for the truncated token branch), and replace the
IconButton tooltip ('Copy dashboard URL') and the SnackBar text ('Dashboard URL
copied') to use l10n as well; modify the code around the token variable,
StatusCard.subtitle, IconButton.tooltip and the
ScaffoldMessenger.of(context).showSnackBar call to fetch and interpolate the
proper l10n keys.
In `@flutter_app/lib/screens/node_screen.dart`:
- Around line 184-233: The Serial capability tile is hard-coded instead of
localized; update the _capabilityTile call that currently passes 'Serial' and
'Bluetooth and USB serial communication' to use l10n.t(...) keys (e.g.,
l10n.t('nodeCapabilitySerialTitle') and l10n.t('nodeCapabilitySerialSubtitle'))
and ensure corresponding localization keys are added to the app's l10n resource
files so the tile displays translated text.
In `@flutter_app/lib/screens/settings_screen.dart`:
- Around line 421-445: The snapshot export currently omits the persisted
language; add the locale to the exported snapshot by including _prefs.localeCode
in the snapshot object created in the export flow (the block building snapshot
in settings_screen.dart) and write it out with the rest of the prefs. In the
import flow (the corresponding snapshot import code around the import handler at
lines ~480-518), read snapshot['localeCode'] and immediately apply it to the app
by invoking the LocaleProvider API (e.g., call
LocaleProvider.of(context).setLocale(Locale(localeCode)) or the project’s
equivalent method) so the UI switches to the restored locale right after import.
---
Nitpick comments:
In `@flutter_app/lib/l10n/app_strings_en.dart`:
- Around line 1-9: The appStringsEn Map has inconsistent indentation for its
entries (e.g., 'appName', 'languageEnglish', 'languageChinese',
'languageTraditionalChinese', 'languageJapanese' within appStringsEn); fix by
normalizing whitespace so all key/value lines use the same indentation style
(run dart format or adjust spaces to match the project's standard) ensuring
consistent alignment inside the appStringsEn map.
In `@flutter_app/lib/l10n/app_strings_zh_hans.dart`:
- Around line 1-9: The appStringsZhHans map has inconsistent indentation for the
keys 'languageTraditionalChinese' and 'languageJapanese' (4 spaces) compared to
the other entries (6 spaces); update those lines to match the existing 6-space
indentation used throughout appStringsZhHans and run the Dart/Flutter formatter
(dart format) on the localization files to ensure consistent formatting across
all localization maps.
In `@flutter_app/lib/screens/logs_screen.dart`:
- Around line 115-121: The post-frame callback is being scheduled inside the
Consumer builder which can run many times and enqueue duplicate callbacks;
change the auto-scroll scheduling so it only schedules once per frame by
introducing a guard flag (e.g., _autoScrollScheduled) next to existing
_autoScroll and _scrollController or move the logic out of the Consumer into a
lifecycle method like didUpdateWidget / initState or a scroll/listener handler;
ensure WidgetsBinding.instance.addPostFrameCallback(...) is only called when
!_autoScrollScheduled and clear the guard after the callback runs so duplicate
callbacks are not queued.
In `@flutter_app/scripts/_expand_l10n.dart`:
- Around line 24-29: The loop over base.keys contains an unreachable fallback:
change the value assignment in the loop (where variables base, localized, value,
buffer, seen and _escape are used) to remove the redundant "?? key" so it
becomes value = localized[key] ?? base[key];; update the buffer.writeln call to
use that value unchanged and keep seen.add(key) as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9eb07c0-31e1-46b7-820a-0ef05961ee67
📒 Files selected for processing (24)
flutter_app/lib/app.dartflutter_app/lib/l10n/app_localizations.dartflutter_app/lib/l10n/app_strings_en.dartflutter_app/lib/l10n/app_strings_ja.dartflutter_app/lib/l10n/app_strings_zh_hans.dartflutter_app/lib/l10n/app_strings_zh_hant.dartflutter_app/lib/models/ai_provider.dartflutter_app/lib/providers/locale_provider.dartflutter_app/lib/screens/configure_screen.dartflutter_app/lib/screens/dashboard_screen.dartflutter_app/lib/screens/logs_screen.dartflutter_app/lib/screens/node_screen.dartflutter_app/lib/screens/onboarding_screen.dartflutter_app/lib/screens/packages_screen.dartflutter_app/lib/screens/provider_detail_screen.dartflutter_app/lib/screens/providers_screen.dartflutter_app/lib/screens/settings_screen.dartflutter_app/lib/screens/setup_wizard_screen.dartflutter_app/lib/services/preferences_service.dartflutter_app/lib/services/provider_config_service.dartflutter_app/lib/widgets/gateway_controls.dartflutter_app/lib/widgets/node_controls.dartflutter_app/pubspec.yamlflutter_app/scripts/_expand_l10n.dart
Comment on lines
+89
to
+92
| 'dashboardNodeTitle': 'Node', | ||
| 'dashboardNodeConnected': 'Connected to gateway', | ||
| 'dashboardNodeDisabled': 'Device capabilities for AI', | ||
| 'dashboardVersionLabel': 'OpenClaw v{version}', |
There was a problem hiding this comment.
dashboardNodeDisabled no longer matches its usage.
DashboardScreen now uses this key specifically for the disabled-state subtitle, but the current value is the generic node description. Japanese users will see the wrong message whenever the node is disabled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/l10n/app_strings_ja.dart` around lines 89 - 92, The
translation for the key dashboardNodeDisabled is incorrect for its current use
as the disabled-state subtitle; update the value of dashboardNodeDisabled in
app_strings_ja.dart so it conveys the "node is disabled" subtitle (e.g., a
concise Japanese phrase indicating the node is disabled) instead of the generic
device capabilities description, ensuring DashboardScreen shows the correct
disabled-state message.
Comment on lines
+64
to
+69
| try { | ||
| await NativeBridge.setupDirs(); | ||
| } catch (_) {} | ||
| try { | ||
| await NativeBridge.writeResolv(); | ||
| } catch (_) {} |
There was a problem hiding this comment.
Don't swallow native setup failures here.
setupDirs() and writeResolv() are startup prerequisites, but both exceptions are ignored. That leaves the configure flow to continue in a partially prepared state and usually surfaces a much less actionable failure later in _startConfigure().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/configure_screen.dart` around lines 64 - 69, The
current empty catches around NativeBridge.setupDirs() and
NativeBridge.writeResolv() swallow startup errors; change them to surface
failures: catch the exception (and stack) in the try blocks around
NativeBridge.setupDirs and NativeBridge.writeResolv, log the error with context
via your app logger or Flutter's debug/logging utilities, and rethrow or
return/fail so _startConfigure() does not continue in a partially prepared state
(alternatively propagate a specific SetupException so callers can handle/display
the error).
Comment on lines
+76
to
+145
| child: RadioGroup<bool>( | ||
| groupValue: _isLocal, | ||
| onChanged: (value) { | ||
| if (value != null) { | ||
| setState(() => _isLocal = value); | ||
| } | ||
| }, | ||
| child: Column( | ||
| crossAxisAlignment: CrossAxisAlignment.start, | ||
| children: [ | ||
| RadioListTile<bool>( | ||
| title: Text(l10n.t('nodeLocalGateway')), | ||
| subtitle: | ||
| Text(l10n.t('nodeLocalGatewaySubtitle')), | ||
| value: true, | ||
| ), | ||
| const SizedBox(height: 12), | ||
| TextField( | ||
| controller: _portController, | ||
| decoration: const InputDecoration( | ||
| labelText: 'Gateway Port', | ||
| hintText: '18789', | ||
| ), | ||
| keyboardType: TextInputType.number, | ||
| RadioListTile<bool>( | ||
| title: Text(l10n.t('nodeRemoteGateway')), | ||
| subtitle: | ||
| Text(l10n.t('nodeRemoteGatewaySubtitle')), | ||
| value: false, | ||
| ), | ||
| const SizedBox(height: 12), | ||
| TextField( | ||
| controller: _tokenController, | ||
| decoration: const InputDecoration( | ||
| labelText: 'Gateway Token', | ||
| hintText: 'Paste token from gateway dashboard URL', | ||
| helperText: 'Found in dashboard URL after #token=', | ||
| prefixIcon: Icon(Icons.key), | ||
| if (!_isLocal) ...[ | ||
| const SizedBox(height: 12), | ||
| TextField( | ||
| controller: _hostController, | ||
| decoration: InputDecoration( | ||
| labelText: l10n.t('nodeGatewayHost'), | ||
| hintText: '192.168.1.100', | ||
| ), | ||
| ), | ||
| obscureText: true, | ||
| ), | ||
| const SizedBox(height: 12), | ||
| FilledButton.icon( | ||
| onPressed: () { | ||
| final host = _hostController.text.trim(); | ||
| final port = int.tryParse(_portController.text.trim()) ?? 18789; | ||
| final token = _tokenController.text.trim(); | ||
| if (host.isNotEmpty) { | ||
| provider.connectRemote(host, port, | ||
| token: token.isNotEmpty ? token : null); | ||
| } | ||
| }, | ||
| icon: const Icon(Icons.link), | ||
| label: const Text('Connect'), | ||
| ), | ||
| const SizedBox(height: 12), | ||
| TextField( | ||
| controller: _portController, | ||
| decoration: InputDecoration( | ||
| labelText: l10n.t('nodeGatewayPort'), | ||
| hintText: '18789', | ||
| ), | ||
| keyboardType: TextInputType.number, | ||
| ), | ||
| const SizedBox(height: 12), | ||
| TextField( | ||
| controller: _tokenController, | ||
| decoration: InputDecoration( | ||
| labelText: l10n.t('nodeGatewayToken'), | ||
| hintText: l10n.t('nodeGatewayTokenHint'), | ||
| helperText: | ||
| l10n.t('nodeGatewayTokenHelper'), | ||
| prefixIcon: const Icon(Icons.key), | ||
| ), | ||
| obscureText: true, | ||
| ), | ||
| const SizedBox(height: 12), | ||
| FilledButton.icon( | ||
| onPressed: () { | ||
| final host = _hostController.text.trim(); | ||
| final port = int.tryParse( | ||
| _portController.text.trim()) ?? | ||
| 18789; | ||
| final token = _tokenController.text.trim(); | ||
| if (host.isNotEmpty) { | ||
| provider.connectRemote(host, port, | ||
| token: | ||
| token.isNotEmpty ? token : null); | ||
| } | ||
| }, | ||
| icon: const Icon(Icons.link), | ||
| label: Text(l10n.t('nodeConnect')), | ||
| ), | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant implementations =="
for f in \
flutter_app/lib/screens/node_screen.dart \
flutter_app/lib/providers/node_provider.dart \
flutter_app/lib/services/preferences_service.dart; do
echo "----- $f -----"
sed -n '1,260p' "$f"
done
echo
echo "== Gateway connection call sites =="
rg -n -C2 'connectRemote|connectLocal|nodeGatewayHost|nodeGatewayPort|nodeGatewayToken' \
flutter_app/lib/screens/node_screen.dart \
flutter_app/lib/providers/node_provider.dart \
flutter_app/lib/services/preferences_service.dartRepository: mithun50/openclaw-termux
Length of output: 29562
Selecting "Local Gateway" has no effect — no reconnection or persistence.
The remote path calls provider.connectRemote(...) to persist settings and reconnect. Selecting "Local Gateway" only toggles _isLocal and hides the form. The local connection is never saved to preferences or established; if the gateway was previously remote, reopening the screen will reload the remote settings.
Add a connectLocal() method (or trigger the local flow in the existing connect logic) and call it when the user selects the local option.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/node_screen.dart` around lines 76 - 145, The radio
selection only toggles _isLocal but never persists or establishes a local
connection; update the RadioGroup<bool> onChanged handler (and/or create
provider.connectLocal()) so when value == true you call the provider's
local-connect/persist method (e.g., provider.connectLocal() or
provider.connectLocal(save: true)) after setState, mirroring how
provider.connectRemote(host, port, token: ...) is used for remote; ensure any
previous remote settings are cleared/updated so reopening the screen reflects
the local choice (use _hostController/_portController/_tokenController only for
remote).
Comment on lines
+122
to
+129
| '/bin/bash', | ||
| '-lc', | ||
| 'echo "=== OpenClaw Onboarding ===" && ' | ||
| 'echo "Configure your API keys and binding settings." && ' | ||
| 'echo "TIP: Select Loopback (127.0.0.1) when asked for binding!" && ' | ||
| 'echo "" && ' | ||
| 'openclaw onboard; ' | ||
| 'echo "" && echo "Onboarding complete! You can close this screen."', | ||
| 'echo "Configure your API keys and binding settings." && ' | ||
| 'echo "TIP: Select Loopback (127.0.0.1) when asked for binding!" && ' | ||
| 'echo "" && ' | ||
| 'openclaw onboard; ' | ||
| 'echo "" && echo "Onboarding complete! You can close this screen."', |
There was a problem hiding this comment.
Gate the completion banner on a successful openclaw onboard exit.
openclaw onboard; always falls through to the success echo. That banner matches _completionPattern, so an aborted or failed run can still be presented as complete.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/onboarding_screen.dart` around lines 122 - 129, The
shell pipeline in onboarding_screen.dart currently runs 'openclaw onboard;' and
always prints the success banner, which can falsely match _completionPattern;
change the command element so onboarding success is gated on the process exit
status (e.g., replace 'openclaw onboard; echo "" && echo "Onboarding
complete!..."' with a conditional such as 'if openclaw onboard; then echo "" &&
echo "Onboarding complete! You can close this screen."; else echo "" && echo
"Onboarding failed" >&2; fi'), so the success banner is only emitted when the
openclaw command exits with status 0 and thus only matches _completionPattern on
real success. Ensure this updated string is placed in the same command array
element passed in onboarding_screen.dart.
Comment on lines
42
to
+46
| builder: (_) => ProviderDetailScreen( | ||
| provider: provider, | ||
| existingApiKey: providerConfig?['apiKey'] as String?, | ||
| existingModel: _activeModel, | ||
| existingBaseUrl: providerConfig?['baseUrl'] as String?, | ||
| existingModel: providerConfig?['model'] as String? ?? _activeModel, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant implementations =="
for f in \
flutter_app/lib/screens/providers_screen.dart \
flutter_app/lib/screens/provider_detail_screen.dart \
flutter_app/lib/models/ai_provider.dart; do
echo "----- $f -----"
sed -n '1,260p' "$f"
done
echo
echo "== Model fallback / matching logic =="
rg -n -C3 'existingModel|matchesModel' \
flutter_app/lib/screens/providers_screen.dart \
flutter_app/lib/screens/provider_detail_screen.dart \
flutter_app/lib/models/ai_provider.dartRepository: mithun50/openclaw-termux
Length of output: 25589
Scope the active-model fallback to the matching provider.
When a provider has no saved model, existingModel falls back to the global _activeModel. Since ProviderDetailScreen treats unrecognized models as custom, opening Anthropic while an OpenAI model is active will prefill the custom model field with the wrong model name instead of using Anthropic's default.
Use provider.matchesModel(_activeModel) to validate the fallback before passing it:
💡 Safer fallback
- existingModel: providerConfig?['model'] as String? ?? _activeModel,
+ existingModel: providerConfig?['model'] as String? ??
+ (_activeModel != null && provider.matchesModel(_activeModel!)
+ ? _activeModel
+ : null),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/providers_screen.dart` around lines 42 - 46, The
existingModel fallback incorrectly uses the global _activeModel for any
provider; change it so the fallback is only applied if the active model actually
matches the provider by using provider.matchesModel(_activeModel). In other
words, when constructing ProviderDetailScreen (symbols: ProviderDetailScreen,
existingModel, providerConfig, provider.matchesModel, _activeModel), pass
providerConfig?['model'] as String? if present, otherwise pass _activeModel only
when provider.matchesModel(_activeModel) is true; otherwise pass null so
ProviderDetailScreen can use the provider's own default.
Comment on lines
+209
to
+211
| label: state.step == step | ||
| ? _localizedSetupMessage(l10n, state.message) | ||
| : label, |
There was a problem hiding this comment.
Keep the static step label when there is no runtime message.
_localizedSetupMessage returns '' for null/empty input, so the active step renders with a blank label until state.message is populated. Fall back to label when the localized runtime message is empty.
Also applies to: 311-314
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/setup_wizard_screen.dart` around lines 209 - 211,
Active step label becomes blank because _localizedSetupMessage(l10n,
state.message) returns an empty string for null/empty messages; change the
conditional so that when state.step == step you use the localized runtime
message only if it is non-empty, otherwise fall back to the original static
label. Locate the expressions using state.step, step, label and
_localizedSetupMessage (appearing around the current snippet and the similar
block at lines ~311-314) and replace the ternary to check for a non-empty result
(e.g., compute msg = _localizedSetupMessage(...); use msg.isNotEmpty ? msg :
label) so the static label remains until a runtime message is present.
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="flutter_app/lib/app.dart">
<violation number="1" location="flutter_app/lib/app.dart:142">
P2: Replacing `cardTheme` with `cardColor` silently drops the custom card elevation (0), shape (12px border radius + border side), and margin (4px vertical). Cards will fall back to Material 3 defaults, causing a visible styling regression. Restore the full `CardThemeData` if you intend to keep the previous card appearance.</violation>
<violation number="2" location="flutter_app/lib/app.dart:204">
P2: Replacing `dialogTheme` with `dialogBackgroundColor` drops the custom dialog shape (16px border radius + border side), causing dialogs to fall back to Material defaults. Additionally, `ThemeData.dialogBackgroundColor` is deprecated as of Flutter 3.27 in favor of `DialogThemeData.backgroundColor`. Restore the full `DialogThemeData` to keep the styling and stay forward-compatible.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
flutter_app/lib/screens/node_screen.dart (1)
230-235:⚠️ Potential issue | 🟡 MinorMissing localization for Serial capability.
All other capability tiles use localized strings via
l10n.t(...), but the Serial capability still has hard-coded English strings. This is inconsistent and will break the localized UI.🌐 Proposed fix
_capabilityTile( theme, - 'Serial', - 'Bluetooth and USB serial communication', + l10n.t('nodeCapabilitySerialTitle'), + l10n.t('nodeCapabilitySerialSubtitle'), Icons.usb, ),Ensure the corresponding keys are added to all localization files (
app_strings_en.dart,app_strings_ja.dart,app_strings_zh_hans.dart,app_strings_zh_hant.dart).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/node_screen.dart` around lines 230 - 235, Replace the hard-coded "Serial" capability title and description passed to _capabilityTile with localized strings using the same pattern as the other tiles (e.g., use l10n.t(...) for the title and description instead of the literal 'Serial' and 'Bluetooth and USB serial communication'), and add the corresponding localization keys and translated values to all app_strings_* localization files (app_strings_en.dart, app_strings_ja.dart, app_strings_zh_hans.dart, app_strings_zh_hant.dart) so the UI uses the new keys for the Serial capability.flutter_app/lib/screens/settings_screen.dart (1)
423-434:⚠️ Potential issue | 🟡 MinorInclude the selected language in snapshot backup/restore.
The snapshot now carries several persisted settings, but not the locale added in this PR. Exporting from one device and importing on another will silently drop the user’s language choice.
Please add the persisted locale to the snapshot payload and restore it through
LocaleProviderduring import so backup/restore stays aligned with the new settings surface.Also applies to: 485-505
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/settings_screen.dart` around lines 423 - 434, The snapshot payload created in the export (the snapshot map built with keys like 'version', 'timestamp', 'openclawConfig', and various 'node*' prefs) must also include the persisted locale (e.g., add a 'locale' or 'language' key using the stored preference, e.g., _prefs.locale or whatever the app stores). On import (the restore logic that reads that snapshot—see the restore handler around where the snapshot map is consumed), read that locale key and apply it via LocaleProvider (call LocaleProvider.of(context).setLocale(...) or the app's equivalent) so the restored snapshot re-applies the user's language choice. Ensure both export and import code paths are updated so locale is preserved across backup/restore.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter_app/lib/screens/provider_detail_screen.dart`:
- Around line 52-54: The form currently seeds _baseUrlController with
widget.existingBaseUrl ?? widget.provider.baseUrl which causes the provider
default to be treated as a saved override; change initialization to use only
widget.existingBaseUrl (or empty string) so an untouched form doesn't serialize
the provider default. In the save/submit logic (the method handling form
submission, e.g., _onSave/_saveProvider or where baseUrl is persisted) only
persist a custom baseUrl when the user actually entered one: if
_baseUrlController.text.trim().isEmpty then set the saved baseUrl to null (to
mean "use provider default"); otherwise persist the trimmed value. Also ensure
clearing the field is allowed and results in baseUrl being cleared (null) rather
than rejected.
In `@flutter_app/lib/screens/settings_screen.dart`:
- Around line 323-324: The update-check UI in SettingsScreen (the ListTile with
title 'Check for Updates' and subtitle 'Check GitHub for a newer release', plus
the update dialog and snackbars used in the update flow) are hardcoded English
strings; replace those direct strings with calls to the app's localization
getters (l10n.*) and add matching keys to the ARB/localization resources (e.g.,
keys for updateTileTitle, updateTileSubtitle, updateDialogTitle,
updateDialogBody, updateAvailableSnackbar, updateNotAvailableSnackbar,
updateErrorSnackbar). Update the SettingsScreen references to use the new l10n
properties and then add the corresponding entries and translations to the
localization files so the dialog text and snackbars are localized across
supported languages.
---
Outside diff comments:
In `@flutter_app/lib/screens/node_screen.dart`:
- Around line 230-235: Replace the hard-coded "Serial" capability title and
description passed to _capabilityTile with localized strings using the same
pattern as the other tiles (e.g., use l10n.t(...) for the title and description
instead of the literal 'Serial' and 'Bluetooth and USB serial communication'),
and add the corresponding localization keys and translated values to all
app_strings_* localization files (app_strings_en.dart, app_strings_ja.dart,
app_strings_zh_hans.dart, app_strings_zh_hant.dart) so the UI uses the new keys
for the Serial capability.
In `@flutter_app/lib/screens/settings_screen.dart`:
- Around line 423-434: The snapshot payload created in the export (the snapshot
map built with keys like 'version', 'timestamp', 'openclawConfig', and various
'node*' prefs) must also include the persisted locale (e.g., add a 'locale' or
'language' key using the stored preference, e.g., _prefs.locale or whatever the
app stores). On import (the restore logic that reads that snapshot—see the
restore handler around where the snapshot map is consumed), read that locale key
and apply it via LocaleProvider (call LocaleProvider.of(context).setLocale(...)
or the app's equivalent) so the restored snapshot re-applies the user's language
choice. Ensure both export and import code paths are updated so locale is
preserved across backup/restore.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31605551-96ef-45bf-bc67-2fb2c327883f
📒 Files selected for processing (4)
flutter_app/lib/app.dartflutter_app/lib/screens/node_screen.dartflutter_app/lib/screens/provider_detail_screen.dartflutter_app/lib/screens/settings_screen.dart
Comment on lines
+52
to
+54
| _baseUrlController = TextEditingController( | ||
| text: widget.existingBaseUrl ?? widget.provider.baseUrl, | ||
| ); |
There was a problem hiding this comment.
Don't persist the provider default as a custom base URL.
Lines 52-54 and Lines 91-112 currently turn the built-in provider.baseUrl into a saved override. That makes "use the provider default" impossible again: clearing the field is rejected, and untouched forms still serialize a custom baseUrl. If the app updates a provider's default endpoint later, these configs will stay pinned to the old value.
Suggested fix
- _baseUrlController = TextEditingController(
- text: widget.existingBaseUrl ?? widget.provider.baseUrl,
- );
+ _baseUrlController =
+ TextEditingController(text: widget.existingBaseUrl ?? '');
...
- final baseUrl = _baseUrlController.text.trim();
- if (_supportsCustomBaseUrl && !_isValidBaseUrl(baseUrl)) {
+ final rawBaseUrl =
+ _supportsCustomBaseUrl ? _baseUrlController.text.trim() : '';
+ if (rawBaseUrl.isNotEmpty && !_isValidBaseUrl(rawBaseUrl)) {
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(content: Text(l10n.t('providerDetailEndpointInvalid'))),
);
return;
}
+ final baseUrl = rawBaseUrl.isEmpty ? null : rawBaseUrl;
...
await ProviderConfigService.saveProviderConfig(
provider: widget.provider,
apiKey: apiKey,
baseUrl: baseUrl,
model: model,
);Also applies to: 91-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/provider_detail_screen.dart` around lines 52 - 54,
The form currently seeds _baseUrlController with widget.existingBaseUrl ??
widget.provider.baseUrl which causes the provider default to be treated as a
saved override; change initialization to use only widget.existingBaseUrl (or
empty string) so an untouched form doesn't serialize the provider default. In
the save/submit logic (the method handling form submission, e.g.,
_onSave/_saveProvider or where baseUrl is persisted) only persist a custom
baseUrl when the user actually entered one: if
_baseUrlController.text.trim().isEmpty then set the saved baseUrl to null (to
mean "use provider default"); otherwise persist the trimmed value. Also ensure
clearing the field is allowed and results in baseUrl being cleared (null) rather
than rejected.
Comment on lines
323
to
324
| title: const Text('Check for Updates'), | ||
| subtitle: const Text('Check GitHub for a newer release'), |
There was a problem hiding this comment.
Finish localizing the update-check flow.
This screen is otherwise wired to l10n, but the update tile, dialog, and snackbars are still hardcoded in English. Switching languages will leave this path untranslated.
Possible fix
- ListTile(
- title: const Text('Check for Updates'),
- subtitle: const Text('Check GitHub for a newer release'),
+ ListTile(
+ title: Text(l10n.t('settingsCheckForUpdates')),
+ subtitle: Text(l10n.t('settingsCheckForUpdatesSubtitle')),
...
Future<void> _checkForUpdates() async {
+ final l10n = context.l10n;
setState(() => _checkingUpdate = true);
try {
final result = await UpdateService.check();
if (!mounted) return;
if (result.available) {
showDialog(
context: context,
builder: (ctx) => AlertDialog(
- title: const Text('Update Available'),
+ title: Text(l10n.t('settingsUpdateAvailableTitle')),
content: Text(
- 'A new version is available.\n\n'
- 'Current: ${AppConstants.version}\n'
- 'Latest: ${result.latest}',
+ l10n.t('settingsUpdateAvailableBody', {
+ 'current': AppConstants.version,
+ 'latest': result.latest,
+ }),
),
actions: [
TextButton(
onPressed: () => Navigator.pop(ctx),
- child: const Text('Later'),
+ child: Text(l10n.t('commonLater')),
),
FilledButton(
onPressed: () {
Navigator.pop(ctx);
launchUrl(
Uri.parse(result.url),
mode: LaunchMode.externalApplication,
);
},
- child: const Text('Download'),
+ child: Text(l10n.t('commonDownload')),
),
],
),
);
} else {
ScaffoldMessenger.of(context).showSnackBar(
- const SnackBar(content: Text("You're on the latest version")),
+ SnackBar(content: Text(l10n.t('settingsAlreadyLatest'))),
);
}
} catch (_) {
if (!mounted) return;
ScaffoldMessenger.of(context).showSnackBar(
- const SnackBar(content: Text('Could not check for updates')),
+ SnackBar(content: Text(l10n.t('settingsUpdateCheckFailed'))),
);
} finally {Add the corresponding keys to the localization resources as well.
Also applies to: 533-565
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/settings_screen.dart` around lines 323 - 324, The
update-check UI in SettingsScreen (the ListTile with title 'Check for Updates'
and subtitle 'Check GitHub for a newer release', plus the update dialog and
snackbars used in the update flow) are hardcoded English strings; replace those
direct strings with calls to the app's localization getters (l10n.*) and add
matching keys to the ARB/localization resources (e.g., keys for updateTileTitle,
updateTileSubtitle, updateDialogTitle, updateDialogBody,
updateAvailableSnackbar, updateNotAvailableSnackbar, updateErrorSnackbar).
Update the SettingsScreen references to use the new l10n properties and then add
the corresponding entries and translations to the localization files so the
dialog text and snackbars are localized across supported languages.
Owner
|
@CodeRabbit why workflow not using the sign keys. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the workflow files in the repository. 🏁 Script executed: Length of output: 11203
Here's the relevant conditional in - name: Setup signing
if: env.KEYSTORE_BASE64 != '' # ← skipped when secret is absent
env:
KEYSTORE_BASE64: ${{ secrets.KEYSTORE_BASE64 }}
KEYSTORE_PASSWORD: ${{ secrets.KEYSTORE_PASSWORD }}
KEY_ALIAS: ${{ secrets.KEY_ALIAS }}
KEY_PASSWORD: ${{ secrets.KEY_PASSWORD }}When How to fix it — add the four secrets to your repository:
Once all four secrets are present, the signing step will activate automatically on the next workflow run and produce a properly release-signed APK/AAB. (ﾐ･ω･ﾐ) |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
flutter_app/lib/l10n/app_strings_ja.dart (1)
91-91:⚠️ Potential issue | 🟡 Minor
dashboardNodeDisabledis still the wrong Japanese copy.
AI にデバイス機能を提供describes the feature in general, not the disabled state, so Japanese users will still see the wrong subtitle when Node is off.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/l10n/app_strings_ja.dart` at line 91, The string value for the localization key dashboardNodeDisabled is incorrect: it describes the feature rather than the disabled state; update the dashboardNodeDisabled entry in app_strings_ja.dart so the Japanese copy clearly indicates that Node is disabled (e.g., change to a phrase like "Node は無効です" or "Node がオフになっています" or similar contextualized subtitle), ensuring the wording matches the intended disabled/subtitle tone used elsewhere (compare with dashboardNodeEnabled or other related keys for consistency).flutter_app/lib/screens/configure_screen.dart (1)
64-69:⚠️ Potential issue | 🟠 MajorDon't swallow configure prerequisite failures.
If
setupDirs()orwriteResolv()fails here, the screen still continues into a partially prepared proot environment and the user gets a much less actionable failure later. Surface the error and stop startup instead of ignoring it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/configure_screen.dart` around lines 64 - 69, The current empty catch blocks around NativeBridge.setupDirs() and NativeBridge.writeResolv() swallow startup errors; change them to surface and stop startup by either removing the try/catch so exceptions propagate or catching and rethrowing a wrapped descriptive error (e.g., "Failed to setupDirs" / "Failed to writeResolv") after logging; ensure this happens in the configure screen initialization so the screen does not continue into a partially prepared proot environment and the caller can show an error to the user or abort startup.flutter_app/lib/screens/setup_wizard_screen.dart (1)
205-207:⚠️ Potential issue | 🟡 MinorKeep the static step label until a runtime message exists.
_localizedSetupMessage()returns''for null/empty input, so the active step still renders blank untilstate.messageis populated. Fall back tolabelwhen the localized message is empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/setup_wizard_screen.dart` around lines 205 - 207, The active step label is being replaced by an empty string because _localizedSetupMessage(l10n, state.message) returns '' for null/empty messages; update the conditional so that when state.step == step you compute the localized string once (e.g., call _localizedSetupMessage(l10n, state.message)) and fall back to the original label if that localized result is empty (use localized.isEmpty ? label : localized) instead of unconditionally using the localized result.flutter_app/lib/screens/node_screen.dart (1)
79-99:⚠️ Potential issue | 🟠 MajorPersist and reconnect when “Local Gateway” is selected.
This block only flips
_isLocal. It never tellsNodeProviderto switch back to the local gateway or persist that choice, so a previously saved remote config can stay in effect even after the user picks the local option.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/node_screen.dart` around lines 79 - 99, The radio handlers currently only set the local state `_isLocal` but must also instruct the app to switch gateways and persist the choice; update both RadioListTile onChanged callbacks in NodeScreen to (1) call setState to set `_isLocal`, (2) call the NodeProvider API (e.g., NodeProvider.switchToLocal()/setGatewayLocal() or the provider method used to select gateway) to switch the runtime gateway, and (3) persist the selection (e.g., NodeProvider.saveGatewayPreference(...) or the provider’s persist method) and trigger a reconnect (e.g., NodeProvider.reconnect() or equivalent). Ensure you await async provider calls or handle futures and propagate errors to the UI.flutter_app/lib/screens/onboarding_screen.dart (2)
86-91:⚠️ Potential issue | 🟠 MajorDon't swallow onboarding prerequisite failures.
If
setupDirs()orwriteResolv()fails, onboarding still continues in a partially prepared environment and the eventual error becomes much harder to diagnose. Surface the failure and stop startup here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/onboarding_screen.dart` around lines 86 - 91, Onboarding currently swallows failures from NativeBridge.setupDirs() and NativeBridge.writeResolv(), so change the empty catches to surface the error and stop startup: either remove the try/catch so exceptions propagate, or catch (e, s) and log the error (e.g., using your logger or debugPrint) and then rethrow (or return an error state so onboarding halts). Update the calls to NativeBridge.setupDirs and NativeBridge.writeResolv in OnboardingScreen to ensure failures are not ignored and the onboarding flow stops when those methods fail.
124-129:⚠️ Potential issue | 🟠 MajorOnly emit the onboarding success banner on exit code 0.
With
openclaw onboard;the success text is still printed on failure, and that output matches_completionPattern, so aborted runs can still surface the completion CTA.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/screens/onboarding_screen.dart` around lines 124 - 129, The onboarding command string currently prints the success banner regardless of exit status (so outputs match _completionPattern even on failure); change the shell sequence used where the command is built (the multiline string containing 'openclaw onboard' and the success banner) to only emit the success text on a zero exit code — e.g., run the onboarding command with a conditional (use && before the banner or wrap in if ...; then ...; fi) so the success echo is executed only when openclaw onboard exits 0, leaving _completionPattern semantics unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter_app/lib/l10n/app_strings_en.dart`:
- Line 99: Update the value for the string identifier dashboardNodeDisabled so
it explicitly describes the disabled state (e.g., mention that device AI
capabilities are disabled or turned off) rather than the generic node
description; change the base English string in app_strings_en.dart and ensure
all translated bundles use the same updated meaning so subtitles remain
consistent when the Node is off.
In `@flutter_app/lib/screens/configure_screen.dart`:
- Around line 98-102: The current shell command string always prints the
"Configuration complete" banner because 'openclaw configure;' unconditionally
falls through; modify the command assembly in configure_screen.dart so that it
checks the exit status of 'openclaw configure' and only prints the success echo
when the command exits 0, and print a failure/aborted message (or propagate
failure) when non-zero or terminated; locate where the multiline string
containing 'openclaw configure;' is built and replace the simple sequential
echos with a conditional check on the command's exit code (or use '&&' / '||'
logic) so only a successful run yields the completion banner and non-zero exits
show an error/abort banner instead.
---
Duplicate comments:
In `@flutter_app/lib/l10n/app_strings_ja.dart`:
- Line 91: The string value for the localization key dashboardNodeDisabled is
incorrect: it describes the feature rather than the disabled state; update the
dashboardNodeDisabled entry in app_strings_ja.dart so the Japanese copy clearly
indicates that Node is disabled (e.g., change to a phrase like "Node は無効です" or
"Node がオフになっています" or similar contextualized subtitle), ensuring the wording
matches the intended disabled/subtitle tone used elsewhere (compare with
dashboardNodeEnabled or other related keys for consistency).
In `@flutter_app/lib/screens/configure_screen.dart`:
- Around line 64-69: The current empty catch blocks around
NativeBridge.setupDirs() and NativeBridge.writeResolv() swallow startup errors;
change them to surface and stop startup by either removing the try/catch so
exceptions propagate or catching and rethrowing a wrapped descriptive error
(e.g., "Failed to setupDirs" / "Failed to writeResolv") after logging; ensure
this happens in the configure screen initialization so the screen does not
continue into a partially prepared proot environment and the caller can show an
error to the user or abort startup.
In `@flutter_app/lib/screens/node_screen.dart`:
- Around line 79-99: The radio handlers currently only set the local state
`_isLocal` but must also instruct the app to switch gateways and persist the
choice; update both RadioListTile onChanged callbacks in NodeScreen to (1) call
setState to set `_isLocal`, (2) call the NodeProvider API (e.g.,
NodeProvider.switchToLocal()/setGatewayLocal() or the provider method used to
select gateway) to switch the runtime gateway, and (3) persist the selection
(e.g., NodeProvider.saveGatewayPreference(...) or the provider’s persist method)
and trigger a reconnect (e.g., NodeProvider.reconnect() or equivalent). Ensure
you await async provider calls or handle futures and propagate errors to the UI.
In `@flutter_app/lib/screens/onboarding_screen.dart`:
- Around line 86-91: Onboarding currently swallows failures from
NativeBridge.setupDirs() and NativeBridge.writeResolv(), so change the empty
catches to surface the error and stop startup: either remove the try/catch so
exceptions propagate, or catch (e, s) and log the error (e.g., using your logger
or debugPrint) and then rethrow (or return an error state so onboarding halts).
Update the calls to NativeBridge.setupDirs and NativeBridge.writeResolv in
OnboardingScreen to ensure failures are not ignored and the onboarding flow
stops when those methods fail.
- Around line 124-129: The onboarding command string currently prints the
success banner regardless of exit status (so outputs match _completionPattern
even on failure); change the shell sequence used where the command is built (the
multiline string containing 'openclaw onboard' and the success banner) to only
emit the success text on a zero exit code — e.g., run the onboarding command
with a conditional (use && before the banner or wrap in if ...; then ...; fi) so
the success echo is executed only when openclaw onboard exits 0, leaving
_completionPattern semantics unchanged.
In `@flutter_app/lib/screens/setup_wizard_screen.dart`:
- Around line 205-207: The active step label is being replaced by an empty
string because _localizedSetupMessage(l10n, state.message) returns '' for
null/empty messages; update the conditional so that when state.step == step you
compute the localized string once (e.g., call _localizedSetupMessage(l10n,
state.message)) and fall back to the original label if that localized result is
empty (use localized.isEmpty ? label : localized) instead of unconditionally
using the localized result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a454d709-e5c8-4aaf-a0bd-6b6385409700
📒 Files selected for processing (9)
flutter_app/lib/app.dartflutter_app/lib/l10n/app_strings_en.dartflutter_app/lib/l10n/app_strings_ja.dartflutter_app/lib/l10n/app_strings_zh_hans.dartflutter_app/lib/l10n/app_strings_zh_hant.dartflutter_app/lib/screens/configure_screen.dartflutter_app/lib/screens/node_screen.dartflutter_app/lib/screens/onboarding_screen.dartflutter_app/lib/screens/setup_wizard_screen.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- flutter_app/lib/l10n/app_strings_zh_hant.dart
- flutter_app/lib/l10n/app_strings_zh_hans.dart
| 'dashboardSnapshotSubtitle': 'Backup or restore your config', | ||
| 'dashboardNodeTitle': 'Node', | ||
| 'dashboardNodeConnected': 'Connected to gateway', | ||
| 'dashboardNodeDisabled': 'Device capabilities for AI', |
There was a problem hiding this comment.
dashboardNodeDisabled should describe the disabled state.
This value reads like the generic node description, not a disabled-state subtitle, so the dashboard copy will be misleading when Node is off. Please update this base string and keep the translated bundles aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/l10n/app_strings_en.dart` at line 99, Update the value for
the string identifier dashboardNodeDisabled so it explicitly describes the
disabled state (e.g., mention that device AI capabilities are disabled or turned
off) rather than the generic node description; change the base English string in
app_strings_en.dart and ensure all translated bundles use the same updated
meaning so subtitles remain consistent when the Node is off.
Comment on lines
98
to
+102
| 'echo "=== OpenClaw Configure ===" && ' | ||
| 'echo "Manage your gateway settings." && ' | ||
| 'echo "" && ' | ||
| 'openclaw configure; ' | ||
| 'echo "" && echo "Configuration complete! You can close this screen."', | ||
| 'echo "Manage your gateway settings." && ' | ||
| 'echo "" && ' | ||
| 'openclaw configure; ' | ||
| 'echo "" && echo "Configuration complete! You can close this screen."', |
There was a problem hiding this comment.
Only show the completion banner after a successful configure run.
openclaw configure; always falls through to the success echo. Since this screen also treats process exit as "finished", non-zero exits and user aborts still look like successful configuration.
Suggested fix
- 'echo "=== OpenClaw Configure ===" && '
- 'echo "Manage your gateway settings." && '
- 'echo "" && '
- 'openclaw configure; '
- 'echo "" && echo "Configuration complete! You can close this screen."',
+ 'echo "=== OpenClaw Configure ===" && '
+ 'echo "Manage your gateway settings." && '
+ 'echo "" && '
+ 'if openclaw configure; then '
+ ' echo "" && echo "Configuration complete! You can close this screen."; '
+ 'else '
+ ' echo "" && echo "Configuration failed." >&2; '
+ ' exit 1; '
+ 'fi',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 'echo "=== OpenClaw Configure ===" && ' | |
| 'echo "Manage your gateway settings." && ' | |
| 'echo "" && ' | |
| 'openclaw configure; ' | |
| 'echo "" && echo "Configuration complete! You can close this screen."', | |
| 'echo "Manage your gateway settings." && ' | |
| 'echo "" && ' | |
| 'openclaw configure; ' | |
| 'echo "" && echo "Configuration complete! You can close this screen."', | |
| 'echo "=== OpenClaw Configure ===" && ' | |
| 'echo "Manage your gateway settings." && ' | |
| 'echo "" && ' | |
| 'if openclaw configure; then ' | |
| ' echo "" && echo "Configuration complete! You can close this screen."; ' | |
| 'else ' | |
| ' echo "" && echo "Configuration failed." >&2; ' | |
| ' exit 1; ' | |
| 'fi', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/screens/configure_screen.dart` around lines 98 - 102, The
current shell command string always prints the "Configuration complete" banner
because 'openclaw configure;' unconditionally falls through; modify the command
assembly in configure_screen.dart so that it checks the exit status of 'openclaw
configure' and only prints the success echo when the command exits 0, and print
a failure/aborted message (or propagate failure) when non-zero or terminated;
locate where the multiline string containing 'openclaw configure;' is built and
replace the simple sequential echos with a conditional check on the command's
exit code (or use '&&' / '||' logic) so only a successful run yields the
completion banner and non-zero exits show an error/abort banner instead.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter_app/lib/l10n/app_strings_zh_hant.dart`:
- Around line 81-82: Choose a single Traditional Chinese term set for
"package/component" and "uninstall/remove" and make the strings consistent:
update the values for 'dashboardPackagesTitle' and 'dashboardPackagesSubtitle'
and the related keys around the later package section (the strings that
currently mix 組件/元件 and 解除安裝/卸載) to use the chosen vocabulary throughout (e.g.,
replace all occurrences to use 套件 + 卸載 or 元件 + 解除安裝), keeping the original keys
intact so only the localized text changes.
- Line 20: The translation for the shared key commonSaveFailed is
screenshot-specific; change its value from '截圖失敗' to a generic phrase such as
'儲存失敗' so the key stays reusable across flows (update the entry for
commonSaveFailed in app_strings_zh_hant.dart to the generic value). If you
prefer a screenshot-specific message instead, rename the key to something like
commonSaveFailedScreenshot and update all references to that new key throughout
the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ba98103-4162-4ab6-91c3-76f9bc2a36fa
📒 Files selected for processing (2)
flutter_app/lib/l10n/app_strings_zh_hant.dartflutter_app/lib/screens/dashboard_screen.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- flutter_app/lib/screens/dashboard_screen.dart
Comment on lines
+81
to
+82
| 'dashboardPackagesTitle': '可選組件', | ||
| 'dashboardPackagesSubtitle': '安裝可選工具（Go、Homebrew、SSH）', |
There was a problem hiding this comment.
Normalize zh-Hant terminology in the packages section.
This locale mixes 組件/元件 and 解除安裝/卸載 in the same feature area. That reads inconsistently in Traditional Chinese. Pick one vocabulary set and use it throughout this section.
Suggested fix
- 'dashboardPackagesTitle': '可選組件',
+ 'dashboardPackagesTitle': '可選元件',
...
'packagesTitle': '可選元件',
...
'packagesUninstall': '解除安裝',
- 'packagesUninstallTitle': '卸載 {name}？',
+ 'packagesUninstallTitle': '解除安裝 {name}？',Also applies to: 113-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/l10n/app_strings_zh_hant.dart` around lines 81 - 82, Choose a
single Traditional Chinese term set for "package/component" and
"uninstall/remove" and make the strings consistent: update the values for
'dashboardPackagesTitle' and 'dashboardPackagesSubtitle' and the related keys
around the later package section (the strings that currently mix 組件/元件 and
解除安裝/卸載) to use the chosen vocabulary throughout (e.g., replace all occurrences
to use 套件 + 卸載 or 元件 + 解除安裝), keeping the original keys intact so only the
localized text changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter_app/lib/l10n/app_strings_zh_hans.dart`:
- Line 20: The translation for the shared key commonSaveFailed is too specific
("截图失败"); change its zh-Hans value to a generic "保存失败" to match shared usage, or
if the key is meant only for screenshots, rename the key to a
screenshot-specific identifier (e.g., screenshotSaveFailed) and update all
usages accordingly; locate the key commonSaveFailed in app_strings_zh_hans.dart
and apply the appropriate change plus any corresponding code references.
- Line 120: The current Chinese text for the resource key
'packageBrewDescription' incorrectly calls Homebrew the default Linux package
manager; update the value for packageBrewDescription to a neutral phrasing that
states Homebrew is an optional/common package manager on Linux (e.g., mention
"可选" or "常用的可选包管理器" or "适用于 Linux 的常用包管理工具") so it doesn't imply it's the distro
default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e200914a-8384-4872-b653-09df83bb914f
📒 Files selected for processing (3)
flutter_app/lib/l10n/app_strings_en.dartflutter_app/lib/l10n/app_strings_ja.dartflutter_app/lib/l10n/app_strings_zh_hans.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- flutter_app/lib/l10n/app_strings_en.dart
- flutter_app/lib/l10n/app_strings_ja.dart
… and Traditional Chinese
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR brings in the translation/i18n work (including Simplified Chinese, Traditional Chinese, and Japanese) and related text wiring across the app.
Also, important context:
in commit
d17812d232d90937359b7e743bc7f20884cb9b43, I removed some adaptation changes that were only for my own fork/environment.Why those changes were removed
To avoid polluting the upstream repository with fork-specific adaptations, I intentionally trimmed those parts from this PR.
Those adaptations introduced dependency/build-chain differences.
If kept in this PR, the branch was not runnable in the target setup, so I kept this PR focused on translation-only changes.
Known local build issue
On my machine, this branch can fail to build with Flutter dependency validation errors, mainly:
8.3.0, Flutter warns to move to>= 8.7.0)8.1.0, Flutter requires at least8.1.1)dev.flutter.flutter-gradle-pluginfails duringassembleDebugBecause of that, this branch is not guaranteed to build on my local environment unless I roll back to:
d17812d232d90937359b7e743bc7f20884cb9b43Where the runnable version is
The version that runs correctly in my local environment is available in my
mainbranch (personal fork), where the required dependency/build adaptations are kept.Notes for reviewers
d17812d....Summary by cubic
Adds full app-wide localization (English, Simplified Chinese, Traditional Chinese, Japanese) with a persisted language setting and localized provider metadata. Also adds optional custom base URLs in provider config and updates UI APIs for Flutter 3.24.
New Features
AppLocalizationsandflutter_localizations; language choice persists viaLocaleProviderandPreferencesService.ProviderConfigServicewrites it when set.Bug Fixes
Written for commit c7a96ea. Summary will update on new commits.
Summary by CodeRabbit
New Features
Updates