Merge branch 'main' into cursor/cloud-env-caveats-231a

Author: geekypunk

.github/workflows/codeql.yml

@@ -18,13 +18,19 @@ jobs:
   analyze:
     name: analyze (${{ matrix.language }})
     runs-on: ubuntu-latest
-    # Code scanning is free on public repositories; on a private one it requires
-    # GitHub Advanced Security, which this organisation's free plan does not
-    # include. Without this guard every run fails with a 403 until the repository
-    # is public, and a permanently red workflow trains people to stop reading CI.
-    # DELETE THIS LINE once the repository is public — from then on a failure
-    # here is a real finding rather than a billing state.
-    if: github.event.repository.visibility == 'public'
+    # No visibility guard here, deliberately. A job-level `if:` is evaluated
+    # before the matrix expands, so a skipped run reports a single check under
+    # the raw template name — `analyze (${{ matrix.language }})` — rather than
+    # the two expanded names. That phantom name reached the `Protect main branch`
+    # ruleset as a required check (GitHub's suggestion list offers whatever was
+    # last reported), and once this repository went public and the job began
+    # reporting the real names instead, nothing was left to satisfy the phantom:
+    # every pull request stalled on "Expected — waiting for status to be
+    # reported", unmergeable, with no failing job to point at.
+    #
+    # Code scanning is free while this repository is public. If it ever goes
+    # private without GitHub Advanced Security, drop these contexts from the
+    # ruleset — do not reintroduce an `if:` here.
     permissions:
       # Required to upload results to the code-scanning API.
       security-events: write

CLAUDE.md

