proposal: enhance AI functionalities #1040
Conversation
📝 WalkthroughWalkthroughThis pull request introduces comprehensive AI assistance features including context management, safety utilities for redaction and risk classification, UI components for AI interactions (global FAB overlay and assist sheet), and integrates AI-assisted workflows across multiple container, system, and SSH pages. It also adds persistent AI history and FAB position settings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AiFabOverlay
participant AiContextProvider
participant AiAssistSheet
participant AskAiRepository
participant OpenAI as OpenAI API
participant AiSafety
User->>AiFabOverlay: Tap FAB
AiFabOverlay->>AiContextProvider: Read current context
AiContextProvider-->>AiFabOverlay: Return AiContextSnapshot
AiFabOverlay->>AiAssistSheet: Show with scenario & context blocks
AiAssistSheet->>AiSafety: Redact sensitive blocks
AiSafety-->>AiAssistSheet: Redacted blocks
User->>AiAssistSheet: Enter prompt
AiAssistSheet->>AskAiRepository: ask(scenario, contextBlocks)
AskAiRepository->>OpenAI: Stream request with scenario prompt
OpenAI-->>AskAiRepository: Stream responses (delta, tool_call)
AskAiRepository-->>AiAssistSheet: AskAiEvent stream
AiAssistSheet->>AiSafety: classifyRisk(command)
AiSafety-->>AiAssistSheet: AiCommandRisk
AiAssistSheet->>AiAssistSheet: Risk confirmation if needed
User->>AiAssistSheet: Apply command
AiAssistSheet->>User: Execute callback (insert/openSsh/copy)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Comment |
Summary of ChangesHello @lollipopkit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the application's AI capabilities by introducing a more structured, context-aware, and secure AI assistance framework. It provides a unified UI for AI interactions, implements robust safety mechanisms for command suggestions, and integrates AI support into critical operational workflows, aiming to deliver a more intelligent and helpful user experience for server management. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed enhancement to the app's AI functionalities. The changes are impressive, focusing on making the AI assistance more context-aware, modular, and, most importantly, safe. Key additions include a global AI floating action button, a generic AI assistant sheet, context providers, and a robust safety layer for redacting sensitive data and classifying command risk. The introduction of unit tests for the safety features is also a great practice. I've included a few suggestions to further improve robustness and user experience consistency.
|
|
||
| final _rxRmRf = RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*r[a-z-]*f[a-z-]*\b'); | ||
|
|
||
| final _rxChmodChownRoot = RegExp(r'\b(chmod|chown)\b[^\n\r]*\s-\w*r\w*\b[^\n\r]*\s/\b'); |
There was a problem hiding this comment.
The current regex for detecting recursive chmod/chown on the root directory is a good start but doesn't handle the long-form --recursive flag. This could cause a high-risk command to be missed by the risk classifier. Adding an alternative for --recursive will make the detection more robust.
| final _rxChmodChownRoot = RegExp(r'\b(chmod|chown)\b[^\n\r]*\s-\w*r\w*\b[^\n\r]*\s/\b'); | |
| final _rxChmodChownRoot = RegExp(r'\b(chmod|chown)\b[^\n\r]*\s(-\w*r\w*|--recursive)\b[^\n\r]*\s/\b'); |
| List<String>? prechecks; | ||
| final preRaw = decoded['prechecks']; | ||
| if (preRaw is List) { | ||
| prechecks = preRaw.map((e) => e.toString()).where((e) => e.trim().isNotEmpty).toList(); | ||
| } |
There was a problem hiding this comment.
The current logic for parsing prechecks converts every item in the list to a string using .toString(). This is a bit too permissive and could hide potential issues if the AI model returns a non-string type (e.g., a number) in the list. It's safer to filter for items that are already strings to ensure type safety.
| List<String>? prechecks; | |
| final preRaw = decoded['prechecks']; | |
| if (preRaw is List) { | |
| prechecks = preRaw.map((e) => e.toString()).where((e) => e.trim().isNotEmpty).toList(); | |
| } | |
| List<String>? prechecks; | |
| final preRaw = decoded['prechecks']; | |
| if (preRaw is List) { | |
| prechecks = preRaw.whereType<String>().where((e) => e.trim().isNotEmpty).toList(); | |
| } |
| fromObj: (val) => List<Map<String, dynamic>>.from( | ||
| (val as List).map((e) => Map<String, dynamic>.from(e as Map)), | ||
| ), |
There was a problem hiding this comment.
The current implementation of fromObj uses as for casting, which is unsafe and can lead to runtime errors if the data from storage is not in the exact expected format. Using type checks like is and whereType will make this data parsing more robust and prevent potential crashes.
fromObj: (val) {
if (val is! List) return const [];
return val.whereType<Map>().map((e) => Map<String, dynamic>.from(e)).toList();
},| icon: const Icon(Icons.smart_toy_outlined), | ||
| tooltip: context.l10n.askAi, | ||
| onPressed: () { | ||
| final args = SshPageArgs(spi: si.spi); | ||
| SSHPage.route.go(context, args); | ||
| }, | ||
| ), |
There was a problem hiding this comment.
This "Ask AI" button navigates to the SSH page, which is inconsistent with other AI entry points in the app that directly open the AI assistant sheet with relevant context. For a better and more consistent user experience, this button should open the AiAssistSheet with general server status information as context.
Note: You will need to add the following imports to the file:
import 'package:server_box/data/provider/ai/ask_ai.dart';
import 'package:server_box/view/widget/ai/ai_assist_sheet.dart'; icon: const Icon(Icons.smart_toy_outlined),
tooltip: context.l10n.askAi,
onPressed: () {
final serverState = ref.read(serverProvider(si.spi.id));
final status = serverState.status;
final blocks = [
'[Server Info]\nName: ${si.spi.name}\nOS: ${status.osPrettyName}\nUptime: ${status.uptime}',
'[CPU Usage]\n${status.cpu.usedPercent(coreIdx: 0).toStringAsFixed(1)}%',
'[Memory Usage]\n${(status.mem.usedPercent * 100).toStringAsFixed(1)}%',
];
showAiAssistSheet(
context,
AiAssistArgs(
title: context.l10n.askAi,
contextBlocks: blocks,
scenario: AskAiScenario.general,
applyLabel: libL10n.ok,
applyBehavior: AiApplyBehavior.openSsh,
onOpenSsh: (cmd) {
final args = SshPageArgs(spi: si.spi, initCmd: cmd);
SSHPage.route.go(context, args);
},
),
);
},| final inserted = _seenCommands.add(event.command.command); | ||
| if (inserted) { | ||
| final risk = event.command.risk != null | ||
| ? (AiCommandRiskX.tryParse(event.command.risk) ?? AiSafety.classifyRisk(event.command.command)) | ||
| : AiSafety.classifyRisk(event.command.command); | ||
| _chatEntries.add(_ChatEntry.command(event.command, risk)); | ||
| shouldScroll = true; | ||
| } |
There was a problem hiding this comment.
The logic for determining the risk of a command is duplicated here and again in the AskAiCompleted event handler (lines 191-199). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, this logic should be extracted into a private helper method within the _AiAssistSheetState class.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| final _rxDdToBlockDevice = RegExp(r'\bdd\b[^\n\r]*\bof\s*=\s*/dev/'); | ||
|
|
||
| final _rxRmRf = RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*r[a-z-]*f[a-z-]*\b'); |
There was a problem hiding this comment.
🔴 Risk classification regex fails to detect rm -fr command variant
The _rxRmRf regex pattern in ai_safety.dart fails to detect the common rm -fr command variant (where -f comes before -r), allowing it to be classified as low risk instead of high risk.
Click to expand
Analysis
The regex pattern at line 166 is:
final _rxRmRf = RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*r[a-z-]*f[a-z-]*\b');This pattern requires r to appear before f in the flags ([a-z-]*r[a-z-]*f[a-z-]*).
Commands that match (high risk detected):
rm -rf /✓rm -rfi /✓
Commands that DON'T match (incorrectly classified as low risk):
rm -fr /✗rm -fR /✗ (though case is lowered,-frorder still fails)
Impact
rm -fr is equally common and dangerous as rm -rf. Users could execute a destructive command suggested by AI without the appropriate high-risk countdown confirmation dialog, since AiSafety.classifyRisk('rm -fr /') returns AiCommandRisk.low instead of AiCommandRisk.high.
Recommendation
Modify the regex to match either flag order, e.g.:
final _rxRmRf = RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*([rf][a-z-]*[rf])[a-z-]*\b');Or use two patterns:
final _rxRmRf = RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*(rf|fr)[a-z-]*\b');Recommendation: Change the regex to detect both rm -rf and rm -fr variants: RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*(rf|fr)[a-z-]*\b')
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
lib/data/res/store.dart (1)
14-47:⚠️ Potential issue | 🟡 MinorAdd
@lollipopkitto theparticipantsfield inlib/data/res/github_id.dart.
Issue#1038was filed by@lollipopkit, but this contributor is not listed in theparticipantsset.
🤖 Fix all issues with AI agents
In `@lib/data/provider/ai/ai_safety.dart`:
- Line 166: The _rxRmRf regex only matches when the 'r' flag appears before 'f'
(e.g., "-rf"), so update detection to handle any flag ordering by either
creating two regexes (e.g., _rxRmRecursive and _rxRmForce) that independently
detect recursive and force flags and then modify classifyRisk to treat a string
as high risk when both patterns match, or change _rxRmRf to a pattern that
asserts presence of both flags in any order; reference _rxRmRf and classifyRisk
when applying the fix.
In `@lib/view/widget/ai/ai_fab_overlay.dart`:
- Around line 24-36: The restored FAB position in didChangeDependencies can land
off-screen; update the logic that computes _offsetPx to clamp the computed
Offset into the visible bounds: obtain MediaQuery.of(context).size and
MediaQuery.of(context).padding (or viewInsets as needed), compute min/max X and
Y allowed (e.g. between left padding and width - right padding, and top padding
and height - bottom padding, optionally considering the FAB size), then clamp
Stores.setting.aiFabOffsetX.fetch() and aiFabOffsetY.fetch() into [0,1] as
before but map them into those constrained pixel ranges before assigning
_offsetPx so the FloatingActionButton stays on-screen.
🧹 Nitpick comments (4)
lib/app.dart (1)
112-115: Guard against a nullchildin the builder.
MaterialApp.buildercan receive a null child; using a safe fallback avoids a potential NPE in the overlay wrapper.Suggested tweak
- final responsiveChild = ResponsivePoints.builder(context, child); - return AiFabOverlay(child: responsiveChild); + final safeChild = child ?? const SizedBox.shrink(); + final responsiveChild = ResponsivePoints.builder(context, safeChild); + return AiFabOverlay(child: responsiveChild);lib/data/provider/ai/ai_context.dart (2)
14-16: MutableList<String>in@immutableclass can break immutability guarantees.The
blocksfield is a mutableList<String>. Callers could mutate it after construction, breaking the immutability contract. Consider usingList<String>.unmodifiable()in the constructor or storing asIListfromfast_immutable_collections.♻️ Option: Defensive copy in constructor
const AiContextSnapshot({ required this.title, required this.scenario, - required this.blocks, + required List<String> blocks, this.spiId, this.updatedAtMs, -}); +}) : blocks = blocks; +// Or for true immutability: +AiContextSnapshot({ + required this.title, + required this.scenario, + required List<String> blocks, + this.spiId, + this.updatedAtMs, +}) : blocks = List.unmodifiable(blocks);
37-37: Consider usingriverpod_annotationfor consistency.Based on learnings, the codebase prefers Riverpod with code generation. This provider uses the manual approach. If the rest of the codebase uses
@riverpodannotations, consider aligning for consistency.lib/view/widget/ai/ai_assist_sheet.dart (1)
541-544: Minor: TrailingSizedBoxafter last context block.The loop adds spacing after each block including the last one. This creates 8px extra padding before the column ends. Not a bug, but could be refined if pixel-perfect layout is desired.
♻️ Optional: Remove trailing spacer
child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - for (final block in widget.args.contextBlocks) ...[ - SelectableText(block, style: const TextStyle(fontFamily: 'monospace')), - const SizedBox(height: 8), - ], + for (var i = 0; i < widget.args.contextBlocks.length; i++) ...[ + SelectableText(widget.args.contextBlocks[i], style: const TextStyle(fontFamily: 'monospace')), + if (i < widget.args.contextBlocks.length - 1) const SizedBox(height: 8), + ], ], ),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
lib/app.dartlib/data/model/ai/ask_ai_models.dartlib/data/model/app/menu/container.dartlib/data/provider/ai/ai_context.dartlib/data/provider/ai/ai_safety.dartlib/data/provider/ai/ask_ai.dartlib/data/res/store.dartlib/data/store/ai_history.dartlib/data/store/setting.dartlib/view/page/container/actions.dartlib/view/page/container/container.dartlib/view/page/server/detail/view.dartlib/view/page/ssh/page/ask_ai.dartlib/view/page/ssh/page/page.dartlib/view/page/systemd.dartlib/view/widget/ai/ai_assist_sheet.dartlib/view/widget/ai/ai_fab_overlay.darttest/ai_safety_test.dart
🧰 Additional context used
📓 Path-based instructions (4)
lib/data/model/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
After modifying annotated model files, run: dart run build_runner build --delete-conflicting-outputs
Files:
lib/data/model/ai/ask_ai_models.dartlib/data/model/app/menu/container.dart
lib/view/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/view/**/*.dart: Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Prefer using libL10n strings before adding new ones to project l10n
Split UI into build, actions, and utils; use extension on to separate concerns
Files:
lib/view/page/container/container.dartlib/view/page/ssh/page/page.dartlib/view/widget/ai/ai_fab_overlay.dartlib/view/page/systemd.dartlib/view/widget/ai/ai_assist_sheet.dartlib/view/page/server/detail/view.dartlib/view/page/container/actions.dartlib/view/page/ssh/page/ask_ai.dart
lib/data/provider/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under lib/data/provider/
Files:
lib/data/provider/ai/ai_context.dartlib/data/provider/ai/ai_safety.dartlib/data/provider/ai/ask_ai.dart
lib/data/store/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Persist state using Hive stores placed under lib/data/store/
Files:
lib/data/store/ai_history.dartlib/data/store/setting.dart
🧠 Learnings (14)
📓 Common learnings
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-22T05:10:54.502Z
Learning: For all future pull requests in the flutter_server_box repository, check if participants from issues resolved by the PR are added to the `participants` field in `lib/data/res/github_id.dart`. If any participants are missing, propose adding them. First verify if they already exist or have been added in the PR commits.
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/container/**/*.dart : Place container/Docker models under lib/data/model/container/
Applied to files:
lib/view/page/container/container.dartlib/view/page/ssh/page/page.dartlib/data/model/app/menu/container.dartlib/view/page/container/actions.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/server/**/*.dart : Place server management models under lib/data/model/server/
Applied to files:
lib/view/page/container/container.dartlib/view/page/ssh/page/page.dartlib/app.dartlib/data/res/store.dartlib/view/page/server/detail/view.dart
📚 Learning: 2026-01-17T04:24:16.501Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
Applied to files:
lib/view/page/container/container.dartlib/view/page/ssh/page/page.dartlib/view/page/systemd.dartlib/view/page/server/detail/view.dartlib/view/page/container/actions.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/ssh/**/*.dart : Place SSH models under lib/data/model/ssh/
Applied to files:
lib/view/page/container/container.dartlib/view/page/ssh/page/page.dartlib/view/page/server/detail/view.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Split UI into build, actions, and utils; use extension on to separate concerns
Applied to files:
lib/view/page/container/container.dartlib/app.dartlib/view/page/server/detail/view.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/sftp/**/*.dart : Place SFTP models under lib/data/model/sftp/
Applied to files:
lib/view/page/ssh/page/page.dartlib/view/page/server/detail/view.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Prefer using libL10n strings before adding new ones to project l10n
Applied to files:
lib/view/page/ssh/page/page.dartlib/app.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/view/**/*.dart : Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Applied to files:
lib/view/page/ssh/page/page.dartlib/view/widget/ai/ai_fab_overlay.dartlib/app.dartlib/view/widget/ai/ai_assist_sheet.dartlib/view/page/server/detail/view.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/provider/**/*.dart : Organize Riverpod providers by feature under lib/data/provider/
Applied to files:
lib/data/provider/ai/ai_context.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Use Riverpod with code generation (riverpod_annotation) for state management
Applied to files:
lib/data/provider/ai/ai_context.dart
📚 Learning: 2026-01-29T08:50:01.572Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:360-363
Timestamp: 2026-01-29T08:50:01.572Z
Learning: In lib/data/provider/container.dart, within the _buildSudoCmd function, do not redirect stderr at the shell level (e.g., 2>/dev/null) when piping the password to sudo -S. Shell-level suppression can swallow stderr before the SSH client receives it, breaking onStderr callbacks in execWithPwd that detect sudo password errors (such as "Sorry, try again.") and Podman emulation messages. Instead, preserve the shell's stderr, receive it via the SSH-level onStderr callback, and filter it from the final result at the application level by using stderr: false in execWithPwd.
Applied to files:
lib/data/provider/ai/ai_context.dartlib/data/provider/ai/ai_safety.dartlib/data/provider/ai/ask_ai.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/store/**/*.dart : Persist state using Hive stores placed under lib/data/store/
Applied to files:
lib/data/store/ai_history.dartlib/data/res/store.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Use dependency injection via GetIt for services (Stores, Services, etc.)
Applied to files:
lib/data/res/store.dart
🔇 Additional comments (24)
lib/data/model/ai/ask_ai_models.dart (1)
29-53: Run build_runner if this model is codegen-backed.New fields look good. Please confirm whether this model participates in code generation; if yes, regenerate outputs to keep them in sync.
As per coding guidelines:
lib/data/model/**/*.dart: After modifying annotated model files, run: dart run build_runner build --delete-conflicting-outputs.lib/data/model/app/menu/container.dart (1)
5-50: LGTM: askAi menu option is fully wired.Enum value, menu lists, icon, and label mapping are consistent.
lib/data/provider/ai/ask_ai.dart (4)
212-239: Prompt + context block assembly looks consistent.
265-283: Tool-call schema and parsing updates are consistent.Also applies to: 357-376
291-335: Scenario prompt mapping and parsing extension look solid.
23-26: All call sites already updated correctly.The only
ask()call site found in the codebase (lib/view/widget/ai/ai_assist_sheet.dart:143) is already updated with both required arguments:scenarioandcontextBlocks. No further updates needed.Likely an incorrect or invalid review comment.
lib/view/page/container/container.dart (1)
15-20: Imports align with AI assist integration.lib/view/page/ssh/page/page.dart (1)
19-28: Imports for AiSafety/AiAssistSheet look fine.lib/data/store/setting.dart (1)
168-170: Defaults for AI FAB offsets look good.lib/app.dart (1)
109-115: Manually verify all issue#1038participants are included inlib/data/res/github_id.dart.The file was added in this PR with a participants set containing 136 entries. However, without access to issue
#1038's full participant data, you must manually confirm that all issue participants (author, assignees, and commenters) are present in the list.lib/view/page/server/detail/view.dart (1)
128-136: AI entry point looks good.
Localized tooltip + SSH navigation is wired cleanly.lib/view/page/container/actions.dart (1)
226-249: AI assist context wiring looks solid.
Runtime + item metadata are captured and the open‑SSH apply path is consistent.lib/data/res/store.dart (1)
3-44: AiHistoryStore is correctly wired into DI and backups.
Getter, backup list, and lazy singleton registration are all in place.lib/view/page/systemd.dart (1)
34-56: Systemd AI assist action integrates cleanly.
Context blocks + open‑SSH apply path are consistent with the rest of the flow.lib/data/store/ai_history.dart (1)
1-22: AiHistoryStore implementation looks good.
Simple HiveStore wrapper with a clear history API.test/ai_safety_test.dart (1)
1-73: AI safety tests are thorough.
Good coverage for redaction variants and risk classification tiers.lib/view/page/ssh/page/ask_ai.dart (2)
35-52: Good implementation of context block preparation with pre-redaction.The flow correctly redacts sensitive data via
AiSafety.redactBlocksbefore passing to the AI sheet, then setsredacted: falseto prevent double-redaction. This is a sensible pattern.One minor note: the
redacted: falseflag name could be confusing since the blocks are redacted—it just means "don't redact again." Consider adding a brief inline comment for future maintainers, though this is optional.
61-69: LGTM on the scrollback tail builder.The boundary handling with
clamp(0, lines.length)is correct, and limiting to 200 lines is reasonable for context size management.lib/data/provider/ai/ai_safety.dart (2)
116-137: Redaction order matters: short patterns may interfere with compound patterns.In
_redactSpiIdentity, you correctly handleuser@ip:portbeforeuser@ipbefore individual components. Good defensive ordering. However, ifuseroripare common strings (e.g., user is "root" or ip is "127.0.0.1"), this could over-redact unrelated content. This is acceptable for a safety-first approach.
67-89: Risk classification logic is well-structured.Clear separation of high-risk destructive patterns vs medium-risk operational patterns. The cascading return pattern is readable and maintainable.
lib/view/widget/ai/ai_assist_sheet.dart (4)
100-115: Good lifecycle management with proper cleanup.The
disposemethod correctly cancels the subscription and cleans up controllers. The cascade operator usage for_inputControllercleanup is clean.Minor suggestion: Set
_subscription = nullafter cancel in dispose to be explicit, though not strictly necessary since the widget is being disposed.
215-225: Correct auto-scroll pattern with post-frame callback.Properly checks
hasClientsbefore and after the callback to avoid exceptions when the scroll controller is detached.
263-293: Well-designed confirmation flow with risk-aware countdown.The countdown button for high-risk or flagged commands provides good UX protection against accidental execution. Uses
context.showRoundDialogfrom fl_lib appropriately.
52-59: LGTM on the public API.
showAiAssistSheetprovides a clean entry point with appropriateisScrollControlledanduseSafeAreasettings for a modal bottom sheet.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| final _rxDdToBlockDevice = RegExp(r'\bdd\b[^\n\r]*\bof\s*=\s*/dev/'); | ||
|
|
||
| final _rxRmRf = RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*r[a-z-]*f[a-z-]*\b'); |
There was a problem hiding this comment.
_rxRmRf pattern misses rm -fr and similar orderings.
The pattern \s-[a-z-]*r[a-z-]*f[a-z-]*\b requires r to appear before f in the flags. Commands like rm -fr /, rm -f -r /, or rm --force -r / won't match. Consider matching both flags independently.
🔧 Suggested fix to match both flag orderings
-final _rxRmRf = RegExp(r'\brm\b[^\n\r]*\s-[a-z-]*r[a-z-]*f[a-z-]*\b');
+// Match rm with both -r/-R and -f/--force in any order
+final _rxRmRf = RegExp(
+ r'\brm\b[^\n\r]*(?=.*\s-[a-z-]*r)(?=.*\s-[a-z-]*f)',
+);Or use two separate checks in classifyRisk:
// Check for rm with both recursive and force flags
if (_rxRmRecursive.hasMatch(s) && _rxRmForce.hasMatch(s)) return AiCommandRisk.high;🤖 Prompt for AI Agents
In `@lib/data/provider/ai/ai_safety.dart` at line 166, The _rxRmRf regex only
matches when the 'r' flag appears before 'f' (e.g., "-rf"), so update detection
to handle any flag ordering by either creating two regexes (e.g., _rxRmRecursive
and _rxRmForce) that independently detect recursive and force flags and then
modify classifyRisk to treat a string as high risk when both patterns match, or
change _rxRmRf to a pattern that asserts presence of both flags in any order;
reference _rxRmRf and classifyRisk when applying the fix.
| @override | ||
| void didChangeDependencies() { | ||
| super.didChangeDependencies(); | ||
|
|
||
| if (_offsetPx != null) return; | ||
|
|
||
| final media = MediaQuery.of(context); | ||
| final size = media.size; | ||
| final x = Stores.setting.aiFabOffsetX.fetch().clamp(0.0, 1.0); | ||
| final y = Stores.setting.aiFabOffsetY.fetch().clamp(0.0, 1.0); | ||
|
|
||
| _offsetPx = Offset(size.width * x, size.height * y); | ||
| } |
There was a problem hiding this comment.
Clamp the restored FAB position to bounds.
When screen size or insets change, the initial/restored position can end up off‑screen because it isn’t clamped on load. Consider clamping in didChangeDependencies so the FAB is always visible.
🛠️ Suggested fix
`@override`
void didChangeDependencies() {
super.didChangeDependencies();
- if (_offsetPx != null) return;
-
- final media = MediaQuery.of(context);
- final size = media.size;
- final x = Stores.setting.aiFabOffsetX.fetch().clamp(0.0, 1.0);
- final y = Stores.setting.aiFabOffsetY.fetch().clamp(0.0, 1.0);
-
- _offsetPx = Offset(size.width * x, size.height * y);
+ if (_offsetPx == null) {
+ final media = MediaQuery.of(context);
+ final size = media.size;
+ final x = Stores.setting.aiFabOffsetX.fetch().clamp(0.0, 1.0);
+ final y = Stores.setting.aiFabOffsetY.fetch().clamp(0.0, 1.0);
+ _offsetPx = Offset(size.width * x, size.height * y);
+ }
+ _offsetPx = _clampToBounds(_offsetPx!);
}🤖 Prompt for AI Agents
In `@lib/view/widget/ai/ai_fab_overlay.dart` around lines 24 - 36, The restored
FAB position in didChangeDependencies can land off-screen; update the logic that
computes _offsetPx to clamp the computed Offset into the visible bounds: obtain
MediaQuery.of(context).size and MediaQuery.of(context).padding (or viewInsets as
needed), compute min/max X and Y allowed (e.g. between left padding and width -
right padding, and top padding and height - bottom padding, optionally
considering the FAB size), then clamp Stores.setting.aiFabOffsetX.fetch() and
aiFabOffsetY.fetch() into [0,1] as before but map them into those constrained
pixel ranges before assigning _offsetPx so the FloatingActionButton stays
on-screen.
1 participant
Fixes #1038
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.