Repository navigation
Add connection aliases, colours, and wrapping tabs - #84
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis change introduces connection color profiles with aliases, profile editing, profile-aware rendering, pinned query tabs, resizable result columns, coordinated sidecar connection operations, normalized and secured connection storage, Tauri profile reassignment, and expanded automated validation. ChangesConnection appearance and frontend integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsDialog
participant ConnectionProfilesPanel
participant Tauri
participant Sidecar
participant ConnectionStore
User->>SettingsDialog: Open Connections settings
SettingsDialog->>ConnectionProfilesPanel: Render profile editor
User->>ConnectionProfilesPanel: Delete custom profile
ConnectionProfilesPanel->>Tauri: Reassign linked connections
Tauri->>Sidecar: connection.reassignColorProfile
Sidecar->>ConnectionStore: Update matching profiles
ConnectionStore-->>Sidecar: updatedCount
Sidecar-->>Tauri: JSON result
Tauri-->>ConnectionProfilesPanel: Reassignment result
ConnectionProfilesPanel->>ConnectionStore: Refresh connections
ConnectionProfilesPanel->>SettingsDialog: Remove deleted profile
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Adds first-class connection appearance features (optional aliases + required colour profiles) and applies them consistently across the frontend workbench, while also improving tab UX (wrapping + pinning), results grid sizing/resizing, and hardening sidecar persistence + single-instance startup.
Changes:
- Introduces connection aliases (blank → server name) and colour profiles (built-in + custom) with settings UI and consistent markers across lists, explorer, query tabs, results tabs, and status.
- Implements pinned/unpinned wrapping query tab bands and wrapped result tabs; adds result column auto-sizing capped at 500px plus pointer/keyboard resizing.
- Hardens connection persistence and concurrency (sidecar file locking + normalization + atomic reassign), adds JSON-RPC contract test, and registers Tauri single-instance plugin.
Reviewed changes
Copilot reviewed 69 out of 74 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/frontend/queryUi.test.tsx | DOM tests for tab session restore, profile markers, wrapping, and column sizing/resizing. |
| tests/frontend/queryTabs.test.ts | Node tests for pinned tab partition/restore and column sizing helpers. |
| tests/frontend/connectionAppearanceDom.test.tsx | DOM tests for profile combobox, settings panel behavior, and deletion/reassignment flows. |
| tests/frontend/connectionAppearance.test.ts | Unit tests for alias fallback, profile normalization/validation, and appearance save rules. |
| src/shared/connectionAppearance.ts | Shared alias + colour profile utilities (built-ins, validation, migration helpers). |
| src/shared/components/ConfirmDialog.tsx | Refactors confirm dialog to native <dialog> with focus restore and busy state. |
| src/shared/components/ColorProfileMarker.tsx | Adds two-colour profile marker component for consistent UI affordance. |
| src/shared/components/ColorProfileCombobox.tsx | Adds accessible combobox control for selecting colour profiles. |
| src/features/settings/types.ts | Extends app settings with connections.colorProfiles and profile types. |
| src/features/settings/store/settingsStore.ts | Adds settings migration/persistence for profiles + storage error surfacing. |
| src/features/settings/settingsSchema.ts | Adds default settings for connection colour profiles. |
| src/features/settings/components/SettingsDialog.tsx | Adds Connections category + profile panel + improved a11y/focus + storage error display. |
| src/features/settings/components/ConnectionProfilesPanel.tsx | Settings UI to create/edit/delete custom profiles with reassignment and validation. |
| src/features/query/utils/resultColumnSizing.ts | Adds result column width estimation + clamp helpers (80–500px). |
| src/features/query/utils/queryTabs.ts | Adds pinned/unpinned partitioning + robust restoration normalization. |
| src/features/query/types.ts | Adds optional pinned state to query tabs (removes legacy connectionColor). |
| src/features/query/store/queryStore.ts | Restores normalized tabs + adds setTabPinned state action. |
| src/features/query/components/QueryTargetBar.tsx | Uses alias fallback + profile marker in the query target controls. |
| src/features/query/components/QueryTabBar.tsx | Adds pinned tab band, wrapping, pin/unpin actions, and profile-based styling/markers. |
| src/features/query/components/QueryStatusBar.tsx | Uses alias fallback + profile marker in status bar connection info. |
| src/features/query/components/QueryResultsTable.tsx | Wraps result tabs, adds profile markers, implements capped/resizable column sizing + semantics. |
| src/features/query/components/QueryPanel.tsx | Wires active connection profile through to results table. |
| src/features/explorer/store/explorerStore.ts | Uses alias fallback for server node naming. |
| src/features/explorer/components/TreeNode.tsx | Adds profile marker and alias label for server nodes. |
| src/features/connection/types.ts | Makes name nullable/optional, adds colorProfileId, and adds reassignment result type. |
| src/features/connection/store/connectionStore.ts | Adds appearance draft state, improves refresh/selection behavior, and hardens persistence flows. |
| src/features/connection/components/PropertiesTab.tsx | Adds shared appearance fields and updates save-before-connect logic. |
| src/features/connection/components/CustomTab.tsx | Removes legacy “Custom” connection tab UI. |
| src/features/connection/components/ConnectionStringTab.tsx | Adds shared appearance fields and preserves dirty state on parse-to-properties. |
| src/features/connection/components/ConnectionList.tsx | Applies alias + profile markers + selection styling + improves a11y attributes. |
| src/features/connection/components/ConnectionDialog.tsx | Removes legacy “Custom” tab from connection dialog. |
| src/features/connection/components/ConnectionAuthProgressDialog.tsx | Uses alias fallback for operation target labeling. |
| src/features/connection/components/AppearanceFields.tsx | New shared component for alias + profile selection in connection editors. |
| src/features/connection/api/connectionApi.ts | Adds API bridge for connection_reassign_color_profile. |
| src/app/App.tsx | Uses alias/profile markers in app chrome and removes legacy connectionColor propagation. |
| src-tauri/src/lib.rs | Registers single-instance plugin early and exposes new connection reassignment command. |
| src-tauri/src/commands/connection.rs | Adds JSON-RPC bridge command for connection.reassignColorProfile (with tests). |
| src-tauri/Cargo.toml | Adds tauri-plugin-single-instance dependency for desktop targets. |
| src-tauri/Cargo.lock | Locks new Rust dependencies introduced by the single-instance plugin. |
| sidecar/tests/Ssmsx.Protocol.Tests/Models/ConnectionInfoTests.cs | Updates protocol tests for nullable Name + default ColorProfileId behavior. |
| sidecar/tests/Ssmsx.Protocol.Tests/JsonRpcTests.cs | Adds protocol JSON shape tests for reassign color profile messages. |
| sidecar/tests/Ssmsx.Core.Tests/Storage/ConnectionStoreTests.cs | Adds extensive persistence/migration/concurrency tests for connection store changes. |
| sidecar/tests/Ssmsx.Core.Tests/Connections/ConnectionOperationCoordinatorTests.cs | Tests new per-connection operation serialization coordinator. |
| sidecar/tests/Ssmsx.Core.Tests/Connections/ConnectionManagerTests.cs | Tests connection disposal when deletion races with connect. |
| sidecar/src/Ssmsx.Sidecar/Program.cs | Serializes connection operations and adds reassignColorProfile handler. |
| sidecar/src/Ssmsx.Protocol/ProtocolJsonContext.cs | Adds source-gen entries for reassignColorProfile params/result. |
| sidecar/src/Ssmsx.Protocol/Models/ConnectionInfo.cs | Makes Name nullable and adds ColorProfileId with default + explicitness tracking. |
| sidecar/src/Ssmsx.Protocol/Messages/ConnectionMessages.cs | Adds reassignColorProfile params/result DTOs. |
| sidecar/src/Ssmsx.Core/Storage/ConnectionStore.cs | Adds file locking, normalization/migration, secure file perms, and atomic profile reassignment. |
| sidecar/src/Ssmsx.Core/Connections/SqlConnectionFactory.cs | Makes factory overridable and ensures failed opens dispose connections. |
| sidecar/src/Ssmsx.Core/Connections/ConnectionOperationCoordinator.cs | New coordinator to serialize per-connection sidecar operations. |
| sidecar/src/Ssmsx.Core/Connections/ConnectionManager.cs | Injects factory + hardens connect against cancellation/deletion races. |
| scripts/test-sidecar-contract.mjs | New end-to-end JSON-RPC contract test for reassignColorProfile persistence. |
| scripts/smoke-tauri-mcp.mjs | Extends smoke test to assert appearance controls semantics/styles. |
| package.json | Adds frontend test script + test deps (Testing Library, jsdom, vitest) and updates overrides. |
| .github/workflows/ci.yml | Runs frontend tests and sidecar contract test in CI. |
| .claude/skills/ssmsx-design/ui_kits/ssmsx-desktop/Workbench.jsx | Updates UI kit to show profile-based tabs/results and pinned/wrapping behavior. |
| .claude/skills/ssmsx-design/ui_kits/ssmsx-desktop/data.js | Updates UI kit sample data to use profiles + alias. |
| .claude/skills/ssmsx-design/ui_kits/ssmsx-desktop/ConnectDialog.jsx | Updates UI kit connect dialog to use alias + profile selection (removes old color picker). |
| .claude/skills/ssmsx-design/tokens/radius.css | Updates token comments to reference profile markers vs legacy dots. |
| .claude/skills/ssmsx-design/tokens/colors.css | Replaces legacy connection dot tokens with profile bg/fg token pairs. |
| .claude/skills/ssmsx-design/SKILL.md | Updates design skill copy to require connection colour profiles. |
| .claude/skills/ssmsx-design/readme.md | Updates design foundations to describe connection colour profiles and usage rules. |
| .claude/skills/ssmsx-design/guidelines/colors-connection.card.html | Updates guideline card to show profile pairs + updated description. |
| .claude/skills/ssmsx-design/components/navigation/QueryTab.jsx | Updates UI kit QueryTab component to use profiles + pin state/context menu support. |
| .claude/skills/ssmsx-design/components/navigation/QueryTab.d.ts | Updates typings for UI kit QueryTab to use profile props + pin/context menu. |
| .claude/skills/ssmsx-design/components/navigation/navigation.card.html | Updates UI kit navigation card to show wrapping pinned/unpinned bands. |
| .claude/skills/ssmsx-design/components/explorer/TreeRow.jsx | Updates UI kit TreeRow to use profile foreground marker. |
| .claude/skills/ssmsx-design/components/explorer/TreeRow.d.ts | Updates typings for UI kit TreeRow marker prop rename. |
| .claude/skills/ssmsx-design/components/explorer/explorer.card.html | Updates UI kit explorer card for profile marker token usage. |
| .claude/skills/ssmsx-design/components/data/data.card.html | Updates UI kit data card to use alias/profile props. |
| .claude/skills/ssmsx-design/components/data/ConnectionItem.jsx | Updates UI kit ConnectionItem to use profile background/foreground + alias. |
| .claude/skills/ssmsx-design/components/data/ConnectionItem.d.ts | Updates typings for UI kit ConnectionItem to require profile props + alias. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 69 out of 74 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/features/settings/store/settingsStore.ts:106
saveColorProfilescurrently validates names/colours but does not prevent profile ID collisions (duplicate IDs in the payload) or reserved IDs that match built-in profiles. This can let corrupted localStorage or programmatic calls persist invalid profiles that shadow built-ins.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
sidecar/src/Ssmsx.Protocol/Models/ConnectionInfo.cs (1)
42-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the default profile id instead of duplicating the literal.
"red"here andConnectionStore.DefaultColorProfileIdmust stay in sync; exposing apublic const string DefaultColorProfileId = "red";onConnectionInfoand referencing it from the store removes the drift risk.♻️ Proposed change
+ public const string DefaultColorProfileId = "red"; + [JsonPropertyName("colorProfileId")] public string ColorProfileId { - get => string.IsNullOrWhiteSpace(_colorProfileId) ? "red" : _colorProfileId; + get => string.IsNullOrWhiteSpace(_colorProfileId) ? DefaultColorProfileId : _colorProfileId; init => _colorProfileId = value; }🤖 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 `@sidecar/src/Ssmsx.Protocol/Models/ConnectionInfo.cs` around lines 42 - 50, Expose a public const string DefaultColorProfileId = "red" on ConnectionInfo, then update ConnectionStore.DefaultColorProfileId to reference ConnectionInfo.DefaultColorProfileId instead of duplicating the literal. Use the shared constant in the ColorProfileId getter as well.scripts/test-sidecar-contract.mjs (1)
76-90: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard
stdin.writeagainst EPIPE.If the sidecar has already exited, this write emits an unhandled
errorevent on the stream and crashes the script with a confusing failure instead of the pending-request rejection.♻️ Proposed tweak
- pending.set(id, { resolve, reject, timeout }); - child.stdin.write(`${JSON.stringify({ id, method, params })}\n`); + pending.set(id, { resolve, reject, timeout }); + child.stdin.write(`${JSON.stringify({ id, method, params })}\n`, (error) => { + if (error) { + clearTimeout(timeout); + pending.delete(id); + reject(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 `@scripts/test-sidecar-contract.mjs` around lines 76 - 90, Update the send function to handle errors from child.stdin.write, especially EPIPE when the sidecar has exited, by rejecting the existing pending request and clearing its timeout instead of allowing an unhandled stream error to crash the script. Preserve the current timeout rejection behavior and request tracking.tests/frontend/connectionAppearanceDom.test.tsx (1)
97-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
showModal/closepatches are installed globally and never restored.These
Object.definePropertyoverrides onHTMLDialogElement.prototypepersist for the whole worker; if another test file in the same environment relies on real dialog semantics, it inherits the stub. Consider restoring them in anafterAll.🤖 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 `@tests/frontend/connectionAppearanceDom.test.tsx` around lines 97 - 110, Restore the original HTMLDialogElement.prototype.showModal and close implementations after the tests complete. Capture each original descriptor before the overrides in beforeAll, then use afterAll to restore those descriptors so the global prototype patches from the dialog test do not leak into other tests.src-tauri/Cargo.toml (1)
28-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBroaden the single-instance dependency target gate.
#[cfg(desktop)]insrc-tauri/src/lib.rsalso matches the BSD targets covered by Tauri’sdesktopcfg, but the dependency only allows Linux/macOS/Windows. Build targets for BSD platforms will fail to resolvetauri_plugin_single_instance; include BSD targets with the dependency or gate this registration under the same three OSes.🤖 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/Cargo.toml` around lines 28 - 29, Update the target dependency declaration for tauri-plugin-single-instance to cover all desktop targets supported by the #[cfg(desktop)] registration in src-tauri/src/lib.rs, including BSD platforms, or narrow that registration to macOS, Windows, and Linux. Keep the dependency gate and registration conditions consistent so BSD builds do not reference an unavailable dependency.src/features/connection/components/ConnectionList.tsx (1)
205-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
aria-currentinstead ofaria-pressedfor row selection.
aria-pressedcommunicates a toggle button; clicking this button doesn't unpress it — it re-selects/loads the connection.aria-current="true"(or a listbox/option pattern) more accurately conveys "currently selected item in a list."🤖 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/connection/components/ConnectionList.tsx` at line 205, Update the selected connection row’s accessibility state in the component rendering aria-pressed to use aria-current="true" when isSelected and omit or unset it otherwise, preserving the existing selection and loading behavior.src/features/settings/components/SettingsDialog.tsx (1)
13-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated search-term list across two files.
CONNECTION_SEARCH_TERMSduplicates the literal term list also declared inline inConnectionProfilesPanel.tsx'sisGeneralProfileSearch. Keeping two independent copies risks the "should the Connections panel show" and "which profiles match" checks drifting out of sync.Consider extracting this list to a shared constant (e.g. in
src/shared/connectionAppearance.ts) and importing it in bothSettingsDialog.tsxandConnectionProfilesPanel.tsx.🤖 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/components/SettingsDialog.tsx` around lines 13 - 22, Extract the shared connection/profile search terms into a single exported constant, such as in connectionAppearance.ts, and remove the duplicate literal from SettingsDialog.tsx. Update both SettingsDialog.tsx and ConnectionProfilesPanel.tsx, including isGeneralProfileSearch, to import and reuse that constant so panel visibility and profile matching remain synchronized.src/features/query/components/QueryPanel.tsx (1)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate connection lookup on every render.
connections.find(...)with the identical predicate is called twice inline to build theprofileprop. This duplicates work and hurts readability; other files in this PR (QueryTargetBar.tsx,QueryStatusBar.tsx) compute the connection once and reuse it.♻️ Proposed refactor
const activeTab = tabs.find((t) => t.id === activeTabId); + const activeConnection = activeTab?.connectionId + ? connections.find((connection) => connection.id === activeTab.connectionId) + : undefined; + const activeConnectionProfile = activeConnection + ? resolveColorProfile(activeConnection.colorProfileId, customProfiles, activeConnection.color) + : null; const activeSql = activeTabId ? tabSql[activeTabId] ?? "" : "";- <QueryResultsTable result={activeResult} profile={activeTab.connectionId ? resolveColorProfile(connections.find((connection) => connection.id === activeTab.connectionId)?.colorProfileId, customProfiles, connections.find((connection) => connection.id === activeTab.connectionId)?.color) : null} /> + <QueryResultsTable result={activeResult} profile={activeConnectionProfile} />Also applies to: 202-202
🤖 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/query/components/QueryPanel.tsx` around lines 34 - 35, In QueryPanel, compute the matching connection from connections once before constructing the profile prop, store it in a local variable, and reuse that variable for both references instead of calling connections.find(...) twice. Follow the existing pattern used in QueryTargetBar.tsx and QueryStatusBar.tsx.
🤖 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 `@scripts/test-sidecar-contract.mjs`:
- Around line 20-25: Update the test process environment in the sidecar contract
test to isolate Windows application-data storage by setting APPDATA and
LOCALAPPDATA to dataHome, alongside the existing HOME and USERPROFILE overrides.
Verify the store’s folder-resolution code uses those variables or the
corresponding SpecialFolder APIs, and preserve the temporary-directory isolation
for all persistence assertions.
In `@sidecar/src/Ssmsx.Core/Storage/ConnectionStore.cs`:
- Around line 307-329: Update ConnectionInfo to track whether the name/alias was
explicitly supplied, mirroring HasExplicitColorProfileId with a HasExplicitName
backing field. In MergeSameId, use HasExplicitName rather than inferring
omission from incoming.Name or color presence, preserving the existing alias
when the name is omitted while allowing an explicit clear to remove it. Ensure
normalization and deserialization preserve this distinction.
In `@sidecar/src/Ssmsx.Sidecar/Program.cs`:
- Around line 131-144: Move the connection.delete handler from handlers into
cancellableHandlers, preserving its existing connectionOperations.RunAsync
serialization and deletion logic. Ensure the main processing loop does not await
this potentially blocked operation inline, while retaining cancellation support
and the current response behavior.
In `@sidecar/tests/Ssmsx.Core.Tests/Storage/ConnectionStoreTests.cs`:
- Around line 532-561: The test
ListAsync_ReturnsNormalizedData_WhenMigrationRewriteFails should skip when
running as root, since chmod cannot reliably deny writes for uid 0. Add a Unix
root check alongside the existing OperatingSystem.IsWindows guard, using the
platform’s effective-user-ID mechanism, while preserving the current test
behavior for non-root Unix environments.
In `@src/features/connection/components/ConnectionList.tsx`:
- Around line 191-267: Move the onContextMenu handler from the inner select
button to the row container div keyed by c.id, so right-clicking the auth badge
or delete-action area invokes handleContextMenu consistently across the entire
connection row.
In `@src/features/connection/components/PropertiesTab.tsx`:
- Around line 52-71: Update the password input change handler in PropertiesTab
to mark the form dirty whenever the user edits the password, alongside updating
the password value. Reuse the existing setFormDirty mechanism used by update and
setAppearanceDraft so password-only edits prevent the selection refresh effect
from clearing the field.
In `@src/features/query/components/QueryResultsTable.tsx`:
- Around line 147-157: Update QueryResultsTable’s columnSizingKey to include the
active tab identity alongside the result-set index and column signature,
ensuring sizing state cannot collide between tabs. Update QueryPanel to pass its
existing activeTabId to QueryResultsTable via the tabId prop, and use that prop
when constructing the key while preserving the existing sizing read/write
behavior.
In `@src/features/query/components/QueryTabBar.tsx`:
- Around line 134-145: Update the close button in the QueryTabBar tab-rendering
block to include an aria-label that identifies the action and tab, such as
“Close [tab title]”, while preserving the existing title and click behavior.
In `@src/features/query/utils/queryTabs.ts`:
- Around line 15-47: Update normalizeRestoredQueryTab to require a valid
diagramViewId whenever kind is "diagram"; return null for diagram tabs where it
is missing or not a string, while preserving optional diagramViewId handling for
non-diagram tabs.
In `@src/features/settings/store/settingsStore.ts`:
- Around line 102-128: Update saveColorProfiles to reject duplicate profile IDs
before persisting normalized profiles, using an ID-tracking set or equivalent
alongside validateCustomColorProfile. Return the existing validation error type
when a duplicate is detected, and preserve the current normalization and save
flow for unique IDs.
---
Nitpick comments:
In `@scripts/test-sidecar-contract.mjs`:
- Around line 76-90: Update the send function to handle errors from
child.stdin.write, especially EPIPE when the sidecar has exited, by rejecting
the existing pending request and clearing its timeout instead of allowing an
unhandled stream error to crash the script. Preserve the current timeout
rejection behavior and request tracking.
In `@sidecar/src/Ssmsx.Protocol/Models/ConnectionInfo.cs`:
- Around line 42-50: Expose a public const string DefaultColorProfileId = "red"
on ConnectionInfo, then update ConnectionStore.DefaultColorProfileId to
reference ConnectionInfo.DefaultColorProfileId instead of duplicating the
literal. Use the shared constant in the ColorProfileId getter as well.
In `@src-tauri/Cargo.toml`:
- Around line 28-29: Update the target dependency declaration for
tauri-plugin-single-instance to cover all desktop targets supported by the
#[cfg(desktop)] registration in src-tauri/src/lib.rs, including BSD platforms,
or narrow that registration to macOS, Windows, and Linux. Keep the dependency
gate and registration conditions consistent so BSD builds do not reference an
unavailable dependency.
In `@src/features/connection/components/ConnectionList.tsx`:
- Line 205: Update the selected connection row’s accessibility state in the
component rendering aria-pressed to use aria-current="true" when isSelected and
omit or unset it otherwise, preserving the existing selection and loading
behavior.
In `@src/features/query/components/QueryPanel.tsx`:
- Around line 34-35: In QueryPanel, compute the matching connection from
connections once before constructing the profile prop, store it in a local
variable, and reuse that variable for both references instead of calling
connections.find(...) twice. Follow the existing pattern used in
QueryTargetBar.tsx and QueryStatusBar.tsx.
In `@src/features/settings/components/SettingsDialog.tsx`:
- Around line 13-22: Extract the shared connection/profile search terms into a
single exported constant, such as in connectionAppearance.ts, and remove the
duplicate literal from SettingsDialog.tsx. Update both SettingsDialog.tsx and
ConnectionProfilesPanel.tsx, including isGeneralProfileSearch, to import and
reuse that constant so panel visibility and profile matching remain
synchronized.
In `@tests/frontend/connectionAppearanceDom.test.tsx`:
- Around line 97-110: Restore the original HTMLDialogElement.prototype.showModal
and close implementations after the tests complete. Capture each original
descriptor before the overrides in beforeAll, then use afterAll to restore those
descriptors so the global prototype patches from the dialog test do not leak
into other tests.
🪄 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: 8abe13f6-6099-4d69-be39-5e2d337d54fa
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (72)
.claude/skills/ssmsx-design/SKILL.md.claude/skills/ssmsx-design/components/data/ConnectionItem.d.ts.claude/skills/ssmsx-design/components/data/ConnectionItem.jsx.claude/skills/ssmsx-design/components/data/data.card.html.claude/skills/ssmsx-design/components/explorer/TreeRow.d.ts.claude/skills/ssmsx-design/components/explorer/TreeRow.jsx.claude/skills/ssmsx-design/components/explorer/explorer.card.html.claude/skills/ssmsx-design/components/navigation/QueryTab.d.ts.claude/skills/ssmsx-design/components/navigation/QueryTab.jsx.claude/skills/ssmsx-design/components/navigation/navigation.card.html.claude/skills/ssmsx-design/guidelines/colors-connection.card.html.claude/skills/ssmsx-design/readme.md.claude/skills/ssmsx-design/tokens/colors.css.claude/skills/ssmsx-design/tokens/radius.css.claude/skills/ssmsx-design/ui_kits/ssmsx-desktop/ConnectDialog.jsx.claude/skills/ssmsx-design/ui_kits/ssmsx-desktop/Workbench.jsx.claude/skills/ssmsx-design/ui_kits/ssmsx-desktop/data.js.github/workflows/ci.ymlpackage.jsonscripts/smoke-tauri-mcp.mjsscripts/test-sidecar-contract.mjssidecar/src/Ssmsx.Core/Connections/ConnectionManager.cssidecar/src/Ssmsx.Core/Connections/ConnectionOperationCoordinator.cssidecar/src/Ssmsx.Core/Connections/SqlConnectionFactory.cssidecar/src/Ssmsx.Core/Storage/ConnectionStore.cssidecar/src/Ssmsx.Protocol/Messages/ConnectionMessages.cssidecar/src/Ssmsx.Protocol/Models/ConnectionInfo.cssidecar/src/Ssmsx.Protocol/ProtocolJsonContext.cssidecar/src/Ssmsx.Sidecar/Program.cssidecar/tests/Ssmsx.Core.Tests/Connections/ConnectionManagerTests.cssidecar/tests/Ssmsx.Core.Tests/Connections/ConnectionOperationCoordinatorTests.cssidecar/tests/Ssmsx.Core.Tests/Storage/ConnectionStoreTests.cssidecar/tests/Ssmsx.Protocol.Tests/JsonRpcTests.cssidecar/tests/Ssmsx.Protocol.Tests/Models/ConnectionInfoTests.cssrc-tauri/Cargo.tomlsrc-tauri/src/commands/connection.rssrc-tauri/src/lib.rssrc/app/App.tsxsrc/features/connection/api/connectionApi.tssrc/features/connection/components/AppearanceFields.tsxsrc/features/connection/components/ConnectionAuthProgressDialog.tsxsrc/features/connection/components/ConnectionDialog.tsxsrc/features/connection/components/ConnectionList.tsxsrc/features/connection/components/ConnectionStringTab.tsxsrc/features/connection/components/CustomTab.tsxsrc/features/connection/components/PropertiesTab.tsxsrc/features/connection/store/connectionStore.tssrc/features/connection/types.tssrc/features/explorer/components/TreeNode.tsxsrc/features/explorer/store/explorerStore.tssrc/features/query/components/QueryPanel.tsxsrc/features/query/components/QueryResultsTable.tsxsrc/features/query/components/QueryStatusBar.tsxsrc/features/query/components/QueryTabBar.tsxsrc/features/query/components/QueryTargetBar.tsxsrc/features/query/store/queryStore.tssrc/features/query/types.tssrc/features/query/utils/queryTabs.tssrc/features/query/utils/resultColumnSizing.tssrc/features/settings/components/ConnectionProfilesPanel.tsxsrc/features/settings/components/SettingsDialog.tsxsrc/features/settings/settingsSchema.tssrc/features/settings/store/settingsStore.tssrc/features/settings/types.tssrc/shared/components/ColorProfileCombobox.tsxsrc/shared/components/ColorProfileMarker.tsxsrc/shared/components/ConfirmDialog.tsxsrc/shared/connectionAppearance.tstests/frontend/connectionAppearance.test.tstests/frontend/connectionAppearanceDom.test.tsxtests/frontend/queryTabs.test.tstests/frontend/queryUi.test.tsx
💤 Files with no reviewable changes (2)
- src/features/connection/components/ConnectionDialog.tsx
- src/features/connection/components/CustomTab.tsx
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 69 out of 74 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
package.json:40
jsdomis currently specified as a caret range, which can introduce non-reproducible installs. Pinning to an exact version would match the rest of the devDependencies in this repo.
"jsdom": "^29.1.1",
package.json:43
vitestis specified with a caret range; pinning it to an exact version would keep dependency management consistent with the rest of package.json and make CI installs deterministic.
"vitest": "^4.1.10"
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3378c3d8b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Jul 27, 2026
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
npm run test:frontend(30 tests)npm run buildnpm audit --audit-level=moderatedotnet test sidecar/Ssmsx.Sidecar.slnx --filter "Category!=Integration"(84 tests)node scripts/test-sidecar-contract.mjscargo fmt --manifest-path src-tauri/Cargo.toml --all -- --checkcargo check --manifest-path src-tauri/Cargo.toml --all-targetscargo test --manifest-path src-tauri/Cargo.tomlcargo clippy --manifest-path src-tauri/Cargo.toml --all-targets -- -D warningsThe UI behavior is covered by DOM tests. I did not open the native app during this pass because that would take keyboard and mouse focus.
Summary by CodeRabbit
New Features
Bug Fixes
Tests