feat: pin a default connection; fix Editor misdiagnosing malformed SQL as a permissions error

CLAUDE.md

@@ -499,6 +499,51 @@ so "View as" resolves membership as the target user).
   "authentication is not authorization" trap `BrainController` documents — there is still
   no filter doing it for you.
 
+### Default connection (pinning)
+
+A user can pin one connection as their default, from the pin column in **Manage
+Connections** or the pin toggle in the sidebar connection switcher.
+
+- **The pin is per user, not per connection.** `connection_pin` keys on username with a
+  unique constraint (`V120__create_connection_pin.sql`; applied by `ddl-auto` from the
+  `ConnectionPin` entity — verified on a scratch database, the table and its unique index
+  are created on boot). A column on `database_connection` would have been wrong twice
+  over: a connection shared through `connection_access_grant` would let one user's choice
+  decide what everyone else opens on, and a shared connection is `canManageConfig=false`
+  for its recipients, so exactly the people who most want a default could not set one.
+- **One pin per user is the point.** `ConnectionPinService.pin` moves the existing row
+  rather than inserting a second; the unique constraint is the backstop, and a losing
+  concurrent insert re-reads and updates instead of surfacing a 500.
+- **`PUT|DELETE /connections/{id}/pin` are gated on `assertCanUseConnection`**, not
+  `assertCanManageConnectionConfig` — choosing where you land is a preference, not a
+  change to the connection. Verified live: a DEVELOPER holding only a grant on a
+  connection (`canManageConfig: false`) pins it and gets 200, while the same user pinning
+  a connection they hold no grant on gets **403, not 500** — the `ResponseStatusException`
+  rethrow before the catch-all is doing its job.
+- **Unpin is scoped to the connection named.** A stale click in a background tab must not
+  clear a pin the user has since moved elsewhere.
+- **`GET /connections` carries `pinned` per caller**, so no surface needs a second
+  request, and two users listing the same shared connection see different values —
+  confirmed live. `deleteConnection` clears every pin on the connection alongside its
+  grants.
+- **`ConnectionScopedAuthorizationSafetyTest` flags `GET /connections` now**, because the
+  handler resolves the caller's *pinned* connection id and the scanner matches
+  `(?i)connection_?id` anywhere in a handler body. That endpoint takes no arguments at all
+  — it returns whatever `getConnectionsForUser(username, isAdmin)` gives — so it is in
+  `AUTHORIZED_ELSEWHERE`, and `connectionListingTakesNoCallerSuppliedId` re-derives that
+  claim so the exemption cannot rot into cover for a real gap. Do not resolve this by
+  adding a meaningless assert, and do not weaken the scanner.
+- **The pin must beat an already-selected connection, not just an empty one.**
+  `useDashboardStore` persists `connectionId`, so after a reload something is always
+  selected — the original auto-select ran only when nothing was. `useConnectionManager`
+  therefore applies the pin once per page load (`pinAppliedThisLoad`, module scope, reset
+  by `resetConnectionPinApplied()` in the auth reset). Module scope and not a ref: the
+  hook is called from a dozen sections, and a per-instance guard would let a
+  later-mounted section yank the user back to the pin after they deliberately switched.
+  Switching mid-session still sticks; the pin re-applies on the next load.
+- Pinned connections sort first in `useConnectionManager`, so every consumer — the sidebar
+  switcher included — shows the default at the top.
+
 ### Admin profile switch
 Admins can **View as** a sub-user from the top-right of the home layout (`ProfileSwitch`) to verify connection ACLs, chat/editor policies, and role-gated nav.
 
@@ -680,6 +725,25 @@ it against a real database — not a theoretical hardening pass.
   it killed **every** active query on the connection, including other users' work.
   The cancel endpoint is scoped to the connection *and* the user who started the
   run, so an execution id is not a kill primitive for someone else's query.
