Add CLI Tools settings UI, fix stuck cancel, DO-block limit clause, a… - #229
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe update improves Windows CLI execution and caching, adds CLI Tools management in Settings, handles PostgreSQL ChangesCLI and query reliability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CliToolsSettings
participant useCliStore
participant Tauri
participant CliManager
User->>CliToolsSettings: select CLI tool version
CliToolsSettings->>useCliStore: download or remove cached version
useCliStore->>Tauri: invoke CLI command
Tauri->>CliManager: download or remove tool cache
CliManager-->>Tauri: return operation result
Tauri-->>useCliStore: refresh cached and detected tools
useCliStore-->>CliToolsSettings: display updated status and disk usage
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
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/cli.rs (1)
390-409: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the MySQL ZIP runtime layout.
relocate_binaries()only moves the MySQL binaries intoversioned_dir/bin; the extractedlib,share, and related installation-tree resources stay behind in the ZIP’s root directory. Move/copy the complete MySQL installation tree for Windows ZIP downloads, then keep the versioned binary path pointing inside it.🤖 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/cli.rs` around lines 390 - 409, Update relocate_binaries() to preserve the complete MySQL Windows ZIP installation tree, including lib, share, and related root resources, when selecting the extracted archive layout. Move or copy that tree into the versioned destination and ensure the resulting MySQL binary path points to its bin directory within the relocated tree, while leaving other archive types’ behavior unchanged.
🤖 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/storage.rs`:
- Around line 151-162: Serialize the machine-ID cache-miss initialization around
detect_machine_id, cache insertion, and keyring persistence using a shared lock.
In the relevant storage initialization function, recheck MACHINE_ID_CACHE after
acquiring the lock, and persist only the ID successfully established by the
winning cache set; avoid writing IDs from losing concurrent callers.
In `@src/components/editor/QueryEditor.tsx`:
- Around line 622-623: Update the statement parsing flow in QueryEditor to pass
the original untrimmed editor text to splitStatements, preserving leading blank
lines for accurate lineNumber values. Keep trimming limited to any separate
execution or display use, and continue mapping the parsed statements into
allStatements unchanged.
In `@src/components/explorer/DatabaseExplorer.tsx`:
- Around line 1649-1667: Add a nearby Vitest regression test for the DDL lookup
flow in DatabaseExplorer, using the nested trigger identifier
trig-public.orders-audit_trigger and asserting it calls getDDL with type
"trigger" and name "audit_trigger" only. Ensure the test would fail if the table
name is included in the trigger lookup.
In `@src/components/layout/MainContent.tsx`:
- Around line 552-556: Update the session restoration flow in MainContent so
sessionRestoredRef.current remains false until the load_sessions await
completes, assigning true only in the completion/finally path after restoration
has finished. Preserve the save guard in the effect, and add an executable
regression test covering delayed restoration to ensure the initial empty
queryTabs list is not saved while loading.
- Around line 2087-2090: Update the cancellation flow around isExecutingRef and
executionGenRef so a cancelled run remains prevented from overlapping a newly
started run until its promise/loop fully settles; avoid resetting shared
cancelFlagRef in a way that lets the abandoned run continue, using a per-run
cancellation token or retaining the execution gate until completion. Preserve
generation checks for stale cleanup, and add a regression test covering
cancel-then-run-again, including run-all behavior.
In `@src/components/settings/SettingsDialog.tsx`:
- Around line 1531-1535: Update the cache-removal handler around
cli.removeCached to catch rejected deletions and surface the error through the
SettingsDialog UI or existing toast mechanism. Preserve the finally cleanup that
calls setRemovingKey(null), and ensure failures no longer propagate as unhandled
event-handler rejections.
In `@src/utils/applyQueryLimit.test.ts`:
- Around line 87-106: Update applyQueryLimit’s DO-block detection to recognize
an optional LANGUAGE clause between DO and the dollar-quoted or string body,
preventing LIMIT from being appended to forms such as DO LANGUAGE plpgsql. Add a
regression test covering this syntax and preserve unchanged behavior for
existing DO blocks.
In `@src/utils/applyQueryLimit.ts`:
- Around line 21-24: Centralize the DO-statement predicate used by
applyQueryLimit so it recognizes both dollar/quote forms and DO LANGUAGE plpgsql
statements, then reuse it in the run-all classification logic at
src/components/layout/MainContent.tsx lines 1003-1011, 1363-1372, and 1684-1693
using the cleaned statement text rather than raw text. Update
src/utils/applyQueryLimit.ts lines 21-24 to consume the shared predicate, and
add a regression case for DO LANGUAGE plpgsql in
src/utils/applyQueryLimit.test.ts lines 87-106.
---
Outside diff comments:
In `@src-tauri/src/cli.rs`:
- Around line 390-409: Update relocate_binaries() to preserve the complete MySQL
Windows ZIP installation tree, including lib, share, and related root resources,
when selecting the extracted archive layout. Move or copy that tree into the
versioned destination and ensure the resulting MySQL binary path points to its
bin directory within the relocated tree, while leaving other archive types’
behavior unchanged.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f8c3b5b0-9861-4397-88d1-e2a736c5aa39
📒 Files selected for processing (13)
CHANGELOG.mdsrc-tauri/src/cli.rssrc-tauri/src/lib.rssrc-tauri/src/storage.rssrc/components/editor/QueryEditor.tsxsrc/components/explorer/DatabaseExplorer.tsxsrc/components/layout/MainContent.tsxsrc/components/settings/SettingsDialog.tsxsrc/lib/ipc.tssrc/store/cliStore.tssrc/store/savedQueryStore.tssrc/utils/applyQueryLimit.test.tssrc/utils/applyQueryLimit.ts
…nd Windows CLI/session bugs - #39: Settings -> CLI Tools management UI (status, cached versions with disk usage, remove-cached, pre-download picker) backed by new cli_remove_cached command - #212: Cancel Query left the app unable to run further queries; the abandoned run's finally block could still clobber isExecuting/generation state after cancellation - applyQueryLimit / query executor no longer appends a LIMIT clause to PL/pgSQL DO $$ ... $$ blocks containing RETURNING - Double-clicking a table-nested trigger no longer mis-resolves its DDL lookup path - Saved query tabs are no longer wiped from sessions.json by a save/restore race on launch - MySQL CLI auto-download now works on Windows (missing URL + binary relocation for .exe-suffixed archives, shared with mongosh) - CLI subprocess calls no longer flash a console window on Windows - Saved queries no longer silently disappear on Windows due to unstable machine-ID detection; machine ID is now cached in the OS keyring and fingerprint/decrypt failures are now logged instead of swallowed
kix007
force-pushed
the
feat/cli-tools-settings-ui-and-windows-fixes
branch
from
July 29, 2026 07:21
39b4835 to
a9438ec
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/components/explorer/DatabaseExplorer.tsx`:
- Around line 1649-1664: Update the trigger DDL resolution flow around getDDL so
it passes the trigger’s schema and parent table, rather than relying on
node.name/tgname alone. Extend the lookup contract and pg_trigger query to
constrain by schema, parent relation, and trigger name, preserving the intended
trigger definition when names duplicate across tables.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4691dd6c-9a37-4809-bec4-945337f612bc
📒 Files selected for processing (13)
CHANGELOG.mdsrc-tauri/src/cli.rssrc-tauri/src/lib.rssrc-tauri/src/storage.rssrc/components/editor/QueryEditor.tsxsrc/components/explorer/DatabaseExplorer.tsxsrc/components/layout/MainContent.tsxsrc/components/settings/SettingsDialog.tsxsrc/lib/ipc.tssrc/store/cliStore.tssrc/store/savedQueryStore.tssrc/utils/applyQueryLimit.test.tssrc/utils/applyQueryLimit.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- src/utils/applyQueryLimit.ts
- src/store/savedQueryStore.ts
- src/utils/applyQueryLimit.test.ts
- src/store/cliStore.ts
- src-tauri/src/lib.rs
- src/components/editor/QueryEditor.tsx
- src-tauri/src/storage.rs
- src/lib/ipc.ts
- src-tauri/src/cli.rs
- src/components/settings/SettingsDialog.tsx
- CHANGELOG.md
- src/components/layout/MainContent.tsx
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
- Document the two trigger node.id formats (schema-level vs table-nested) and the limitation with in quoted identifiers - Clarify that cancelFlagRef is not legacy — it is still the primary cancel signal for the CLI psql path (run-all, \watch, post-exec bail, catch block)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/contexts/ConnectionContext.tsx`:
- Around line 1333-1347: The trigger lookup logic must preserve schema context
when only schema is provided and must not silently choose among duplicate
matches. Update the query branching around currentDb.select to add a
schema-filtered lookup for schema without table, and validate the returned rows
so multiple matches are reported as ambiguous rather than always using
result[0]; retain the existing schema-and-table constraint and name-only
fallback for their respective cases.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 322bacad-080a-4b11-808d-fd7262bca458
📒 Files selected for processing (5)
src-tauri/src/storage.rssrc/components/explorer/DatabaseExplorer.tsxsrc/components/layout/MainContent.tsxsrc/components/settings/SettingsDialog.tsxsrc/contexts/ConnectionContext.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src-tauri/src/storage.rs
- src/components/explorer/DatabaseExplorer.tsx
- src/components/layout/MainContent.tsx
- src/components/settings/SettingsDialog.tsx
Comment on lines
+1333
to
+1347
| let result; | ||
| // If schema and table are provided, constrain lookup to avoid ambiguity | ||
| // when the same trigger name exists on different tables | ||
| if (schema && table) { | ||
| result = await currentDb.select(` | ||
| SELECT pg_get_triggerdef(t.oid) | ||
| FROM pg_trigger t | ||
| JOIN pg_class c ON t.tgrelid = c.oid | ||
| JOIN pg_namespace n ON c.relnamespace = n.oid | ||
| WHERE t.tgname = $1 AND n.nspname = $2 AND c.relname = $3 | ||
| `, [name, schema, table]); | ||
| } else { | ||
| // Fallback: look up by trigger name alone (may be ambiguous) | ||
| result = await currentDb.select("SELECT pg_get_triggerdef(oid) FROM pg_trigger WHERE tgname = $1", [name]); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve schema context for schema-only trigger lookups.
src/components/explorer/DatabaseExplorer.tsx passes triggerSchema for trig-<schema>.<triggerName> nodes while table is undefined. This enters the fallback query, which searches every schema by trigger name and returns the first row. Duplicate trigger names can therefore open the wrong DDL. Add a schema-filtered branch, and report ambiguity when multiple matches remain instead of blindly using result[0].
🤖 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/contexts/ConnectionContext.tsx` around lines 1333 - 1347, The trigger
lookup logic must preserve schema context when only schema is provided and must
not silently choose among duplicate matches. Update the query branching around
currentDb.select to add a schema-filtered lookup for schema without table, and
validate the returned rows so multiple matches are reported as ambiguous rather
than always using result[0]; retain the existing schema-and-table constraint and
name-only fallback for their respective cases.
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
2 participants
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
Related issue
Changes
Testing notes
Checklist
npm run tauri devornpm run tauri build) and verified the change works.npx tsc --noEmit).cargo check --manifest-path src-tauri/Cargo.toml).#[tauri::command], I registered it insrc-tauri/src/lib.rs'sgenerate_handler!.CHANGELOG.mdunder## [Unreleased].storage.rs,updater.rs,capabilities/, or the AI assistant code path, I flagged it in the summary above.Summary by CodeRabbit
DOhandling (run-all execution, success messaging, and query-limit protection) and made cancellations run-scoped.DO-block regression coverage for query limiting.