@@ -43,7 +43,23 @@ npm run build     # Build (dev)
 npm run build:production  # Build (prod)
 ```
 
-**Dev credentials**: admin/admin (auth bypass in dev mode)
+**Dev credentials**: There is no baked-in admin/admin login — `AuthController.login` requires a
+real `User` row matched by **email**, not username, so a fresh database (new Postgres volume)
+has no account to log in with at all. `SECURITY_AUTH_ENABLED=false` only bypasses JWT/MCP token
+*validation* (`JwtAuthenticationFilter`, `McpTokenAuthenticationFilter`); it does not create a
+user or skip the login form. Create the first admin via the bootstrap endpoint, gated by
+`SECURITY_ADMIN_BOOTSTRAP_ENABLED=true` + `ADMIN_BOOTSTRAP_SECRET`, and only callable from
+localhost:
+
+```bash
+curl -X POST http://localhost:8080/api/users/admin/bootstrap \
+  -H "Content-Type: application/json" \
+  -H "X-Admin-Bootstrap-Secret: $ADMIN_BOOTSTRAP_SECRET" \
+  -d '{"email":"admin@localhost","password":"<your-password>"}'
+```
+
+Then log in with that **email** (not `admin`) and password. `POST /users/admin/reset` (same
+header) replaces the existing admin if you need to rotate the password.
 
 ### Database
 

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

@@ -1,9 +1,12 @@
 package com.dbaagent.controller;
 
+import com.dbaagent.model.SavedDashboard;
 import com.dbaagent.service.DashboardAgentService;
+import com.dbaagent.service.SavedDashboardService;
 import com.dbaagent.service.security.AccessControlService;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import org.springframework.dao.OptimisticLockingFailureException;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
@@ -18,6 +21,7 @@
 
 import java.io.IOException;
 import java.util.Map;
+import java.util.UUID;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -44,6 +48,7 @@ public class DashboardGenerationController {
 
     private final DashboardAgentService dashboardAgentService;
     private final AccessControlService accessControlService;
+    private final SavedDashboardService savedDashboardService;
 
     @PostMapping("/generate")
     public ResponseEntity<?> generate(@RequestBody GenerateRequest request) {
@@ -68,13 +73,47 @@ public ResponseEntity<?> generate(@RequestBody GenerateRequest request) {
     @PostMapping(value = "/generate/stream", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
     public SseEmitter generateStream(@RequestBody GenerateRequest request) {
         requireValid(request);
-        accessControlService.assertCanReadConnectionContent(request.connectionId());
+        // This path creates/updates a SavedDashboard row (beginGenerationTurn etc.)
+        // on every call, not just reads — a VIEWER (read-only) must not be able to
+        // mint or mutate drafts via chat.
+        accessControlService.assertCanManageConnectionContent(request.connectionId());
+        SseEmitter emitter = new SseEmitter(600_000L);
+
+        // Resolve (or create) the target dashboard and record the user's message
+        // SYNCHRONOUSLY, before any slow agent work starts — this is what lets a
+        // reload mid-generation see "still working" (generationStatus=RUNNING)
+        // instead of nothing at all, even for a brand-new, never-saved dashboard.
+        // See SavedDashboardService's "Server-owned chat-turn persistence" section.
+        final SavedDashboard dashboard;
+        try {
+            dashboard = savedDashboardService.beginGenerationTurn(
+                request.dashboardId(), request.connectionId(), request.prompt());
+        } catch (IllegalArgumentException | IllegalStateException e) {
+            sendErrorAndComplete(emitter, e.getMessage());
+            return emitter;
+        } catch (OptimisticLockingFailureException e) {
+            // Lost the race to another concurrent submit on the same dashboard —
+            // same user-facing shape as the "already running" case above.
+            sendErrorAndComplete(emitter, "A generation is already running for this dashboard.");
+            return emitter;
+        }
+        // The frontend needs this id right away (not just at the end) so a
+        // brand-new dashboard is addressable — e.g. by a reload — well before
+        // the potentially multi-minute build finishes.
+        try {
+            emitter.send(SseEmitter.event().name("created")
+                .data(Map.of("dashboardId", dashboard.getId().toString())));
+        } catch (IOException ignore) {
+            // Client already gone before we even started streaming — fine, the
+            // turn is already durably recorded; the work below still runs and
+            // persists its result regardless of this connection.
+        }
+
         // Coding a whole dashboard (ground + verify every query + write the HTML) can
         // run for minutes. Give it real headroom (10 min) and keep the stream alive
         // with a heartbeat — otherwise it emits only 3 step events and the long idle
         // gap gets cut by nginx/emitter timeouts before `done`, surfacing to the user
         // as "Generation ended unexpectedly".
-        SseEmitter emitter = new SseEmitter(600_000L);
         ScheduledExecutorService heartbeat = Executors.newSingleThreadScheduledExecutor(r -> {
             Thread t = new Thread(r, "dashboard-generate-hb");
             t.setDaemon(true);
@@ -106,21 +145,47 @@ public SseEmitter generateStream(@RequestBody GenerateRequest request) {
                 // `done` event with a real artifact — the FE's done handler always
                 // appends "Done — built…" and auto-saves. A dedicated `chat` event
                 // keeps that path from swallowing out-of-context messages.
-                if (Boolean.TRUE.equals(config.get("chat"))) {
-                    emitter.send(SseEmitter.event().name("chat")
-                        .data(Map.of(
-                            "success", true,
-                            "reply", String.valueOf(config.getOrDefault("reply", "")),
-                            "dashboardConfig", config)));
-                } else {
-                    emitter.send(SseEmitter.event().name("done")
-                        .data(Map.of("success", true, "dashboardConfig", config)));
+                boolean chatOnly = Boolean.TRUE.equals(config.get("chat"));
+                // Persist BEFORE attempting to notify the client — a client that's
+                // gone by now must never turn an already-successful result into a
+                // recorded failure (see the catch block below, which only ever
+                // handles a real dashboardAgentService.generate() failure, not a
+                // dead SSE connection at delivery time).
+                try {
+                    if (chatOnly) {
+                        savedDashboardService.appendAgentReply(
+                            dashboard.getId(), String.valueOf(config.getOrDefault("reply", "")));
+                    } else {
+                        savedDashboardService.completeBuildTurn(dashboard.getId(), config);
+                    }
+                } catch (Exception persistErr) {
+                    log.error("Failed to persist completed dashboard turn {}", dashboard.getId(), persistErr);
+                }
+                try {
+                    if (chatOnly) {
+                        emitter.send(SseEmitter.event().name("chat")
+                            .data(Map.of(
+                                "success", true,
+                                "reply", String.valueOf(config.getOrDefault("reply", "")),
+                                "dashboardConfig", config)));
+                    } else {
+                        emitter.send(SseEmitter.event().name("done")
+                            .data(Map.of("success", true, "dashboardConfig", config)));
+                    }
+                } catch (IOException ignore) {
+                    // Client gone by the time the result was ready — already
+                    // persisted above, so this is a no-op, not a failure.
                 }
                 emitter.complete();
             } catch (ClientGoneException gone) {
                 emitter.complete();
             } catch (Exception e) {
                 log.warn("Streamed dashboard generation failed: {}", e.getMessage());
+                try {
+                    savedDashboardService.appendErrorReply(dashboard.getId(), safe(e));
+                } catch (Exception persistErr) {
+                    log.error("Failed to persist dashboard generation error {}", dashboard.getId(), persistErr);
+                }
                 try {
                     emitter.send(SseEmitter.event().name("error")
                         .data(Map.of("success", false, "error", safe(e))));
@@ -134,6 +199,14 @@ public SseEmitter generateStream(@RequestBody GenerateRequest request) {
         return emitter;
     }
 
+    private static void sendErrorAndComplete(SseEmitter emitter, String message) {
+        try {
+            emitter.send(SseEmitter.event().name("error")
+                .data(Map.of("success", false, "error", message == null ? "Request failed" : message)));
+        } catch (IOException ignore) { }
+        emitter.complete();
+    }
+
     private static void requireValid(GenerateRequest request) {
         if (request == null || request.connectionId() == null
             || request.prompt() == null || request.prompt().isBlank()) {
@@ -149,5 +222,8 @@ private static final class ClientGoneException extends RuntimeException {
         ClientGoneException(Throwable cause) { super(cause); }
     }
 
-    public record GenerateRequest(String connectionId, String prompt, Object currentConfig) { }
+    // dashboardId is optional — omitting it (a brand-new, never-saved dashboard)
+    // always creates a new SavedDashboard row, matching the pre-existing default
+    // behavior for new dashboards.
+    public record GenerateRequest(String connectionId, String prompt, Object currentConfig, UUID dashboardId) { }
 }

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

@@ -5,6 +5,7 @@
 import com.dbaagent.service.security.AccessControlService;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.dao.OptimisticLockingFailureException;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.*;
@@ -25,6 +26,19 @@ public class SavedDashboardController {
     @Autowired
     private AccessControlService accessControlService;
 
+    // Every write method below is load-then-save on a row a background generation
+    // turn (SavedDashboardService.beginGenerationTurn etc.) may be writing at the
+    // same time. Without this helper, the loser's raw Hibernate message
+    // ("Unexpected row count... where id=? and version=?") leaked straight into
+    // the API response as a 500 instead of a clean, retryable conflict.
+    private static ResponseEntity<Map<String, Object>> conflict(OptimisticLockingFailureException e) {
+        log.warn("Dashboard update lost a concurrent-write race: {}", e.getMessage());
+        Map<String, Object> body = new HashMap<>();
+        body.put("success", false);
+        body.put("message", "This dashboard changed elsewhere just now — please retry.");
+        return ResponseEntity.status(HttpStatus.CONFLICT).body(body);
+    }
+
     /** Publish this dashboard to the web (opt-in, revocable public link). */
     @PostMapping("/{id}/share")
     public ResponseEntity<Map<String, Object>> enableShare(@PathVariable UUID id) {
@@ -39,6 +53,8 @@ public ResponseEntity<Map<String, Object>> enableShare(@PathVariable UUID id) {
             return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of("success", false, "message", e.getMessage()));
         } catch (org.springframework.web.server.ResponseStatusException e) {
             throw e;
+        } catch (OptimisticLockingFailureException e) {
+            return conflict(e);
         } catch (Exception e) {
             log.error("Error enabling dashboard share", e);
             return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
@@ -59,6 +75,8 @@ public ResponseEntity<Map<String, Object>> setSharePassword(@PathVariable UUID i
             return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of("success", false, "message", e.getMessage()));
         } catch (org.springframework.web.server.ResponseStatusException e) {
             throw e;
+        } catch (OptimisticLockingFailureException e) {
+            return conflict(e);
         } catch (Exception e) {
             log.error("Error setting dashboard share password", e);
             return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
@@ -79,6 +97,8 @@ public ResponseEntity<Map<String, Object>> disableShare(@PathVariable UUID id) {
             return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of("success", false, "message", e.getMessage()));
         } catch (org.springframework.web.server.ResponseStatusException e) {
             throw e;
+        } catch (OptimisticLockingFailureException e) {
+            return conflict(e);
         } catch (Exception e) {
             log.error("Error disabling dashboard share", e);
             return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
@@ -196,6 +216,8 @@ public ResponseEntity<Map<String, Object>> updateDashboard(@PathVariable UUID id
             return ResponseEntity.status(HttpStatus.NOT_FOUND).body(errorResponse);
         } catch (org.springframework.web.server.ResponseStatusException e) {
             throw e;
+        } catch (OptimisticLockingFailureException e) {
+            return conflict(e);
         } catch (Exception e) {
             log.error("Error updating saved dashboard", e);
             Map<String, Object> errorResponse = new HashMap<>();
@@ -255,6 +277,8 @@ public ResponseEntity<Map<String, Object>> toggleFavorite(@PathVariable UUID id)
             return ResponseEntity.status(HttpStatus.NOT_FOUND).body(errorResponse);
         } catch (org.springframework.web.server.ResponseStatusException e) {
             throw e;
+        } catch (OptimisticLockingFailureException e) {
+            return conflict(e);
         } catch (Exception e) {
             log.error("Error toggling favorite", e);
             Map<String, Object> errorResponse = new HashMap<>();

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

@@ -77,6 +77,23 @@ public boolean isSharePasswordSet() {
     @Column(length = 255)
     private String folder;
 
+    // Server-owned "is a generation turn in flight for this dashboard" marker.
+    // Set to RUNNING the instant a chat submit is accepted (before the slow
+    // agent work starts) and back to IDLE when it finishes — from the backend
+    // code path itself, regardless of whether the SSE client that started it
+    // is still connected. Lets a reload mid-generation distinguish "still
+    // working" from "answer's ready" without needing to have stayed connected.
+    // See SavedDashboardService.beginGenerationTurn/appendAgentReply/
+    // completeBuildTurn/appendErrorReply.
+    @Column(nullable = false, length = 16)
+    private String generationStatus = "IDLE";
+
+    // When the current RUNNING turn started, so a client can tell a live
+    // generation from one abandoned by a backend crash (see
+    // SavedDashboardService.STALE_RUNNING_THRESHOLD).
+    @Column
+    private LocalDateTime generationStartedAt;
+
     @CreationTimestamp
     @Column(nullable = false, updatable = false)
     private LocalDateTime createdAt;
@@ -85,6 +102,16 @@ public boolean isSharePasswordSet() {
     @Column(nullable = false)
     private LocalDateTime updatedAt;
 
+    // Optimistic lock: beginGenerationTurn/appendAgentReply/completeBuildTurn/
+    // appendErrorReply all do load-then-save on this same row, and two overlapping
+    // turns (e.g. a slow build finishing after the user already sent a follow-up
+    // chat) would otherwise silently lose whichever save landed first. Hibernate
+    // bumps this on every UPDATE and rejects a save whose version is stale with
+    // OptimisticLockException instead of overwriting.
+    @Version
+    @Column(nullable = false)
+    private Long version = 0L;
+
     // Jackson deserializes create/update bodies via Lombok's all-args constructor
     // (Spring's parameter-names module), which bypasses the field defaults and
     // leaves these NOT-NULL booleans null when the client omits them. Coerce here
@@ -94,5 +121,6 @@ public boolean isSharePasswordSet() {
     void applyBooleanDefaults() {
         if (isPublic == null) isPublic = false;
         if (isFavorite == null) isFavorite = false;
+        if (generationStatus == null) generationStatus = "IDLE";
     }
 }