Stabilize provider routing and quality checks#110
Merged
Conversation
Dependency ReviewThe following issues were found:
License Issuessrc-tauri/Cargo.toml
OpenSSF Scorecard
Scanned Files
|
|
Warning Review limit reached
More reviews will be available in 22 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (133)
📝 WalkthroughWalkthroughAdds a trusted local Agent Control domain (service, Tauri commands, UI renderer, settings, tests), refactors console log targeting/clearing and console overview, centralizes AI streaming/provider resolution/validation and local-context logic, switches catalog loading to a backend snapshot, adds release-target helpers, implements module repair, updates many frontend behaviors/tests, and pins CI actions (plus a cross-platform check job). ChangesFull PR
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 164: Replace the unpinned action reference "uses: actions/setup-node@v6"
with a pinned commit SHA for the setup-node action everywhere it appears (CI,
release and security-audit workflows); update each occurrence to "uses:
actions/setup-node@<full-commit-sha>" using the canonical commit SHA from the
actions/setup-node repository, ensure you update all files that currently use
the `@v6` tag (the same string "uses: actions/setup-node@v6") so no workflow
remains unpinned.
- Line 150: In the check-cross-platform job update the actions usage: replace
actions/checkout@v6 with a pinned commit SHA and add with: persist-credentials:
false to the checkout step (the checkout step symbol is actions/checkout), and
replace actions/setup-node@v6 with a pinned commit SHA for actions/setup-node;
ensure you update the checkout step block where persist-credentials is set and
the setup-node step reference so both actions use specific commit SHAs instead
of `@v6`.
In `@src-tauri/src/api/system/log_targets.rs`:
- Around line 46-67: The clear_console_log_target function currently follows
symlinks and may truncate files outside the intended log dir; update
clear_console_log_target (and the similar helpers around the clear_logs area
that call clear_log_file) to first reject symlinked roots/entries by using
symlink_metadata or DirEntry::file_type() (instead of
path.exists()/is_dir()/read_dir following symlinks), ensure the target itself is
not a symlink before iterating, skip any DirEntry where
file_type().is_symlink(), and only call clear_log_file for entries whose
file_type().is_file() and have a .log extension; keep clear_log_file unchanged
but ensure callers only pass canonical non-symlink File paths.
- Around line 6-18: The function resolve_console_log_target currently treats any
unrecognized view_id as the shared LOG_DIR; change it to explicitly allow only
"general" for that shared-root case and return an AppError::Validation for any
other unknown view_id. Update resolve_console_log_target to: keep the existing
engine: and module: branches, then if view_id == "general" return LOG_DIR, else
return Err(AppError::Validation("invalid console view id".into())) (use the
project’s AppError::Validation constructor/pattern). This ensures invalid IPC
payloads like "foo" fail validation instead of clearing platform logs.
In `@src/features/console/ui/ConsoleUI.test.ts`:
- Line 744: The test currently asserts a hardcoded English string via
expect(pane.textContent).toBe('No logs match selected levels'); — replace this
with a stable, i18n-safe assertion: retrieve the localized string via the app's
i18n lookup (the same key used by the Console UI) or assert a stable DOM marker
for the empty-state (e.g., a data-testid or CSS class used by ConsoleUI for the
no-logs state) instead; update ConsoleUI.test.ts to reference the i18n key or
empty-state selector rather than the literal text so tests remain
locale-agnostic (locate the assertion around the pane variable in the test).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5fccdd04-1143-47d5-8d41-aa3017f96ccf
⛔ Files ignored due to path filters (1)
src/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.github/workflows/ci.ymlsrc-tauri/src/api/system/log_targets.rssrc-tauri/src/api/system/logs.rssrc-tauri/src/api/system/mod.rssrc/features/ai/services/AIBridgeMessageController.test.tssrc/features/ai/ui/AISettingsRenderer.test.tssrc/features/console/ui/ConsoleUI.test.tssrc/shared/utils/providerSupport.test.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/api/system/logs.rs (1)
127-132:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCanonicalize
view_idlike the sibling commands do.
get_console_logsandclear_console_logsboth passview_idthroughcanonical_console_view_idbefore resolving, butopen_console_log_targetskips that step. As a result, inputs that the other two accept (e.g." general ","module:foo ", or"engine:llama.cpp"when only the engine branch is canonicalized insideresolve_*) will hit the new "invalid console view id" path here and the folder-open IPC will fail for cases the rest of the API treats as valid.Proposed fix
pub fn open_console_log_target(view_id: String) -> Result<(), AppError> { - let target = resolve_console_log_target(&view_id)?; + let canonical_view_id = canonical_console_view_id(&view_id); + let target = resolve_console_log_target(&canonical_view_id)?; fs::create_dir_all(&target)?; open_folder(&target)?; Ok(()) }As per coding guidelines, "Review as a Windows-first Tauri backend. Prioritize IPC boundary validation..."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/api/system/logs.rs` around lines 127 - 132, open_console_log_target currently calls resolve_console_log_target(&view_id) directly and thus rejects inputs that other commands accept; update open_console_log_target to first canonicalize the incoming view_id by calling canonical_console_view_id(&view_id)? and use that canonical value when calling resolve_console_log_target, preserving existing error propagation and behavior of fs::create_dir_all and open_folder; reference functions open_console_log_target, canonical_console_view_id, and resolve_console_log_target when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/domain/ai/session_context.rs`:
- Around line 260-279: The code currently assigns session.summary =
merge_summary(...) and then advances session.summary_message_count
unconditionally, which can clear the existing summary when merge_summary returns
None and permanently drop those turns; change the flow so you call
merge_summary(...) and inspect its result first: if it returns Some(non_empty)
set session.summary = that value and then set session.summary_message_count =
u32::try_from(self.recent_start_index).unwrap_or(u32::MAX); if it returns None
leave session.summary and session.summary_message_count unchanged (and return
false), ensuring you only advance recent_start_index when a new summary was
actually produced; reference merge_summary, session.summary,
session.summary_message_count, recent_start_index, build_summary_lines and
summary_lines to locate the change.
- Around line 338-351: The loop over recent_turn_ranges currently uses continue
when a turn's tokens would exceed available_budget, which can produce
non-contiguous history; change the logic in that loop so that when used_tokens +
turn_tokens > available_budget you break out of the loop instead of continuing,
preserving contiguity of kept_recent; ensure this update is applied in the
function that iterates recent_turn_ranges and references session.history,
estimate_messages_tokens, kept_recent, used_tokens, and available_budget so
older turns are only appended while the budget allows and the kept window
remains contiguous.
In `@src-tauri/src/domain/ai/streaming_chunks.rs`:
- Around line 202-224: The fallback chain in streaming_chunks.rs duplicates the
json.get("message").and_then(|message|
message.get("content")).and_then(provider_response::extract_stream_text) call,
making the second instance dead code; update the or_else chain in the block that
builds the stream text to either remove the duplicate or (preferably) replace
the second occurrence with the provider variant intended for delta-style
responses, e.g. use json.get("delta").and_then(|delta|
delta.get("content")).and_then(provider_response::extract_stream_text) so the
chain checks message.content, response, delta.content, token.text as distinct
fallbacks around provider_response::extract_stream_text.
---
Outside diff comments:
In `@src-tauri/src/api/system/logs.rs`:
- Around line 127-132: open_console_log_target currently calls
resolve_console_log_target(&view_id) directly and thus rejects inputs that other
commands accept; update open_console_log_target to first canonicalize the
incoming view_id by calling canonical_console_view_id(&view_id)? and use that
canonical value when calling resolve_console_log_target, preserving existing
error propagation and behavior of fs::create_dir_all and open_folder; reference
functions open_console_log_target, canonical_console_view_id, and
resolve_console_log_target when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d6a9cca-812b-48a4-b714-f7465ab7f060
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/dependency-review.yml.github/workflows/release.yml.github/workflows/security-audit.ymlsrc-tauri/Cargo.tomlsrc-tauri/resources/locales/en.jsonsrc-tauri/resources/locales/ru.jsonsrc-tauri/resources/locales/zh.jsonsrc-tauri/src/api/ai/mod.rssrc-tauri/src/api/system/log_targets.rssrc-tauri/src/api/system/logs.rssrc-tauri/src/app/window.rssrc-tauri/src/domain/ai/mod.rssrc-tauri/src/domain/ai/session.rssrc-tauri/src/domain/ai/session_context.rssrc-tauri/src/domain/ai/streaming.rssrc-tauri/src/domain/ai/streaming_chunks.rssrc-tauri/src/domain/modules/controller/lifecycle.rssrc-tauri/src/domain/modules/controller/process.rssrc-tauri/src/domain/modules/github_release_targets.rssrc-tauri/src/domain/modules/github_releases.rssrc-tauri/src/domain/modules/mod.rssrc-tauri/src/domain/system/hardware_probe.rssrc-tauri/src/infrastructure/system/startup.rssrc-tauri/src/utils/memory.rssrc/features/console/ui/ConsoleUI.test.tssrc/features/console/ui/ConsoleUI.ts
💤 Files with no reviewable changes (1)
- src-tauri/src/infrastructure/system/startup.rs
✅ Files skipped from review due to trivial changes (6)
- src-tauri/src/domain/ai/mod.rs
- src-tauri/src/domain/modules/mod.rs
- src/features/console/ui/ConsoleUI.ts
- src-tauri/src/api/ai/mod.rs
- src-tauri/resources/locales/ru.json
- src-tauri/src/domain/modules/controller/process.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/api/system/console_overview.rs`:
- Around line 346-350: The grouping key currently uses
normalize_view_label(&slot.engine.name) which collapses different engines that
share a display name; change the grouping to use
canonical_engine_id(&slot.engine.id) as the dedupe key (i.e., set label_key to
canonical_engine_id(&slot.engine.id) or introduce engine_key =
canonical_engine_id(&slot.engine.id) and use that for label_to_id.entry(...)),
while still using normalize_view_label(&slot.engine.name) only to derive the
human-visible label when first inserting into the map; apply the same change to
the other occurrence handling lines 353-366 so engines are deduplicated by
canonical ID, not by display name.
In `@src-tauri/src/domain/ai/ai_validation.rs`:
- Around line 15-21: The code that builds models_url from the caller-provided
base_url (the base_url variable and models_url construction) accepts arbitrary
strings, enabling SSRF; change validation to parse and enforce a safe whitelist
or strict rules: accept only empty/None or URLs with scheme https and host
matching an allowlist of trusted hosts (e.g., openrouter.ai and any additional
approved domains), reject or return an error for localhost, 127.0.0.0/8, private
IP ranges, and non-HTTPS schemes; apply the same validation logic to the other
similar spot referenced (lines ~70-77) so both usages validate parsed URL host
and scheme before formatting models_url or any other request URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 436209eb-ca79-4403-be9e-65f2745eae09
📒 Files selected for processing (11)
src-tauri/src/api/system/console_overview.rssrc-tauri/src/api/system/logs.rssrc-tauri/src/api/system/mod.rssrc-tauri/src/domain/ai/ai_dispatch.rssrc-tauri/src/domain/ai/ai_provider_resolution.rssrc-tauri/src/domain/ai/ai_service.rssrc-tauri/src/domain/ai/ai_validation.rssrc-tauri/src/domain/ai/mod.rssrc-tauri/src/domain/engine/engine_ids.rssrc-tauri/src/domain/engine/manager.rssrc-tauri/src/domain/engine/mod.rs
✅ Files skipped from review due to trivial changes (1)
- src-tauri/src/domain/engine/mod.rs
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/console/services/ConsoleLogService.ts (1)
124-126:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset known view state in fallback paths to avoid stale general-log filtering.
_knownViewIdsis refreshed only on successful overview fetch. IfgetAvailableViews()falls back, old runtime IDs can remain and hide logs fromgeneralunexpectedly.Suggested fix
public async getAvailableViews(): Promise<IConsoleLogView[]> { if (!this.bridge.isTauri()) { - return [{ id: 'general', label: 'Platform' }]; + const fallback = [{ id: 'general', label: 'Platform' }]; + this._rememberKnownViews(fallback); + return fallback; } @@ - return [{ id: 'general', label: 'Platform' }]; + const fallback = [{ id: 'general', label: 'Platform' }]; + this._rememberKnownViews(fallback); + return fallback; }As per coding guidelines,
src/**/*.ts: “Prioritize state consistency…”.Also applies to: 141-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/console/services/ConsoleLogService.ts` around lines 124 - 126, When getAvailableViews() takes the fallback path (e.g., when this.bridge.isTauri() is false or in the other fallback at the later branch), reset the service's _knownViewIds to include the default 'general' view so stale runtime IDs can't hide general logs; update the fallback branches inside ConsoleLogService.getAvailableViews() to assign _knownViewIds = new Set(['general']) (or equivalent) before returning the fallback view list so state is consistent after failed overview fetches.
🧹 Nitpick comments (2)
src-tauri/src/domain/ai/ai_validation.rs (1)
254-274: ⚡ Quick winAdd test case for IPv4-mapped IPv6 bypass.
Consider adding a test for
https://[::ffff:127.0.0.1]/v1to ensure IPv4-mapped IPv6 addresses are also rejected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/domain/ai/ai_validation.rs` around lines 254 - 274, The test build_validation_request_rejects_unsafe_base_urls is missing a case for IPv4-mapped IPv6 addresses, which can bypass IPv4 checks; update the test (in the function build_validation_request_rejects_unsafe_base_urls) to include "https://[::ffff:127.0.0.1]/v1" in the array of base_url values so that build_validation_request is asserted to reject IPv4-mapped IPv6 loopback addresses as well.src-tauri/src/api/system/config.rs (1)
38-49: 💤 Low valueRun
npm --prefix src run bindings:syncafter merging.New Tauri command
get_catalog_snapshotwas added with Specta annotations. As per coding guidelines, ensure frontend bindings are regenerated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/api/system/config.rs` around lines 38 - 49, The new Tauri command get_catalog_snapshot() was added with #[specta::specta] annotations so frontend bindings must be regenerated; after merging this change run the binding sync command (npm --prefix src run bindings:sync) to update Specta/TypeScript bindings for the new get_catalog_snapshot API so the frontend knows about the new command and types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/domain/ai/ai_validation.rs`:
- Around line 40-92: The SSRF check misses IPv4-mapped IPv6 addresses (e.g.
::ffff:127.0.0.1) — update is_restricted_ip so when it receives IpAddr::V6 it
first checks if the Ipv6Addr is an IPv4-mapped address, and if so converts it to
an Ipv4Addr and calls is_restricted_ipv4; otherwise continue to call
is_restricted_ipv6. Change the implementation referenced by is_restricted_ip
(and adjust callers like validate_openai_compatible_base_url if needed) to
normalize mapped IPv6 -> IPv4 before applying the IPv4-restriction checks.
In `@src-tauri/src/infrastructure/logging/logger.rs`:
- Around line 558-560: In parse_log_timestamp replace the u32 narrowing that
uses u32::try_from(timestamp.timestamp()).ok()? which drops negative or very
large seconds; instead convert the signed seconds (timestamp.timestamp() -> i64)
directly to f64 and add the fractional milliseconds
(timestamp.timestamp_subsec_millis() / 1000.0) so the function returns Some(f64)
for pre-epoch and far-future dates; update the code paths around the local
variable named seconds and the final Some(...) expression to use the i64-to-f64
conversion rather than u32 conversion.
In `@src/features/ai/services/AIBridgeProviderPolicy.ts`:
- Line 25: The current expression in AIBridgeProviderPolicy (return
this._catalogProvider(providerId)?.providerPolicy?.isCloudProvider ?? false)
defaults unknown providers to non-cloud; change it to avoid silent default by
explicitly handling missing catalog entries or missing providerPolicy: fetch the
catalog entry via _catalogProvider(providerId) and if it's null/undefined,
return an explicit "unknown" result or throw/propagate an error instead of
returning false, or use a deterministic fallback based on providerId (e.g.,
lookupAllowedFallback(providerId)) so that providerPolicy.isCloudProvider is
only used when present; update any callers of the method to handle the new
"unknown" outcome (or the thrown error) accordingly.
In `@src/features/ai/services/AIProviderManager.ts`:
- Around line 197-204: The _isLocalProvider method currently treats missing
catalog policy as local which can bypass cloud checks; change it to fail closed
by treating unknown providers as non-local: when
this._getCatalogProvider(providerId) returns null/undefined or providerPolicy is
missing, return false (or throw an explicit error) instead of true so
startProvider() will enforce cloud key checks; update _isLocalProvider (and add
a brief log or error message referencing providerId) to make the decision
explicit.
In `@src/features/ai/ui/AISettingsContentRenderer.ts`:
- Around line 144-146: Replace the hardcoded title on the API anchor element
with a localized string from the i18n resources: locate the anchor with id
`${appId}-api-link` in AISettingsContentRenderer and replace title="Manage your
OpenRouter API Keys" with a call that retrieves the text from
src-tauri/resources/locales (e.g. title={t('ai.manageOpenRouterApiKeys')} or
equivalent project i18n helper), add the new key "ai.manageOpenRouterApiKeys" to
the locale files, and ensure the same localized label is used consistently for
the link tooltip.
---
Outside diff comments:
In `@src/features/console/services/ConsoleLogService.ts`:
- Around line 124-126: When getAvailableViews() takes the fallback path (e.g.,
when this.bridge.isTauri() is false or in the other fallback at the later
branch), reset the service's _knownViewIds to include the default 'general' view
so stale runtime IDs can't hide general logs; update the fallback branches
inside ConsoleLogService.getAvailableViews() to assign _knownViewIds = new
Set(['general']) (or equivalent) before returning the fallback view list so
state is consistent after failed overview fetches.
---
Nitpick comments:
In `@src-tauri/src/api/system/config.rs`:
- Around line 38-49: The new Tauri command get_catalog_snapshot() was added with
#[specta::specta] annotations so frontend bindings must be regenerated; after
merging this change run the binding sync command (npm --prefix src run
bindings:sync) to update Specta/TypeScript bindings for the new
get_catalog_snapshot API so the frontend knows about the new command and types.
In `@src-tauri/src/domain/ai/ai_validation.rs`:
- Around line 254-274: The test
build_validation_request_rejects_unsafe_base_urls is missing a case for
IPv4-mapped IPv6 addresses, which can bypass IPv4 checks; update the test (in
the function build_validation_request_rejects_unsafe_base_urls) to include
"https://[::ffff:127.0.0.1]/v1" in the array of base_url values so that
build_validation_request is asserted to reject IPv4-mapped IPv6 loopback
addresses as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 688d2c81-448d-44ae-93f8-be50912c4283
📒 Files selected for processing (65)
docs/localization/en/CURRENT_STATE.mddocs/localization/en/INTEGRATION_API.mddocs/localization/en/README.mddocs/localization/en/ROADMAP.mddocs/localization/en/TRUST_MODEL.mddocs/localization/en/VISION.mddocs/localization/ru/INTEGRATION_DEVELOPMENT.mddocs/localization/ru/README.mddocs/localization/zh/INTEGRATION_DEVELOPMENT.mddocs/localization/zh/README.mdsrc-tauri/src/api/system/config.rssrc-tauri/src/api/system/console_overview.rssrc-tauri/src/api/system/logs.rssrc-tauri/src/domain/ai/ai_service.rssrc-tauri/src/domain/ai/ai_validation.rssrc-tauri/src/domain/ai/provider_payload.rssrc-tauri/src/domain/ai/session.rssrc-tauri/src/domain/ai/session_context.rssrc-tauri/src/domain/ai/streaming_chunks.rssrc-tauri/src/domain/modules/controller/script_runtime.rssrc-tauri/src/infrastructure/logging/logger.rssrc-tauri/src/lib.rssrc-tauri/src/models/config.rssrc-tauri/src/utils/paths.rssrc/app/CoreStateRestore.test.tssrc/app/CoreStateRestore.tssrc/features/ai/services/AIBridge.test.tssrc/features/ai/services/AIBridge.tssrc/features/ai/services/AIBridgeConfig.tssrc/features/ai/services/AIBridgeMessageController.test.tssrc/features/ai/services/AIBridgeMessageController.tssrc/features/ai/services/AIBridgeProviderPolicy.test.tssrc/features/ai/services/AIBridgeProviderPolicy.tssrc/features/ai/services/AIChatTransport.test.tssrc/features/ai/services/AIChatTransport.tssrc/features/ai/services/AIProviderManager.test.tssrc/features/ai/services/AIProviderManager.tssrc/features/ai/services/EngineStatusService.test.tssrc/features/ai/services/EngineStatusService.tssrc/features/ai/types/aiTypes.tssrc/features/ai/ui/AISettingsContentRenderer.tssrc/features/ai/ui/AISettingsKeyController.test.tssrc/features/ai/ui/AISettingsKeyController.tssrc/features/ai/ui/AISettingsRenderer.test.tssrc/features/ai/ui/AISettingsRenderer.tssrc/features/ai/ui/AISettingsSelectionController.tssrc/features/ai/ui/AISettingsViewPolicy.test.tssrc/features/ai/ui/AISettingsViewPolicy.tssrc/features/console/services/ConsoleLogService.test.tssrc/features/console/services/ConsoleLogService.tssrc/features/settings/services/SettingsService.test.tssrc/features/settings/services/SettingsService.tssrc/shared/services/CatalogLoadSnapshot.tssrc/shared/services/CatalogService.test.tssrc/shared/services/CatalogService.tssrc/shared/shell/AppUI.test.tssrc/shared/shell/AppUI.tssrc/shared/types/bindings.tssrc/shared/types/coreTypes.tssrc/shared/utils/customProviderSupport.test.tssrc/shared/utils/customProviderSupport.tssrc/shared/utils/providerSupport.test.tssrc/shared/utils/providerSupport.tssrc/test/helpers/catalogTestUtils.tssrc/test/integration/CatalogService.integration.test.ts
💤 Files with no reviewable changes (6)
- src/shared/utils/providerSupport.ts
- src/shared/utils/customProviderSupport.test.ts
- src/shared/utils/customProviderSupport.ts
- src/features/ai/services/EngineStatusService.ts
- src/shared/utils/providerSupport.test.ts
- src/shared/services/CatalogLoadSnapshot.ts
✅ Files skipped from review due to trivial changes (9)
- docs/localization/en/README.md
- docs/localization/zh/INTEGRATION_DEVELOPMENT.md
- docs/localization/en/TRUST_MODEL.md
- docs/localization/en/INTEGRATION_API.md
- docs/localization/ru/README.md
- src/shared/types/bindings.ts
- docs/localization/ru/INTEGRATION_DEVELOPMENT.md
- docs/localization/en/CURRENT_STATE.md
- docs/localization/en/VISION.md
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/features/console/services/ConsoleLogService.test.ts (1)
196-225: ⚡ Quick winAdd
clearAllLogs()coverage for agent audit logs.This PR also changed
clearAllLogs()to advance the local agent cursor, but the suite only exercisesclearLogs('agent'). A targeted test here would catch regressions where old audit entries reappear after a full clear.As per coding guidelines
src/**/*.ts: Review as vanilla TypeScript frontend code. Prioritize state consistency, async cancellation, event listener cleanup, DOM injection risks, user-visible regressions, and test coverage for changed flows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/console/services/ConsoleLogService.test.ts` around lines 196 - 225, Add a test that covers clearAllLogs() for the agent audit logs: after mocking invokeModule.invokeSafe and calling service.fetchLogs('agent'), call service.clearAllLogs() (in addition to or instead of service.clearLogs('agent')) and assert it returns true, that bridge.invoke was not called with 'clear_console_logs', and that service.getLogsForView('agent') is empty and any local audit cursor/state that prevents old audit entries from reappearing has been advanced/updated; use existing helpers setupTauri, service.fetchLogs, service.clearLogs/service.clearAllLogs, and service.getLogsForView to locate the code under test.src/features/console/services/ConsoleLogService.ts (1)
51-67: ⚡ Quick winMake localization injection mandatory.
_translate = (key) => keyfails open: any missed constructor wiring renders raw IDs likeui.launcher.web.logs_agentin the UI. Since this service now emits user-facing labels and fallback text, require a translator here so missing integration fails at compile time instead of shipping untranslated keys.♻️ Proposed fix
type ConsoleLogTranslate = (key: string, fallback?: string) => string; ... constructor( private readonly bridge: IBridge, private readonly _tracer: ConsoleLogServiceLogger, - private readonly _translate: ConsoleLogTranslate = (key) => key, + private readonly _translate: ConsoleLogTranslate, ) {}As per coding guidelines
src/**/*.{ts,tsx,html}: Keep UI text in I18n resources under src-tauri/resources/locales/ and do not hardcode Russian or English user-facing strings in TS/HTML.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/console/services/ConsoleLogService.ts` around lines 51 - 67, The constructor currently provides a default _translate = (key) => key which allows missing i18n wiring; remove the default so the ConsoleLogService constructor requires a ConsoleLogTranslate parameter (i.e. change the constructor signature to accept _translate: ConsoleLogTranslate without a default) and update all call sites that construct ConsoleLogService to pass a translator instance; ensure references to _translate in methods (and any tests/mocks) use the injected translator so missing wiring fails at compile time instead of showing raw keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/domain/agent_control.rs`:
- Around line 313-327: pending_token() and discard_pending_token() separate read
and removal, allowing the one-time token to be observed multiple times; replace
them with a single atomic backend-only reveal that returns the token and
invalidates it under the same lock. Implement a method (e.g.,
reveal_and_consume_pending_token or take_pending_token) that locks
self.pending_tokens.lock().await, attempts to remove(id) and returns the token
if present or an AppError::Validation if absent; update any callers of
pending_token/discard_pending_token to use the new atomic method so the token is
copied out and removed in one protected step.
---
Nitpick comments:
In `@src/features/console/services/ConsoleLogService.test.ts`:
- Around line 196-225: Add a test that covers clearAllLogs() for the agent audit
logs: after mocking invokeModule.invokeSafe and calling
service.fetchLogs('agent'), call service.clearAllLogs() (in addition to or
instead of service.clearLogs('agent')) and assert it returns true, that
bridge.invoke was not called with 'clear_console_logs', and that
service.getLogsForView('agent') is empty and any local audit cursor/state that
prevents old audit entries from reappearing has been advanced/updated; use
existing helpers setupTauri, service.fetchLogs,
service.clearLogs/service.clearAllLogs, and service.getLogsForView to locate the
code under test.
In `@src/features/console/services/ConsoleLogService.ts`:
- Around line 51-67: The constructor currently provides a default _translate =
(key) => key which allows missing i18n wiring; remove the default so the
ConsoleLogService constructor requires a ConsoleLogTranslate parameter (i.e.
change the constructor signature to accept _translate: ConsoleLogTranslate
without a default) and update all call sites that construct ConsoleLogService to
pass a translator instance; ensure references to _translate in methods (and any
tests/mocks) use the injected translator so missing wiring fails at compile time
instead of showing raw keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4e385ee-e9aa-4b75-9d1a-51ea10365e69
📒 Files selected for processing (15)
src-tauri/resources/locales/en.jsonsrc-tauri/resources/locales/ru.jsonsrc-tauri/resources/locales/zh.jsonsrc-tauri/src/api/agent_control.rssrc-tauri/src/domain/agent_control.rssrc-tauri/src/domain/modules/controller/mod.rssrc/app/CoreServiceFactory.tssrc/features/console/services/ConsoleLogService.test.tssrc/features/console/services/ConsoleLogService.tssrc/features/console/ui/ConsoleLogPresentationHelper.tssrc/features/console/ui/ConsoleUI.tssrc/features/settings/ui/AgentControlSettingsRenderer.tssrc/public/templates/pages/console.htmlsrc/styles/features/console-page.csssrc/styles/features/settings-page.css
✅ Files skipped from review due to trivial changes (3)
- src/styles/features/console-page.css
- src-tauri/resources/locales/zh.json
- src-tauri/resources/locales/en.json
F0RLE
force-pushed
the
F0RLE/code-quality-stabilization
branch
2 times, most recently
from
3c54c60 to
61b47f0
Compare
F0RLE
force-pushed
the
F0RLE/code-quality-stabilization
branch
from
61b47f0 to
e0103e0
Compare
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/domain/integration_api/routing.rs (1)
429-445:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject Windows drive-relative prefixed paths (
Component::PrefixwithoutRootDir) invalidate_relative_draft_path.The current checks block
is_absolute()and.., but a Windows path likeC:temp\fileis not considered absolute;components()includesComponent::Prefix(_)while omittingComponent::RootDir, so it can pass. If this value is later used withPathBuf::join(draft_dir, input), Windows join semantics can ignore the base and return the prefixed path, defeating path confinement. Extend the component validation to rejectPrefix(_)(andRootDirfor completeness).Suggested fix
pub(super) fn validate_relative_draft_path(path: &str) -> Result<(), AppError> { if path.trim().is_empty() { return Err(AppError::Validation( "Draft entry path cannot be empty".to_string(), )); } let path = Path::new(path); if path.is_absolute() || path .components() - .any(|component| matches!(component, std::path::Component::ParentDir)) + .any(|component| { + matches!( + component, + std::path::Component::ParentDir + | std::path::Component::RootDir + | std::path::Component::Prefix(_) + ) + }) { return Err(AppError::Validation( "Draft entry path must stay inside the draft directory".to_string(), )); } Ok(()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/domain/integration_api/routing.rs` around lines 429 - 445, The validate_relative_draft_path function currently checks is_absolute and ParentDir but allows Windows drive-relative prefixes; update its component validation in validate_relative_draft_path to also reject components matching std::path::Component::Prefix(_) and std::path::Component::RootDir so any path containing Prefix or RootDir returns Err(AppError::Validation("Draft entry path must stay inside the draft directory".to_string())). Locate the check that iterates path.components() and add matches for Prefix(_) and RootDir alongside ParentDir to ensure Windows drive-relative inputs like "C:temp\\file" are refused.
♻️ Duplicate comments (5)
src-tauri/src/api/agent_control.rs (1)
62-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t consume the one-time token before clipboard write succeeds.
take_pending_tokenis called beforewrite_text. On clipboard failure, the token is lost.Suggested fix
- let token = service.take_pending_token(&id).await?; + let token = service.pending_token(&id).await?; app.clipboard() .write_text(token) .map_err(|error| AppError::External { message: format!("Failed to copy Agent Control token: {error}"), request_id: None, })?; + service.take_pending_token(&id).await?; Ok(())If a non-consuming read API is unavailable, restore the token on write failure before returning the error.
As per coding guidelines
src-tauri/**/*.rs: Implement secure state management with backend ownership of sensitive data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/api/agent_control.rs` around lines 62 - 68, The code currently calls service.take_pending_token(&id).await? (consuming the one-time token) before calling app.clipboard().write_text(token), which loses the token on clipboard failure; change the flow so the token is not consumed until clipboard write succeeds: either use a non-consuming read API on service (e.g., peek_pending_token or similar) and only call take_pending_token(&id) after write_text succeeds, or if no non-consuming API exists, attempt to write_text first and on error call a restore method on service (e.g., restore_pending_token(&id, token)) before returning an AppError::External; update references to take_pending_token, app.clipboard().write_text, service, id, token, and AppError::External accordingly.src/features/settings/ui/SettingsUI.ts (1)
136-136:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid hardcoded English fallback in user-facing TS label.
The default value
'Switch settings section'is user-facing English in frontend code.Suggested fix
- const label = this._i18n.t('ui.launcher.settings.section_jump', 'Switch settings section'); + const label = this._i18n.t('ui.launcher.settings.section_jump', '');As per coding guidelines
src/**/*.{ts,tsx,html}: Keep UI text in I18n resources under src-tauri/resources/locales/ and do not hardcode Russian or English user-facing strings in TS/HTML.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/settings/ui/SettingsUI.ts` at line 136, The code currently hardcodes an English fallback string in the i18n call for the label (const label = this._i18n.t('ui.launcher.settings.section_jump', 'Switch settings section')), which violates the rule to keep UI text in locale files; add the key ui.launcher.settings.section_jump to the src-tauri/resources/locales/* locale JSON/YAML files with appropriate translations (including English), then remove the hardcoded fallback from the call and call this._i18n.t('ui.launcher.settings.section_jump') (or ensure the t function uses locale files by default) so the label is sourced from the localization resources instead of inline text.src/public/templates/pages/settings.html (1)
39-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove hardcoded English tooltip/label fallback from the HTML button.
titleandaria-labelare hardcoded in English even though this control already uses i18n attributes.Suggested fix
- title="Switch settings section" - aria-label="Switch settings section"As per coding guidelines
src/**/*.{ts,tsx,html}: Keep UI text in I18n resources under src-tauri/resources/locales/ and do not hardcode Russian or English user-facing strings in TS/HTML.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/public/templates/pages/settings.html` around lines 39 - 40, Remove the hardcoded English title and aria-label attributes from the button (the element in settings.html that currently has title="Switch settings section" and aria-label="Switch settings section") and replace them with the appropriate i18n attributes/keys so the tooltip/accessible label comes from the locale resources under src-tauri/resources/locales/ (use the same i18n key pattern already used by this control so translations are loaded instead of hardcoding English).src-tauri/src/domain/integration_api/routing.rs (2)
251-258:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep successful module control responses independent from UI sync failures.
Line 257 uses
?onsync_launcher_selected_module(...); if sync/save/emit fails, a successful module action is returned as an error.Suggested fix
if response.success && matches!( action, ModuleAction::Start | ModuleAction::Restart | ModuleAction::Repair ) { - sync_launcher_selected_module(&context, client, module_id).await?; + if let Err(error) = sync_launcher_selected_module(&context, client, module_id).await + { + tracing::warn!( + module_id = %module_id, + action = %module_action_name(action), + "Module control succeeded but selected-module sync failed: {error}" + ); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/domain/integration_api/routing.rs` around lines 251 - 258, The code currently uses `?` on `sync_launcher_selected_module(&context, client, module_id).await` inside the block that handles a successful module control response, which turns any UI sync/save/emit failure into an overall error; instead, call and await `sync_launcher_selected_module` and handle its Result locally (e.g., `if let Err(e) = sync_launcher_selected_module(...).await { ... }`) so the successful response from the module action (ModuleAction::Start | Restart | Repair) is preserved; log or trace the error (using the existing logger/context) and continue without propagating the error to the caller.
114-117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict approval list visibility to avoid cross-agent data leakage.
Line 115 still allows
AuthorizedClient::Agent(_), and Line 708 returnsstate.approvalsunfiltered. Any observe-scoped agent can read other agents’ pending approval details.As per coding guidelines `src-tauri/src/**/*.rs`: “Prioritize IPC boundary validation … and keep user data scoped to app directories.”Suggested fix
- ("GET", ["v1", "agent", "approvals"]) => { - ensure_launcher_client(client)?; - ensure_agent_scope(client, AgentScope::Observe)?; - handle_agent_approvals_request(&context).await - } + ("GET", ["v1", "agent", "approvals"]) => { + ensure_strict_launcher_client(client)?; + handle_agent_approvals_request(&context).await + }+fn ensure_strict_launcher_client(client: &AuthorizedClient) -> Result<(), AppError> { + match client { + AuthorizedClient::Launcher => Ok(()), + AuthorizedClient::Agent(_) | AuthorizedClient::Module(_) => Err(AppError::PermissionDenied( + "Only launcher tokens can access global approvals".to_string(), + )), + } +}Also applies to: 699-709
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/domain/integration_api/routing.rs` around lines 114 - 117, The approvals endpoint currently permits AuthorizedClient::Agent(_) with Observe scope and returns state.approvals unfiltered, allowing cross-agent leakage; update handle_agent_approvals_request to accept the caller's agent identity (or the AuthorizedClient) and filter state.approvals to only include approvals scoped to that agent (e.g., match approval.target_agent_id or approval.agent_id against the caller's agent id) and ensure the routing/authorization (ensure_agent_scope / AuthorizedClient::Agent) passes the caller identity through to the handler so only the caller's approvals are returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/styles/features/settings-page.css`:
- Around line 83-87: The CSS uses the keyword "currentColor" in the box-shadow
(the block with offsets "-8px -4px 0 currentColor, 8px -4px 0 currentColor, -4px
0 0 currentColor, 4px 0 0 currentColor, 0 4px 0 currentColor;") which triggers
Stylelint's value-keyword-case rule; update each occurrence of "currentColor" to
the lowercase "currentcolor" so the keyword casing matches the linter
expectation and CI will pass.
---
Outside diff comments:
In `@src-tauri/src/domain/integration_api/routing.rs`:
- Around line 429-445: The validate_relative_draft_path function currently
checks is_absolute and ParentDir but allows Windows drive-relative prefixes;
update its component validation in validate_relative_draft_path to also reject
components matching std::path::Component::Prefix(_) and
std::path::Component::RootDir so any path containing Prefix or RootDir returns
Err(AppError::Validation("Draft entry path must stay inside the draft
directory".to_string())). Locate the check that iterates path.components() and
add matches for Prefix(_) and RootDir alongside ParentDir to ensure Windows
drive-relative inputs like "C:temp\\file" are refused.
---
Duplicate comments:
In `@src-tauri/src/api/agent_control.rs`:
- Around line 62-68: The code currently calls
service.take_pending_token(&id).await? (consuming the one-time token) before
calling app.clipboard().write_text(token), which loses the token on clipboard
failure; change the flow so the token is not consumed until clipboard write
succeeds: either use a non-consuming read API on service (e.g.,
peek_pending_token or similar) and only call take_pending_token(&id) after
write_text succeeds, or if no non-consuming API exists, attempt to write_text
first and on error call a restore method on service (e.g.,
restore_pending_token(&id, token)) before returning an AppError::External;
update references to take_pending_token, app.clipboard().write_text, service,
id, token, and AppError::External accordingly.
In `@src-tauri/src/domain/integration_api/routing.rs`:
- Around line 251-258: The code currently uses `?` on
`sync_launcher_selected_module(&context, client, module_id).await` inside the
block that handles a successful module control response, which turns any UI
sync/save/emit failure into an overall error; instead, call and await
`sync_launcher_selected_module` and handle its Result locally (e.g., `if let
Err(e) = sync_launcher_selected_module(...).await { ... }`) so the successful
response from the module action (ModuleAction::Start | Restart | Repair) is
preserved; log or trace the error (using the existing logger/context) and
continue without propagating the error to the caller.
- Around line 114-117: The approvals endpoint currently permits
AuthorizedClient::Agent(_) with Observe scope and returns state.approvals
unfiltered, allowing cross-agent leakage; update handle_agent_approvals_request
to accept the caller's agent identity (or the AuthorizedClient) and filter
state.approvals to only include approvals scoped to that agent (e.g., match
approval.target_agent_id or approval.agent_id against the caller's agent id) and
ensure the routing/authorization (ensure_agent_scope / AuthorizedClient::Agent)
passes the caller identity through to the handler so only the caller's approvals
are returned.
In `@src/features/settings/ui/SettingsUI.ts`:
- Line 136: The code currently hardcodes an English fallback string in the i18n
call for the label (const label =
this._i18n.t('ui.launcher.settings.section_jump', 'Switch settings section')),
which violates the rule to keep UI text in locale files; add the key
ui.launcher.settings.section_jump to the src-tauri/resources/locales/* locale
JSON/YAML files with appropriate translations (including English), then remove
the hardcoded fallback from the call and call
this._i18n.t('ui.launcher.settings.section_jump') (or ensure the t function uses
locale files by default) so the label is sourced from the localization resources
instead of inline text.
In `@src/public/templates/pages/settings.html`:
- Around line 39-40: Remove the hardcoded English title and aria-label
attributes from the button (the element in settings.html that currently has
title="Switch settings section" and aria-label="Switch settings section") and
replace them with the appropriate i18n attributes/keys so the tooltip/accessible
label comes from the locale resources under src-tauri/resources/locales/ (use
the same i18n key pattern already used by this control so translations are
loaded instead of hardcoding English).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 01e7f732-9e37-4dec-a432-c0e7fefb51e0
⛔ Files ignored due to path filters (1)
src/assets/fonts/Cubic_11.zh-subset.woff2is excluded by!**/*.woff2
📒 Files selected for processing (25)
SECURITY.mddocs/examples/sdk/javascript/axelate-client.mjsdocs/examples/sdk/python/axelate_sdk.pydocs/localization/en/AGENT_CONTROL.mddocs/localization/en/INTEGRATION_API.mddocs/localization/en/ROADMAP.mddocs/localization/en/TRUST_MODEL.mdsrc-tauri/resources/locales/en.jsonsrc-tauri/resources/locales/ru.jsonsrc-tauri/resources/locales/zh.jsonsrc-tauri/src/api/agent_control.rssrc-tauri/src/domain/agent_control.rssrc-tauri/src/domain/integration_api/routing.rssrc-tauri/src/domain/integration_api/tests.rssrc-tauri/src/lib.rssrc/features/settings/services/SettingsService.test.tssrc/features/settings/services/SettingsService.tssrc/features/settings/ui/AgentControlSettingsRenderer.test.tssrc/features/settings/ui/AgentControlSettingsRenderer.tssrc/features/settings/ui/SettingsPageUI.test.tssrc/features/settings/ui/SettingsUI.tssrc/public/templates/pages/settings.htmlsrc/shared/types/bindings.tssrc/styles/features/console-page.csssrc/styles/features/settings-page.css
💤 Files with no reviewable changes (3)
- src-tauri/src/lib.rs
- src/features/settings/services/SettingsService.ts
- src/shared/types/bindings.ts
✅ Files skipped from review due to trivial changes (3)
- SECURITY.md
- docs/localization/en/AGENT_CONTROL.md
- docs/localization/en/ROADMAP.md
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/app/CoreStateRestore.ts (1)
76-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore path now drops any non-catalog selected module IDs.
After this change, unresolved IDs always return
null, so persisted selections outside the current catalog snapshot won’t restore and are silently downgraded to warnings. That’s a user-visible state regression for previously valid saved selections.Suggested direction
function resolveRestoredApp( catalog: CatalogService, - _category: string, + category: string, selectedModule: Partial<IApp>, ): IApp | null { @@ const catalogApp = catalog.getAppById(selectedModule.id); if (catalogApp !== undefined) { return catalogApp; } + + // Preserve restore for persisted AI selections not present in the static catalog snapshot. + if ((category === 'ai_text' || category === 'ai_image') && selectedModule.name) { + return selectedModule as IApp; + } return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/CoreStateRestore.ts` around lines 76 - 89, The restore currently returns null for any selectedModule.id not found in catalog (via getAppById), which drops persisted selections; instead, when typeof selectedModule.id === 'string' but catalog.getAppById(selectedModule.id) returns undefined, construct and return a fallback IApp that preserves the persisted id and any available fields from selectedModule (e.g., name, version) and sets sensible defaults for required IApp properties plus a flag/marker (e.g., missingFromCatalog or source: 'restored') so callers can detect a non-catalog app; update the function handling around selectedModule, catalogApp and IApp construction to cast/populate fields from Partial<IApp> and avoid returning null for unresolved IDs.src-tauri/src/domain/integration_api/mod.rs (1)
289-305:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRestore bearer-token authorization in HTTP preflight.
After loopback validation, all non-health requests are accepted without checking
AXELATE_HTTP_API_TOKEN. That allows any local process to call integration endpoints and bypasses module-scoped token gating.As per coding guidelines, "src-tauri/src/**/*.rs: Review as a Windows-first Tauri backend. Prioritize IPC boundary validation..."Suggested fix
fn preflight_http_request( request: &types::HttpRequest, peer_addr: Option<std::net::SocketAddr>, ) -> Option<types::HttpResponse> { if !is_loopback_peer(peer_addr) { return Some(json_error( 403, "Launcher API only accepts loopback clients", )); } let path = http::request_path(request); if request.method == "GET" && path == "/v1/health" { return None; } + if !auth::is_authorized(&request.headers) { + return Some(json_error(401, "Unauthorized")); + } + None }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/domain/integration_api/mod.rs` around lines 289 - 305, preflight_http_request currently skips token checks for non-health requests; restore bearer-token auth by reading the configured token (e.g., AXELATE_HTTP_API_TOKEN) and rejecting requests without a matching Authorization: Bearer <token> header. Modify preflight_http_request (taking types::HttpRequest and returning Option<types::HttpResponse>) so after the loopback check and after allowing GET /v1/health, read the incoming Authorization header (use your existing http helpers or request.headers) and if a token is configured but the header is missing or doesn’t match, return Some(json_error(401, "Unauthorized")); otherwise continue to accept the request (return None).src-tauri/src/domain/ai/session.rs (1)
103-137:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRetry path waits longer than advertised.
After the failure branches sleep
retry_delay, the re-notified loop still hits the unconditional debounce at Line 103. Failed saves therefore retry afterretry_delay + SAVE_DEBOUNCE_DELAY, while the log message andsave_retry_delay()tests describe only the backoff delay. Retry inline or bypass the debounce for failure requeues.src-tauri/src/api/system/logs.rs (1)
78-80:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCanonicalize console view ID before resolving log target.
open_console_log_targetskips canonicalization, unlikeget_console_logsandclear_console_logs. This can cause folder-open to fail for equivalent IDs with spacing/alias formatting.Suggested fix
pub fn open_console_log_target(view_id: String) -> Result<(), AppError> { - let target = resolve_console_log_target(&view_id)?; + let canonical_view_id = canonical_console_view_id(&view_id); + let target = resolve_console_log_target(&canonical_view_id)?; fs::create_dir_all(&target)?; open_folder(&target)?; Ok(()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/api/system/logs.rs` around lines 78 - 80, open_console_log_target currently uses the raw view_id while get_console_logs and clear_console_logs canonicalize IDs first; modify open_console_log_target to first call the same canonicalization helper (e.g., canonicalize_console_view_id or the existing canonicalize function used by get_console_logs/clear_console_logs) on the incoming view_id, then pass the canonicalized ID to resolve_console_log_target and to fs::create_dir_all so equivalent IDs with spacing/aliases resolve to the same folder.src-tauri/src/domain/modules/controller/script_runtime.rs (1)
638-648:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude package manager in JS dependency stamp invalidation.
Changing
runtime.package_manager(e.g., npm→bun) with unchanged dependency file currently skips reinstall due to stamp cache hit, which can leave an incompatible env.Suggested fix
- let dependencies_hash = compute_sha256(&dependencies_path)?; + let dependencies_hash = compute_sha256(&dependencies_path)?; + let manager = resolve_js_package_manager(manifest, default_package_manager)?; + let install_fingerprint = format!("{manager}:{dependencies_hash}"); let stamp_path = js_dependencies_stamp_path(env_dir); if stamp_path.exists() && fs::read_to_string(&stamp_path) .map(|value| value.trim().to_string()) .ok() - .is_some_and(|value| value == dependencies_hash) + .is_some_and(|value| value == install_fingerprint) { return Ok(()); } @@ - let manager = resolve_js_package_manager(manifest, default_package_manager)?; let package_manager = find_program(manager).await?; @@ - fs::write(&stamp_path, format!("{dependencies_hash}\n")).map_err(|e| { + fs::write(&stamp_path, format!("{install_fingerprint}\n")).map_err(|e| {Also applies to: 668-694
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/domain/modules/controller/script_runtime.rs` around lines 638 - 648, The dependency-stamp logic (using js_dependencies_stamp_path, compute_sha256, dependencies_hash, and stamp_path) only considers the dependency file hash and thus ignores changes to runtime.package_manager; update the stamp generation and comparison to include the package manager (e.g., concatenate or hash runtime.package_manager with the dependencies file contents to produce dependencies_hash) so that a package_manager change invalidates the stamp and forces reinstall; apply the same fix to the other identical block referenced around the 668-694 region so both checks use the combined hash.
♻️ Duplicate comments (1)
src/features/console/ui/ConsoleFilterControlHelper.ts (1)
162-167:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove hardcoded English fallback strings from user-visible TS labels.
These
_t(key, '...')calls still embed user-facing English in frontend code (Confirm clear...,Right-click again...,Clear Console). Move the visible fallback text responsibility to locale resources/central i18n resolution and keep this file key-only.As per coding guidelines: "Keep UI text in I18n resources under
src-tauri/resources/locales/and do not hardcode Russian or English user-facing strings in TS/HTML".Also applies to: 182-189, 209-214
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/console/ui/ConsoleFilterControlHelper.ts` around lines 162 - 167, Replace hardcoded English fallback strings in all this._t(...) calls inside ConsoleFilterControlHelper (e.g., the calls used to set aria-label, button.title and other UI labels) by passing only the i18n key (for example use this._t('ui.debug.logs_clear_confirm') instead of this._t('ui.debug.logs_clear_confirm', 'Confirm clear console logs')); move the visible fallback texts into your locale resources under the app’s i18n files (add the corresponding entries for keys like ui.debug.logs_clear_confirm, ui.debug.logs_clear_confirm_title, and the keys referenced around the other callsites) and then remove the second-argument fallback from each this._t invocation so the component is key-only.
🧹 Nitpick comments (5)
src-tauri/src/api/ai/mod.rs (1)
760-761: ⚡ Quick winRefactor to avoid computing unused
pathon macOS.On macOS,
pathis computed (line 759) but never used—the command block at line 777 usesfileinstead. Thelet _ = &path;suppresses the compiler warning, but it's cleaner to conditionally computepathonly on platforms that use it, or refactor the macOS flow to usepathconsistently.♻️ Option 1: Conditionally compute path
+#[cfg(not(target_os = "macos"))] let path = image_open_directory(&file, open_folder_only)?; -#[cfg(target_os = "macos")] -let _ = &path;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/api/ai/mod.rs` around lines 760 - 761, The code computes `path` on all platforms but only uses `file` in the macOS command block, so remove the unused `path` allocation on macOS by either 1) making its declaration conditional (e.g., #[cfg(not(target_os = "macos"))] let path = ...) so `path` is only computed where used, or 2) make the macOS branch use `path` consistently instead of `file` (update the macOS command block to reference `path`), ensuring you remove the temporary `let _ = &path;` suppression and keep only the needed variable depending on the branch.src/features/console/services/ConsoleLogNormalizer.ts (1)
15-25: ⚖️ Poor tradeoffConsider documenting the fallback normalization contract.
The normalizer now preserves pre-populated fields (
display_time,normalized_level,scope, etc.) and only computes values when they are missing. This assumes upstream callers apply the same normalization rules. If normalization logic evolves, pre-populated fields won't be re-normalized, potentially causing inconsistencies.Consider adding a comment or type annotation clarifying which callers are expected to pre-normalize, or enforce a single normalization path to ensure consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/console/services/ConsoleLogNormalizer.ts` around lines 15 - 25, The current ConsoleLogNormalizer implementation conditionally preserves pre-populated fields (display_time, normalized_level, scope, summary_message, source_label, source_class, page, action, expected) instead of always applying normalization, which can lead to inconsistent logs if upstream callers mutate normalization rules; update ConsoleLogNormalizer (the normalization object creation and the methods _formatSourceLabel, _extractPage, _extractAction, _extractExpected) to enforce a single normalization path by either (a) documenting the contract clearly near the class that callers must pass already-normalized fields, or preferably (b) always running the normalizer logic and overriding incoming values so normalization rules are applied centrally; add a concise comment above ConsoleLogNormalizer explaining the chosen approach and ensure source_class logic (module: check) and source_label formatting remain applied when overriding to preserve existing behavior.src/app/CoreStateRestore.test.ts (1)
67-69: ⚡ Quick winTighten this test to match catalog contract and avoid permissive pass
Use
nullfor missinggetAppByIdresults and assert exact call count/order;toHaveBeenCalledWithalone can pass even with extra unexpected calls.Suggested diff
catalog: { getAppById: (appId: string) => - appId === CUSTOM_TEXT_PROVIDER_ID ? customTextApp : undefined, + appId === CUSTOM_TEXT_PROVIDER_ID ? customTextApp : null, } as never, @@ - expect(updateModuleCard).toHaveBeenCalledWith('ai_text', customTextApp); + expect(updateModuleCard).toHaveBeenCalledTimes(1); + expect(updateModuleCard).toHaveBeenNthCalledWith(1, 'ai_text', customTextApp);Also applies to: 75-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/CoreStateRestore.test.ts` around lines 67 - 69, The test currently returns undefined for missing apps and only checks toHaveBeenCalledWith which can hide extra calls; update the mock getAppById to return null instead of undefined for non-matching appIds (keep the CUSTOM_TEXT_PROVIDER_ID => customTextApp mapping) and add explicit call count/order assertions (e.g., use toHaveBeenCalledTimes and toHaveBeenNthCalledWith or equivalent) to validate exact invocation sequence of getAppById in CoreStateRestore.test.ts.src/features/settings/ui/SettingsPageUI.test.ts (1)
57-58: 💤 Low valueRedundant mock cleanup.
vi.restoreAllMocks()already includes clearing spies. The precedingvi.clearAllMocks()is redundant.vi.clearAllMocks(); - vi.restoreAllMocks();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/settings/ui/SettingsPageUI.test.ts` around lines 57 - 58, The test cleanup calls both vi.clearAllMocks() and vi.restoreAllMocks(); remove the redundant vi.clearAllMocks() call and keep only vi.restoreAllMocks() in the afterEach/teardown so mocks/spies are both cleared and restored (update the code around vi.clearAllMocks() / vi.restoreAllMocks() in SettingsPageUI.test.ts).src/features/ai/ui/AISettingsViewPolicy.ts (1)
4-10: ⚡ Quick winTighten
isCleanAppinput contract to avoid silent false negatives.
stringinputs always returnfalsein the current implementation. Either removestringfrom the signature or implement explicit string-id lookup so valid callers don’t get misleading results.Suggested fix (type-tightening)
- public isCleanApp(app: IApp | string): boolean { - const policy = typeof app === 'string' ? null : app.providerPolicy; - if (policy?.isCleanApp !== undefined) { - return policy.isCleanApp; - } - - return false; - } + public isCleanApp(app: IApp): boolean { + return app.providerPolicy?.isCleanApp ?? false; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/ai/ui/AISettingsViewPolicy.ts` around lines 4 - 10, The isCleanApp method currently accepts IApp | string but treats string IDs as null and returns false, causing silent false negatives; either tighten the signature to accept only IApp (remove string from isCleanApp(…) and update all callers to resolve IDs before calling) or implement an explicit ID lookup path inside isCleanApp by resolving the string to an IApp (e.g., via your app registry/lookup API) then reading providerPolicy.isCleanApp; update references to the method accordingly and ensure providerPolicy and isCleanApp checks remain the same once the app object is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 148-150: The job-level "continue-on-error: true" makes the entire
cross-platform job non-blocking; remove that key from the job definition and
keep "strategy: fail-fast: false" only so the job fails CI on regressions while
still running all matrix entries, or if you truly need non-blocking behavior
make the failure explicit by applying "continue-on-error" to specific steps or
using an allowed_failures matrix entry instead; update the CI YAML to delete the
"continue-on-error: true" line and ensure any intentional non-blocking behavior
is limited to named steps or matrix configuration.
In `@docs/localization/en/CURRENT_STATE.md`:
- Around line 308-311: Remove the four bullets that are already implemented so
the “not yet” list matches reality: delete the lines containing "agent scopes
and approval prompts", "sanitized console log APIs meant for external agents",
"integration draft generation through the launcher", and "an Axelate MCP server
backed by documented launcher capabilities" from the CURRENT_STATE not-yet list
so they no longer contradict the new sections that document these features as
current behavior.
In `@docs/localization/en/TRUST_MODEL.md`:
- Around line 106-108: Update the TRUST_MODEL.md wording so it matches the
actual implementation: remove the claim "The full token is shown once" and
instead state the token is available via the copy-only flow (or reference the
exact non-frontend reveal API if one exists); mention the profile types "Trusted
Local" and "Full Access" and the exposed function/endpoint name
copyAgentProfileToken so readers know where token retrieval occurs, and note
that tokens can be rotated or deleted by the user via the existing local tool or
API.
In `@docs/localization/zh/README.md`:
- Line 28: Remove the future-feature clause "MCP、SDK 和完整的包权限模型" from the README
index line that currently reads "第一版本地 Agent Control 已在 `AGENT_CONTROL.md`
中说明；MCP、SDK 和完整的包权限模型仍是后续阶段.", and instead add that roadmap detail as a short
entry in your planning doc (create or append to VISION.md or ROADMAP.md) so
README stays focused on current-state docs; keep the README line limited to
referencing AGENT_CONTROL.md and move the exact phrase "MCP、SDK 和完整的包权限模型仍是后续阶段"
into the new or existing VISION.md/ROADMAP.md with a brief roadmap note.
In `@src-tauri/resources/locales/en.json`:
- Line 269: The JSON key "ui.launcher.settings.agent_control_trusted_local" is
mapped to the incorrect string "Selected Permissions"; update its value to a
semantically correct label for the "trusted_local" concept (for example "Trusted
Local Agents" or "Trusted Local Devices" depending on UI context) so the UI
displays the intended meaning; locate the key "agent_control_trusted_local" in
the locales file and replace the string value with the appropriate label used
elsewhere for trusted-local semantics.
In `@src-tauri/src/api/system/config.rs`:
- Line 193: The snapshot builder currently sets config_schema only from
installed_module (installed_module.and_then(|module|
module.config_schema.clone())), which causes catalog-only items to lose their
ModuleItem.config_schema; change the assignment to fall back to the catalog
item's module schema when installed_module is None (e.g.
installed_module.and_then(|m| m.config_schema.clone()).or_else(||
catalog_item.module.as_ref().and_then(|m| m.config_schema.clone()))),
referencing config_schema, installed_module, and ModuleItem.config_schema to
locate the fields to update.
In `@src-tauri/src/domain/integration_api/routing.rs`:
- Around line 123-167: The routes handling "/v1/launcher/open-page",
"/v1/launcher/select-module", and "/v1/integration-drafts" currently rely only
on ensure_agent_scope (e.g., ensure_agent_scope(client, AgentScope::Operate) and
AgentScope::DraftCreate) which permits AuthorizedClient::Module(_); add an
explicit guard to reject module-scoped tokens before performing the action:
after the ensure_agent_scope call in the blocks for handle_open_page_request,
handle_select_module_request, and handle_create_integration_draft_request, check
the client identity and return an authorization error if the client is a Module
(AuthorizedClient::Module(_)); keep the existing audit calls
(record_agent_audit) and responses unchanged but ensure the Module case is
denied to prevent launcher-wide mutation from module-scoped tokens.
In `@src/app/CoreLifecycleController.ts`:
- Around line 297-301: The event callback currently calls void
this._applyAgentOpenPageRequest(payload) which ignores rejections and can cause
unhandled promise rejections if showPage(...) inside fails; update the
tauriProvider.listen callbacks (the one using this._applyAgentOpenPageRequest at
the 'agent-control:open-page' listener and the other occurrence at lines
~319-320) to handle the returned promise by either awaiting it in an async
callback or appending .catch(...) to log/report errors (e.g., call
this._applyAgentOpenPageRequest(payload).catch(err => /* log or handle err */))
so all rejections are caught and handled.
In `@src/features/ai/ui/AISettingsRenderer.ts`:
- Around line 399-402: The fallback string "Manual OpenAI-compatible model ID"
is hardcoded in AISettingsRenderer (the desc: translate(...) call); remove the
inline English string and add the text to your i18n locale files under the key
"ui.settings.custom_model_desc" (and translations for other locales), then call
translate('ui.settings.custom_model_desc') (or supply a non-user-facing
fallback) from the desc property so no user-facing text remains in TS.
- Around line 567-569: _getKeyProviderUrl currently returns a fallback '#' which
can be passed to shell-open; instead validate the resolved URL's protocol and
only return it if it's http or https (or return null) to prevent unsafe
shell-open calls. Locate _getKeyProviderUrl and its use of
_getActiveProviderPolicy, change the function to parse/validate the URL scheme
(allow only "http:" or "https:") and return null when invalid; update callers
that call openUrl (e.g., places invoking openUrl with _getKeyProviderUrl) to
no-op when the function returns null so openUrl is only called with safe public
protocols. Ensure similar validation is applied to the other occurrences noted
(lines referenced 256-258) that feed external URLs to shell-open.
In `@src/public/templates/pages/settings.html`:
- Around line 5-10: Remove the hardcoded English fallback text inside the
elements that already use data-i18n: clear the inner text of the <div
class="module-title" data-i18n="ui.launcher.settings.taskbar_title"> and the
<div class="module-desc" data-i18n="ui.launcher.settings.taskbar_desc"> so they
contain no hardcoded strings; instead ensure the keys
ui.launcher.settings.taskbar_title and ui.launcher.settings.taskbar_desc exist
in the src-tauri/resources/locales/ i18n resource files for all supported
languages (also remove/clear similar inline text in the other affected cards
around the same block, noted in the comment for lines ~15-20).
---
Outside diff comments:
In `@src-tauri/src/api/system/logs.rs`:
- Around line 78-80: open_console_log_target currently uses the raw view_id
while get_console_logs and clear_console_logs canonicalize IDs first; modify
open_console_log_target to first call the same canonicalization helper (e.g.,
canonicalize_console_view_id or the existing canonicalize function used by
get_console_logs/clear_console_logs) on the incoming view_id, then pass the
canonicalized ID to resolve_console_log_target and to fs::create_dir_all so
equivalent IDs with spacing/aliases resolve to the same folder.
In `@src-tauri/src/domain/integration_api/mod.rs`:
- Around line 289-305: preflight_http_request currently skips token checks for
non-health requests; restore bearer-token auth by reading the configured token
(e.g., AXELATE_HTTP_API_TOKEN) and rejecting requests without a matching
Authorization: Bearer <token> header. Modify preflight_http_request (taking
types::HttpRequest and returning Option<types::HttpResponse>) so after the
loopback check and after allowing GET /v1/health, read the incoming
Authorization header (use your existing http helpers or request.headers) and if
a token is configured but the header is missing or doesn’t match, return
Some(json_error(401, "Unauthorized")); otherwise continue to accept the request
(return None).
In `@src-tauri/src/domain/modules/controller/script_runtime.rs`:
- Around line 638-648: The dependency-stamp logic (using
js_dependencies_stamp_path, compute_sha256, dependencies_hash, and stamp_path)
only considers the dependency file hash and thus ignores changes to
runtime.package_manager; update the stamp generation and comparison to include
the package manager (e.g., concatenate or hash runtime.package_manager with the
dependencies file contents to produce dependencies_hash) so that a
package_manager change invalidates the stamp and forces reinstall; apply the
same fix to the other identical block referenced around the 668-694 region so
both checks use the combined hash.
In `@src/app/CoreStateRestore.ts`:
- Around line 76-89: The restore currently returns null for any
selectedModule.id not found in catalog (via getAppById), which drops persisted
selections; instead, when typeof selectedModule.id === 'string' but
catalog.getAppById(selectedModule.id) returns undefined, construct and return a
fallback IApp that preserves the persisted id and any available fields from
selectedModule (e.g., name, version) and sets sensible defaults for required
IApp properties plus a flag/marker (e.g., missingFromCatalog or source:
'restored') so callers can detect a non-catalog app; update the function
handling around selectedModule, catalogApp and IApp construction to
cast/populate fields from Partial<IApp> and avoid returning null for unresolved
IDs.
---
Duplicate comments:
In `@src/features/console/ui/ConsoleFilterControlHelper.ts`:
- Around line 162-167: Replace hardcoded English fallback strings in all
this._t(...) calls inside ConsoleFilterControlHelper (e.g., the calls used to
set aria-label, button.title and other UI labels) by passing only the i18n key
(for example use this._t('ui.debug.logs_clear_confirm') instead of
this._t('ui.debug.logs_clear_confirm', 'Confirm clear console logs')); move the
visible fallback texts into your locale resources under the app’s i18n files
(add the corresponding entries for keys like ui.debug.logs_clear_confirm,
ui.debug.logs_clear_confirm_title, and the keys referenced around the other
callsites) and then remove the second-argument fallback from each this._t
invocation so the component is key-only.
---
Nitpick comments:
In `@src-tauri/src/api/ai/mod.rs`:
- Around line 760-761: The code computes `path` on all platforms but only uses
`file` in the macOS command block, so remove the unused `path` allocation on
macOS by either 1) making its declaration conditional (e.g., #[cfg(not(target_os
= "macos"))] let path = ...) so `path` is only computed where used, or 2) make
the macOS branch use `path` consistently instead of `file` (update the macOS
command block to reference `path`), ensuring you remove the temporary `let _ =
&path;` suppression and keep only the needed variable depending on the branch.
In `@src/app/CoreStateRestore.test.ts`:
- Around line 67-69: The test currently returns undefined for missing apps and
only checks toHaveBeenCalledWith which can hide extra calls; update the mock
getAppById to return null instead of undefined for non-matching appIds (keep the
CUSTOM_TEXT_PROVIDER_ID => customTextApp mapping) and add explicit call
count/order assertions (e.g., use toHaveBeenCalledTimes and
toHaveBeenNthCalledWith or equivalent) to validate exact invocation sequence of
getAppById in CoreStateRestore.test.ts.
In `@src/features/ai/ui/AISettingsViewPolicy.ts`:
- Around line 4-10: The isCleanApp method currently accepts IApp | string but
treats string IDs as null and returns false, causing silent false negatives;
either tighten the signature to accept only IApp (remove string from
isCleanApp(…) and update all callers to resolve IDs before calling) or implement
an explicit ID lookup path inside isCleanApp by resolving the string to an IApp
(e.g., via your app registry/lookup API) then reading providerPolicy.isCleanApp;
update references to the method accordingly and ensure providerPolicy and
isCleanApp checks remain the same once the app object is resolved.
In `@src/features/console/services/ConsoleLogNormalizer.ts`:
- Around line 15-25: The current ConsoleLogNormalizer implementation
conditionally preserves pre-populated fields (display_time, normalized_level,
scope, summary_message, source_label, source_class, page, action, expected)
instead of always applying normalization, which can lead to inconsistent logs if
upstream callers mutate normalization rules; update ConsoleLogNormalizer (the
normalization object creation and the methods _formatSourceLabel, _extractPage,
_extractAction, _extractExpected) to enforce a single normalization path by
either (a) documenting the contract clearly near the class that callers must
pass already-normalized fields, or preferably (b) always running the normalizer
logic and overriding incoming values so normalization rules are applied
centrally; add a concise comment above ConsoleLogNormalizer explaining the
chosen approach and ensure source_class logic (module: check) and source_label
formatting remain applied when overriding to preserve existing behavior.
In `@src/features/settings/ui/SettingsPageUI.test.ts`:
- Around line 57-58: The test cleanup calls both vi.clearAllMocks() and
vi.restoreAllMocks(); remove the redundant vi.clearAllMocks() call and keep only
vi.restoreAllMocks() in the afterEach/teardown so mocks/spies are both cleared
and restored (update the code around vi.clearAllMocks() / vi.restoreAllMocks()
in SettingsPageUI.test.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6111a9f-82d3-44b1-ba2d-e4f62f42859e
⛔ Files ignored due to path filters (3)
src-tauri/Cargo.lockis excluded by!**/*.locksrc/assets/fonts/Cubic_11.zh-subset.woff2is excluded by!**/*.woff2src/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (133)
.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/dependency-review.yml.github/workflows/release.yml.github/workflows/security-audit.ymlSECURITY.mddocs/examples/sdk/javascript/axelate-client.mjsdocs/examples/sdk/python/axelate_sdk.pydocs/localization/en/AGENT_CONTROL.mddocs/localization/en/ARCHITECTURE.mddocs/localization/en/CURRENT_STATE.mddocs/localization/en/DEVELOPMENT_WORKFLOW.mddocs/localization/en/INTEGRATION_API.mddocs/localization/en/INTEGRATION_DEVELOPMENT.mddocs/localization/en/README.mddocs/localization/en/ROADMAP.mddocs/localization/en/TRUST_MODEL.mddocs/localization/en/VISION.mddocs/localization/ru/INTEGRATION_DEVELOPMENT.mddocs/localization/ru/README.mddocs/localization/zh/INTEGRATION_DEVELOPMENT.mddocs/localization/zh/README.mdsrc-tauri/Cargo.tomlsrc-tauri/resources/locales/en.jsonsrc-tauri/resources/locales/ru.jsonsrc-tauri/resources/locales/zh.jsonsrc-tauri/src/api/agent_control.rssrc-tauri/src/api/ai/mod.rssrc-tauri/src/api/mod.rssrc-tauri/src/api/modules/mod.rssrc-tauri/src/api/system/config.rssrc-tauri/src/api/system/console_overview.rssrc-tauri/src/api/system/log_targets.rssrc-tauri/src/api/system/logs.rssrc-tauri/src/api/system/mod.rssrc-tauri/src/app/window.rssrc-tauri/src/domain/agent_control.rssrc-tauri/src/domain/ai/ai_dispatch.rssrc-tauri/src/domain/ai/ai_provider_resolution.rssrc-tauri/src/domain/ai/ai_service.rssrc-tauri/src/domain/ai/ai_validation.rssrc-tauri/src/domain/ai/mod.rssrc-tauri/src/domain/ai/provider_payload.rssrc-tauri/src/domain/ai/session.rssrc-tauri/src/domain/ai/session_context.rssrc-tauri/src/domain/ai/streaming.rssrc-tauri/src/domain/ai/streaming_chunks.rssrc-tauri/src/domain/engine/engine_ids.rssrc-tauri/src/domain/engine/manager.rssrc-tauri/src/domain/engine/mod.rssrc-tauri/src/domain/integration_api/auth.rssrc-tauri/src/domain/integration_api/mod.rssrc-tauri/src/domain/integration_api/routing.rssrc-tauri/src/domain/integration_api/tests.rssrc-tauri/src/domain/integration_api/types.rssrc-tauri/src/domain/mod.rssrc-tauri/src/domain/modules/controller/lifecycle.rssrc-tauri/src/domain/modules/controller/mod.rssrc-tauri/src/domain/modules/controller/process.rssrc-tauri/src/domain/modules/controller/script_runtime.rssrc-tauri/src/domain/modules/github_release_targets.rssrc-tauri/src/domain/modules/github_releases.rssrc-tauri/src/domain/modules/mod.rssrc-tauri/src/domain/system/hardware_probe.rssrc-tauri/src/infrastructure/logging/logger.rssrc-tauri/src/infrastructure/system/startup.rssrc-tauri/src/lib.rssrc-tauri/src/models/config.rssrc-tauri/src/models/modules.rssrc-tauri/src/utils/memory.rssrc-tauri/src/utils/paths.rssrc/app/CoreLifecycleController.test.tssrc/app/CoreLifecycleController.tssrc/app/CoreServiceFactory.tssrc/app/CoreStateRestore.test.tssrc/app/CoreStateRestore.tssrc/features/ai/services/AIBridge.test.tssrc/features/ai/services/AIBridge.tssrc/features/ai/services/AIBridgeConfig.tssrc/features/ai/services/AIBridgeMessageController.test.tssrc/features/ai/services/AIBridgeMessageController.tssrc/features/ai/services/AIBridgeProviderPolicy.test.tssrc/features/ai/services/AIBridgeProviderPolicy.tssrc/features/ai/services/AIChatTransport.test.tssrc/features/ai/services/AIChatTransport.tssrc/features/ai/services/AIProviderManager.test.tssrc/features/ai/services/AIProviderManager.tssrc/features/ai/services/EngineStatusService.test.tssrc/features/ai/services/EngineStatusService.tssrc/features/ai/types/aiTypes.tssrc/features/ai/ui/AISettingsContentRenderer.tssrc/features/ai/ui/AISettingsKeyController.test.tssrc/features/ai/ui/AISettingsKeyController.tssrc/features/ai/ui/AISettingsRenderer.test.tssrc/features/ai/ui/AISettingsRenderer.tssrc/features/ai/ui/AISettingsSelectionController.tssrc/features/ai/ui/AISettingsViewPolicy.test.tssrc/features/ai/ui/AISettingsViewPolicy.tssrc/features/console/services/ConsoleLogNormalizer.tssrc/features/console/services/ConsoleLogService.test.tssrc/features/console/services/ConsoleLogService.tssrc/features/console/ui/ConsoleFilterControlHelper.tssrc/features/console/ui/ConsoleLogPresentationHelper.tssrc/features/console/ui/ConsoleUI.test.tssrc/features/console/ui/ConsoleUI.tssrc/features/console/ui/ConsoleViewHelper.tssrc/features/settings/index.tssrc/features/settings/services/SettingsService.test.tssrc/features/settings/services/SettingsService.tssrc/features/settings/ui/AgentControlSettingsRenderer.test.tssrc/features/settings/ui/AgentControlSettingsRenderer.tssrc/features/settings/ui/SettingsPageUI.test.tssrc/features/settings/ui/SettingsUI.tssrc/public/templates/pages/console.htmlsrc/public/templates/pages/settings.htmlsrc/shared/services/CatalogLoadSnapshot.tssrc/shared/services/CatalogService.test.tssrc/shared/services/CatalogService.tssrc/shared/shell/AppUI.test.tssrc/shared/shell/AppUI.tssrc/shared/shell/ui/AppUiDashboardSupport.tssrc/shared/shell/ui/card/ModuleCardRenderer.test.tssrc/shared/shell/ui/card/ModuleCardRenderer.tssrc/shared/types/bindings.tssrc/shared/types/coreTypes.tssrc/shared/utils/customProviderSupport.test.tssrc/shared/utils/customProviderSupport.tssrc/shared/utils/providerSupport.test.tssrc/shared/utils/providerSupport.tssrc/styles/features/console-page.csssrc/styles/features/settings-page.csssrc/test/helpers/catalogTestUtils.tssrc/test/integration/CatalogService.integration.test.ts
💤 Files with no reviewable changes (7)
- src/features/ai/services/EngineStatusService.ts
- src/shared/utils/providerSupport.ts
- src-tauri/src/infrastructure/system/startup.rs
- src/shared/utils/providerSupport.test.ts
- src/shared/services/CatalogLoadSnapshot.ts
- src/shared/utils/customProviderSupport.ts
- src/shared/utils/customProviderSupport.test.ts
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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
1 participant
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.
Summary
Summary by CodeRabbit
New Features
Localization
Improvements
Tests
Chores