Dashboard improvements

[notSumit25]

No description provided.

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[cursor]

Review

Solid product cut — server-owned turn persistence + resumable chat is the right fix for “built but lost on reload,” and the Source/Queries UX is clean. CI required checks look good (frontend/backend/mcp green).

Approve directionally — please address before merge

1. Gallery pollution from chat-only turns
   beginGenerationTurn creates a SavedDashboard row even for "hi" / chat-only prompts when dashboardId is null. That will litter the gallery with husks named after greetings. Prefer either: don’t create a row until a real build succeeds, or create only when dashboardId is already set / the classifier says it’s a build.

2. ACL: read vs manage on a mutating path
   Stream generate now creates/updates saved dashboards but only calls assertCanReadConnectionContent. Prefer assertCanManageConnectionContent (or read for generate + manage for the persist/create side) so VIEWER can’t mint/mutate drafts.

3. Concurrent turn race
   isFreshlyRunning then save is TOCTOU — two parallel submits can both pass and double-append chat. Make the RUNNING transition atomic (UPDATE … WHERE generation_status = 'IDLE' / version column) or otherwise serialize per dashboard id.

Nits (non-blocking)

• Empty PR body — please add a short summary of the four commits (chat-trace fix, server persist, resumable FE, Source/Queries).
• Unbounded chatMessages JSON growth over long threads — worth a soft cap later.
• Blocking POST /generate still doesn’t use the new persist path (fine if UI is stream-only; call that out).
• CodeQL titleFromHtml autofix (/[<>]/g) is fine for display titles; keep titles as text (React), never dangerouslySetInnerHTML.

What looks good

• Persist-before-SSE-notify so a dead client can’t mark success as failure.
• created event + new: → real-id aliasing.
• pagehide unload handling so reload doesn’t write fake errors into chat.
• Uncontrolled Monaco (defaultValue + remount key) — correct.
• Stale RUNNING (20m) + FE poll mirror.
• CLAUDE.md bootstrap/login docs correction.

Verdict: request the three items above, then this is merge-ready for main.

[geekypunk]

@notSumit25 Nice work — the comments explain why rather than what, the migration is idempotent and explicitly acknowledges that schema here is Hibernate-managed, and DashboardArtifact.jsx adds the query telemetry without touching sandbox, the CSP, or the bridge's trust boundary.

Two things that are easy to get wrong and are right here:

• Existing rows are safe. generationStatus has a Java default, a @PrePersist null guard, and isFreshlyRunning() is null-safe on both fields — so rows predating the column neither NPE nor falsely read as RUNNING.
• // Persist BEFORE attempting to notify the client is the correct ordering. A client that has already disconnected can't turn an successful result into a recorded failure.

Findings below, most important first.

---

1. Check-then-act race in beginGenerationTurn — medium

if (isFreshlyRunning(dashboard)) throw new IllegalStateException(...);
...
dashboard.setGenerationStatus("RUNNING");
return savedDashboardRepository.save(dashboard);

The read and the write aren't atomic, and SavedDashboard has no @Version. Two submits against the same dashboard — a double-click, or two open tabs — both read IDLE and both proceed. That's two concurrent agent runs, each costing LLM spend, racing to write the artifact. The guard reads like a mutex but isn't one.

2. Lost update on chatMessages — medium, same root cause

parseMessages → add → writeMessages is a read-modify-write of a whole JSON array in a text column. Under the race above, one turn's user message and reply are silently overwritten by the other.

Both 1 and 2 close with a single @Version field on SavedDashboard — the losing transaction gets an OptimisticLockException instead of silently clobbering.

3. A crash locks the dashboard for 20 minutes — low/medium

STALE_RUNNING_THRESHOLD is the right instinct, but the failure surfaces as a hard IllegalStateException: "A generation is already running for this dashboard." After a backend restart mid-generation the user can't retry for up to 20 minutes, and the message asserts something is running when nothing is. Worth either a shorter window or an explicit override.

Same shape as the db-scheduler dead-execution gap in #39 — a staleness window longer than a user's patience, where "stuck" is indistinguishable from "working".

4. A read permission now gates a write — a decision, not a blocker

assertCanReadConnectionContent previously guarded a path that only computed. This PR makes that same path create rows and mutate chat history, so a user with read-only access to a connection can now create dashboards on it.

Worth noting this cuts the right way overall: POST /api/saved-dashboards — the path this replaces — has no access check at all, so this PR is strictly stricter than what it supersedes. Filing that separately.

---

Happy to see this land once @Version is in; everything else is follow-up material.

[cursor]

Merge attempt

PR is approved (venkateshsakamuri-lab), CI green, and the follow-up commit addresses the merge blockers from review:

• ✅ Manage ACL on generate/stream (assertCanManageConnectionContent)
• ✅ Concurrent race via @Version + OptimisticLockingFailureException → clean conflict
• ⚠️ Gallery pollution from chat-only "hi" still creates a SavedDashboard husk — acceptable follow-up, not blocking given approval

Blocked from merging by repo ruleset: required status check is literally named

analyze (${{ matrix.language }})

(unexpanded matrix expression). Real CodeQL jobs are analyze (java-kotlin) and analyze (javascript-typescript), which already pass. Direct main push is also denied (Changes must be made through a pull request).

To unblock (repo admin)

Rules → Protect main branch → Required status checks — replace

analyze (${{ matrix.language }})

with:

• analyze (java-kotlin)
• analyze (javascript-typescript)

Then squash-merge #51 (or I can retry the merge once that’s fixed).

Follow-up after merge: don’t create a gallery row for chat-only turns when dashboardId is null.