+- **A statement DeepSQL cannot parse is a syntax error, not DDL/DML — say so.** A user
+  pasted a SELECT still carrying the double quotes it had in source code
+  (`"select h.id, ...`) and got **"Only admins can execute DDL or DML from the SQL
+  Editor"**, which reads as a permissions problem and sends people hunting for a role fix.
+  Two keyword heuristics disagreed and the code resolved the disagreement as "mutation":
+  `QueryNormalizer.detectQueryType` sanitizes a prefix away and answered `SELECT`, while
+  the provider's `isReadOnlyQuery` strips only *comments*, still saw the leading `"`, and
+  answered false — so `mutating = !readOnly && type != UNKNOWN` labelled a SELECT a
+  mutation. `classifyStatement` now records that the parser rejected the statement and,
+  when the detected verb is read-only and no hidden write was found, returns
+  `notParseable`; `enforce` throws `STATEMENT_NOT_PARSEABLE` ahead of both the read-only
+  and confirmation branches. **The statement is still blocked, for admins too** — only the
+  diagnosis changed, and an admin is deliberately *not* offered a confirmation prompt for
+  something nothing managed to classify. The reclassification is gated on
+  `isReadOnlyVerb(queryType)` and `hiddenWrite == null`, which is what keeps it from
+  becoming a bypass: an unparseable `DELETE`, and a malformed data-modifying CTE, both keep
+  their mutation handling (covered by `anUnparseableWriteIsStillTreatedAsAMutation` and
+  `aMalformedDataModifyingCteIsStillBlockedAsAWrite`). The MCP guard already reported this
+  case honestly ("Only read-only SQL is allowed …") and was left alone.
 - **Keep the client timeout under the proxy's.** `docker/nginx/default.conf` gives
   up at `proxy_read_timeout 300s`; the Editor used to ask for 600s, so a 6-minute
   query returned an opaque 504 while still running. `QUERY_TIMEOUT_SECONDS = 240`

backend/src/main/java/com/dbaagent/controller/ConnectionController.java

@@ -9,6 +9,7 @@
 import com.dbaagent.repository.ConnectionInitHistoryRepository;
 import com.dbaagent.repository.ConnectionInitStatusRepository;
 import com.dbaagent.repository.SchemaDocumentationRepository;
+import com.dbaagent.service.ConnectionPinService;
 import com.dbaagent.service.ConnectionService;
 import com.dbaagent.service.scheduler.BrainInitSchedulerService;
 import com.dbaagent.service.scheduler.BrainJobsService;
@@ -45,6 +46,7 @@ public class ConnectionController {
     private final AccessControlService accessControlService;
     private final ConnectionAccessService connectionAccessService;
     private final com.dbaagent.repository.ConnectionAccessGrantRepository connectionAccessGrantRepository;
+    private final ConnectionPinService connectionPinService;
 
     @PostMapping("/test")
     public ResponseEntity<Map<String, Object>> testConnection(@RequestBody ConnectionRequest request) {
@@ -429,8 +431,10 @@ public ResponseEntity<List<ConnectionSummaryResponse>> getAllConnections() {
             String username = accessControlService.getCurrentUsername();
             boolean isAdmin = accessControlService.isCurrentUserAdmin();
             List<DatabaseConnection> connections = credentialService.getConnectionsForUser(username, isAdmin);
+            // One lookup for the whole list rather than one per row.
+            String pinnedId = connectionPinService.pinnedConnectionId(username).orElse(null);
             List<ConnectionSummaryResponse> decryptedConnections = connections.stream()
-                .map(conn -> toSummary(conn, username, isAdmin))
+                .map(conn -> toSummary(conn, username, isAdmin, pinnedId))
                 .toList();
             return ResponseEntity.ok(decryptedConnections);
         } catch (org.springframework.web.server.ResponseStatusException e) {
@@ -440,13 +444,62 @@ public ResponseEntity<List<ConnectionSummaryResponse>> getAllConnections() {
         }
     }
 
+    /**
+     * Pin this connection as the caller's default, replacing any connection they had
+     * pinned before.
+     *
+     * <p>Gated on {@code assertCanUseConnection} rather than
+     * {@code assertCanManageConnectionConfig}: choosing which database you land on is a
+     * personal preference, not a change to the connection, and a shared connection is
+     * config-read-only for its recipients. Requiring manage rights would mean the people
+     * who most want a default — the ones who were granted exactly one connection —
+     * could not set one.
+     */
+    @PutMapping("/{id}/pin")
+    public ResponseEntity<Map<String, Object>> pinConnection(@PathVariable String id) {
+        Map<String, Object> response = new HashMap<>();
+        try {
+            accessControlService.assertCanUseConnection(id);
+            connectionPinService.pin(accessControlService.requireCurrentUsername(), id);
+            response.put("success", true);
+            response.put("pinned", true);
+            return ResponseEntity.ok(response);
+        } catch (org.springframework.web.server.ResponseStatusException e) {
+            throw e;
+        } catch (Exception e) {
+            response.put("success", false);
+            response.put("message", "Failed to pin connection: " + e.getMessage());
+            return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response);
+        }
+    }
+
+    /** Clear the caller's default, if this connection is the one currently pinned. */
+    @DeleteMapping("/{id}/pin")
+    public ResponseEntity<Map<String, Object>> unpinConnection(@PathVariable String id) {
+        Map<String, Object> response = new HashMap<>();
+        try {
+            accessControlService.assertCanUseConnection(id);
+            connectionPinService.unpin(accessControlService.requireCurrentUsername(), id);
+            response.put("success", true);
+            response.put("pinned", false);
+            return ResponseEntity.ok(response);
+        } catch (org.springframework.web.server.ResponseStatusException e) {
+            throw e;
+        } catch (Exception e) {
+            response.put("success", false);
+            response.put("message", "Failed to unpin connection: " + e.getMessage());
+            return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response);
+        }
+    }
+
     @DeleteMapping("/{id}")
     public ResponseEntity<Map<String, Object>> deleteConnection(@PathVariable String id) {
         Map<String, Object> response = new HashMap<>();
         try {
             accessControlService.assertCanManageConnectionConfig(id);
             connectionService.closeConnectionPool(id);
             connectionAccessService.deleteAllGrantsForConnection(id);
+            connectionPinService.clearPinsForConnection(id);
             credentialService.deleteConnection(id);
             response.put("success", true);
             response.put("message", "Connection deleted successfully");
@@ -781,7 +834,7 @@ public ResponseEntity<?> runBrainJob(@PathVariable String id, @PathVariable Stri
         }
     }
 
-    private ConnectionSummaryResponse toSummary(DatabaseConnection conn, String username, boolean isAdmin) {
+    private ConnectionSummaryResponse toSummary(DatabaseConnection conn, String username, boolean isAdmin, String pinnedConnectionId) {
         ConnectionSummaryResponse summary = new ConnectionSummaryResponse();
         try {
             ConnectionRequest decrypted = credentialService.getDecryptedConnection(conn.getId());
@@ -821,6 +874,7 @@ private ConnectionSummaryResponse toSummary(DatabaseConnection conn, String user
         summary.setAccessLevel(resolved.getEffectiveAccess().name());
         summary.setCanManageConfig(resolved.canManageConfig());
         summary.setCanManageContent(resolved.canManageContent());
+        summary.setPinned(conn.getId() != null && conn.getId().equals(pinnedConnectionId));
         return summary;
     }
 

backend/src/main/java/com/dbaagent/dto/ConnectionSummaryResponse.java

@@ -31,4 +31,13 @@ public class ConnectionSummaryResponse {
     private String accessLevel;
     private Boolean canManageConfig;
     private Boolean canManageContent;
+
+    /**
+     * Whether the calling user has pinned this connection as their default.
+     *
+     * <p>Per caller, not per connection — two users listing the same shared connection
+     * see different values here. It rides the list response so the UI needs no second
+     * request to know which row carries the pin.
+     */
+    private Boolean pinned;
 }

backend/src/main/java/com/dbaagent/model/ConnectionPin.java

@@ -0,0 +1,70 @@
+package com.dbaagent.model;
+
+import jakarta.persistence.*;
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+import java.time.LocalDateTime;
+
+/**
+ * One user's default database connection.
+ *
+ * <p>The pin is deliberately <em>per user</em> rather than a flag on
+ * {@link DatabaseConnection}. A connection can be shared with several people through
+ * {@code connection_access_grant}, and a default is a personal preference — one user's
+ * choice must not decide what everyone else opens on. A column on the connection row
+ * would also put the setting out of reach of exactly the people who need it: a shared
+ * connection has {@code canManageConfig == false} for its recipients, so they could
+ * never pin the connection they use every day.
+ *
+ * <p>At most one row per user — the unique constraint on {@code username} is what makes
+ * "always the default" true rather than merely intended. Pinning a second connection
+ * moves the pin instead of creating a second one; see
+ * {@code ConnectionPinService.pin}.
+ *
+ * <p>{@code connection_id} carries no foreign key, matching
+ * {@link ConnectionAccessGrant}. {@code ConnectionController.deleteConnection} clears
+ * pins alongside grants; a pin that outlives its connection is inert anyway, since the
+ * flag is only ever computed for connections the caller can already see.
+ */
+@Entity
+@Table(
+    name = "connection_pin",
+    uniqueConstraints = @UniqueConstraint(
+        name = "ux_connection_pin_username",
+        columnNames = {"username"}
+    )
+)
+@Data
+@NoArgsConstructor
+@AllArgsConstructor
+public class ConnectionPin {
+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private Long id;
+
+    @Column(nullable = false)
+    private String username;
+
+    @Column(name = "connection_id", nullable = false, length = 36)
+    private String connectionId;
+
+    @Column(name = "created_at", nullable = false)
+    private LocalDateTime createdAt;
+
+    @Column(name = "updated_at", nullable = false)
+    private LocalDateTime updatedAt;
+
+    @PrePersist
+    void onCreate() {
+        LocalDateTime now = LocalDateTime.now();
+        createdAt = now;
+        updatedAt = now;
+    }
+
+    @PreUpdate
+    void onUpdate() {
+        updatedAt = LocalDateTime.now();
+    }
+}

backend/src/main/java/com/dbaagent/repository/ConnectionPinRepository.java

@@ -0,0 +1,27 @@
+package com.dbaagent.repository;
+
+import com.dbaagent.model.ConnectionPin;
+import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.Optional;
+
+public interface ConnectionPinRepository extends JpaRepository<ConnectionPin, Long> {
+
+    /**
+     * Case-insensitive, matching how {@code ConnectionAccessGrantRepository} resolves
+     * usernames — a login that differs only in casing must not end up with a second pin
+     * the unique constraint cannot see.
+     */
+    @Query("select p from ConnectionPin p where lower(p.username) = lower(?1)")
+    Optional<ConnectionPin> findByUsernameIgnoreCase(String username);
+
+    /**
+     * Derived deletes need their own transaction. Annotating a self-invoked caller does
+     * nothing — Spring proxies are bypassed by {@code this::} — which is the same trap
+     * {@code McpTokenRepository.deleteByUserId} documents.
+     */
+    @Transactional
+    void deleteByConnectionId(String connectionId);
+}

backend/src/main/java/com/dbaagent/service/ConnectionPinService.java

@@ -0,0 +1,96 @@
+package com.dbaagent.service;
+
+import com.dbaagent.model.ConnectionPin;
+import com.dbaagent.repository.ConnectionPinRepository;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.dao.DataIntegrityViolationException;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.Optional;
+
+/**
+ * The per-user default connection.
+ *
+ * <p>Callers are responsible for authorizing the connection first — this service takes an
+ * already-checked id. {@code ConnectionController} calls
+ * {@code assertCanReadConnectionContent} before every pin write, so a pin cannot be used
+ * to assert an interest in a connection the caller cannot see.
+ */
+@Service
+@RequiredArgsConstructor
+@Slf4j
+public class ConnectionPinService {
+
+    private final ConnectionPinRepository pinRepository;
+
+    /** The connection this user opens by default, if they have chosen one. */
+    public Optional<String> pinnedConnectionId(String username) {
+        if (username == null || username.isBlank()) {
+            return Optional.empty();
+        }
+        return pinRepository.findByUsernameIgnoreCase(username).map(ConnectionPin::getConnectionId);
+    }
+
+    /**
+     * Make {@code connectionId} this user's default, replacing any previous pin.
+     *
+     * <p>Moving the existing row rather than inserting a second one is what keeps "the
+     * default" singular; the unique constraint on {@code username} is the backstop. Two
+     * pins racing in from different tabs can still collide on that constraint, so the
+     * loser re-reads and updates instead of surfacing a 500 for what is really a
+     * last-write-wins preference.
+     */
+    @Transactional
+    public void pin(String username, String connectionId) {
+        if (username == null || username.isBlank() || connectionId == null || connectionId.isBlank()) {
+            return;
+        }
+        Optional<ConnectionPin> existing = pinRepository.findByUsernameIgnoreCase(username);
+        if (existing.isPresent()) {
+            ConnectionPin pin = existing.get();
+            if (connectionId.equals(pin.getConnectionId())) {
+                return;
+            }
+            pin.setConnectionId(connectionId);
+            pinRepository.save(pin);
+            return;
+        }
+
+        ConnectionPin pin = new ConnectionPin();
+        pin.setUsername(username);
+        pin.setConnectionId(connectionId);
+        try {
+            pinRepository.save(pin);
+        } catch (DataIntegrityViolationException e) {
+            pinRepository.findByUsernameIgnoreCase(username).ifPresent(concurrent -> {
+                concurrent.setConnectionId(connectionId);
+                pinRepository.save(concurrent);
+            });
+        }
+    }
+
+    /**
+     * Clear this user's default, but only when it is still the connection they asked to
+     * unpin. A stale click in another tab must not silently drop a pin the user has since
+     * moved somewhere else.
+     */
+    @Transactional
+    public void unpin(String username, String connectionId) {
+        if (username == null || username.isBlank()) {
+            return;
+        }
+        pinRepository.findByUsernameIgnoreCase(username)
+            .filter(pin -> connectionId == null || connectionId.equals(pin.getConnectionId()))
+            .ifPresent(pinRepository::delete);
+    }
+
+    /** Drop every user's pin on a connection that is being deleted. */
+    public void clearPinsForConnection(String connectionId) {
+        if (connectionId == null || connectionId.isBlank()) {
+            return;
+        }
+        pinRepository.deleteByConnectionId(connectionId);
+    }
+}