diff --git a/CLAUDE.md b/CLAUDE.md index b046d0e..cc9df5b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -269,6 +269,15 @@ The Agent tab must not inherit the admin MCP token. `/api/agent/session` mints a resilience but only ever handled a *moved ref*, re-issuing the identical refused request against a 429. A fallback that fails the same way as the thing it backs up is not a fallback. +7. **Never offer a write the caller cannot enforce.** `SOUL.md` once asked + "Should everyone on this database see this?" after every good answer, so + Agent chat offered "save this as a shared DeepSQL brain note" to users + without `canManageContent` and then 403'd. `get_brain_context` now stamps + `callerCapabilities`; if `doNotOffer` includes `save_brain_note`, the + agent must not mention it. MCP `save_brain_note` also fail-closes before + the POST. Admins get a non-blocking suggestion bubble only after they + correct or teach the Agent (`POST /brain/notes/propose` + accept) — a + clean first answer stays quiet. Overlaps merge into one intent. ### Verification Anti-Patterns (do not repeat) diff --git a/agent/SOUL.md b/agent/SOUL.md index 97edcd7..5e27f9b 100644 --- a/agent/SOUL.md +++ b/agent/SOUL.md @@ -2,7 +2,7 @@ You are **DeepSQL DBA**, an AI database performance assistant. You answer questi **Lead with the answer.** You ground thoroughly with the tools, but you do **not** narrate that work in your reply. No "I checked / I joined…", no "Grounding used", no "Filters applied", no "Used:" footnotes, no column/filter walkthroughs. Answer with just the result — a number, a short ranked table, or a one-line sentence — and apply business rules silently. Tool steps already show what ran; don't repeat that in the bubble. -After the answer you may offer **one short follow-up question** (a single line) when it helps the user go deeper. Do not stack multiple offers. If the user wants the SQL, the tables, or how you got there, they'll ask, and then you show it. Admit uncertainty instead of guessing; prefer one correct answer over a verbose survey. +After the answer you may offer **one short follow-up question** (a single line) when it helps the user go deeper — a question they can answer, not an action they cannot take. Do not stack multiple offers. If the user wants the SQL, the tables, or how you got there, they'll ask, and then you show it. Admit uncertainty instead of guessing; prefer one correct answer over a verbose survey. (Exception: the schema-consult flow in rule 8 — when proposing a table/migration you DO briefly state what already exists, because that's the point of the consult.) @@ -24,25 +24,35 @@ After the answer you may offer **one short follow-up question** (a single line) 8. **Consult before you commit schema.** When the user says "add a table / track X / write a migration," STOP and run the brain consult (`get_brain_context` → `get_schema` → `list_business_rules` → `get_relationships` → `get_anti_patterns`). There is almost always an existing table or column to extend instead of duplicate. Narrate what you found before proposing DDL. +9. **Never offer an action the caller cannot enforce.** `get_brain_context` and + `list_connections` carry `callerCapabilities`. If `doNotOffer` lists an + action — especially `save_brain_note` — do not mention it, do not ask + "should I save this", and do not render a Yes button. Answering a metric + is not a request to persist it. The product UI may show a non-blocking + save bubble after the user corrects or teaches a definition; leave that + to the UI. Never volunteer it yourself. + ## Remembering things — two different places -There are TWO planes of memory. Route every "remember this" to the right one: +There are TWO planes of memory. Route a remember request only when the user +explicitly asked to remember / pin / save a definition: 1. **Company brain context (shared).** Durable facts about the *data* — what a column means, a join path, a business definition, an accepted recommendation. These ground EVERYONE's answers on this connection. Save them with - **`save_brain_note(connectionId, tableName, noteText, columnName?)`**. - - "Accept this recommendation" / "remember this for the team" → review with - **`list_brain_recommendations`**, then `save_brain_note` for each good one. - - This is **admin-only** (manage-content) and audited. If the user lacks - permission, the backend rejects it — say so, don't work around it. + **`save_brain_note(connectionId, tableName, noteText, columnName?)`** + **only if** `callerCapabilities.canWriteSharedBrainNotes` is true + (`list_connections.canManageContent`). + - If they asked to remember and they cannot write: tell them an admin with + manage-content on this connection has to save it. Do not call the tool. 2. **Individual preference (yours alone).** How *this* user likes answers formatted, a private shortcut, a personal default. That is a **DeepSQL skill** on the user's own profile — it does NOT belong in the shared brain. Never push a personal preference into `save_brain_note`. -When unsure which plane a request belongs to, ask: "Should everyone on this -database see this, or just you?" Shared → brain note. Just you → DeepSQL skill. +Do not volunteer a shared-brain save after answering a data question. Do not +ask "should everyone on this database see this?" unless the user already asked +to remember something **and** they can write shared notes. ## Skills diff --git a/agent/skills/bi-query/SKILL.md b/agent/skills/bi-query/SKILL.md index 5f4fd24..9cb443f 100644 --- a/agent/skills/bi-query/SKILL.md +++ b/agent/skills/bi-query/SKILL.md @@ -27,7 +27,7 @@ Use when the user asks a question whose answer is **in the data** ("how many boo 6. **Run it** with `execute_sql(connectionId, sql, limit=…)`. Remember: default 100 rows, max 1000. For a total, `SELECT COUNT(*)` rather than counting a truncated result set. -7. **Answer only.** Reply with just the result — the number or a short ranked table — then optionally **one** short follow-up question. Apply business rules silently; do NOT append "Grounding used" / "Filters applied" / "Used:" / tool-narration / column-mapping sections. Only if the user asks how you got it do you show the tables, joins, and filters. +7. **Answer only.** Reply with just the result — the number or a short ranked table — then optionally **one** short follow-up question the user can actually act on. Do **not** offer to save a shared brain note, apply an index, or run DDL/DML unless `get_brain_context.callerCapabilities` says they can. Apply business rules silently; do NOT append "Grounding used" / "Filters applied" / "Used:" / tool-narration / column-mapping sections. Only if the user asks how you got it do you show the tables, joins, and filters. ## Guardrails diff --git a/backend/src/main/java/com/dbaagent/controller/BrainController.java b/backend/src/main/java/com/dbaagent/controller/BrainController.java index 6aabfdb..6c1e8e9 100644 --- a/backend/src/main/java/com/dbaagent/controller/BrainController.java +++ b/backend/src/main/java/com/dbaagent/controller/BrainController.java @@ -28,6 +28,9 @@ import com.dbaagent.service.brain.core.BrainNoteService; import com.dbaagent.service.brain.core.BrainTaskService; import com.dbaagent.service.brain.core.NoteSuggestionService; +import com.dbaagent.service.brain.core.BrainNoteProposalService; +import com.dbaagent.dto.BrainNoteProposalRequest; +import com.dbaagent.dto.BrainNoteProposalResponse; import com.dbaagent.service.QueryExecutorService; import com.dbaagent.service.SchemaSnapshotService; import com.dbaagent.service.brain.analysis.ColumnDisambiguationService; @@ -94,6 +97,7 @@ public class BrainController { private final BrainNoteService brainNoteService; private final BrainTaskService brainTaskService; private final NoteSuggestionService noteSuggestionService; + private final BrainNoteProposalService brainNoteProposalService; private final ColumnProfilingService columnProfilingService; private final ColumnDisambiguationService columnDisambiguationService; private final SchemaSnapshotService schemaSnapshotService; @@ -240,6 +244,51 @@ public ResponseEntity getNoteSuggestions( } } + /** + * Draft a shared-brain note from an Agent turn. Overlaps with existing notes + * or business rules are merged into one intent. Read-only — does not persist. + */ + @PostMapping("/notes/propose") + public ResponseEntity proposeNoteFromTurn( + @org.springframework.web.bind.annotation.RequestBody BrainNoteProposalRequest request + ) { + try { + if (request == null || request.getConnectionId() == null || request.getConnectionId().isBlank()) { + return ResponseEntity.badRequest().build(); + } + accessControlService.assertCanReadConnectionContent(request.getConnectionId()); + return brainNoteProposalService.proposeFromTurn(request) + .map(ResponseEntity::ok) + .orElseGet(() -> ResponseEntity.noContent().build()); + } catch (ResponseStatusException e) { + throw e; + } catch (Exception e) { + log.error("Error proposing a brain note from an agent turn", e); + return ResponseEntity.internalServerError().build(); + } + } + + /** + * Accept a proposed note. If it overlaps existing documentation, update that + * row instead of creating a second copy of the same intent. + */ + @PostMapping("/notes/accept") + public ResponseEntity acceptNote( + @org.springframework.web.bind.annotation.RequestBody BrainNoteRequest request + ) { + try { + accessControlService.assertCanManageConnectionContent(request.getConnectionId()); + return ResponseEntity.ok(brainNoteProposalService.accept(request)); + } catch (IllegalArgumentException e) { + return ResponseEntity.badRequest().build(); + } catch (ResponseStatusException e) { + throw e; + } catch (Exception e) { + log.error("Error accepting a brain note proposal", e); + return ResponseEntity.internalServerError().build(); + } + } + @GetMapping("/notes/{connectionId}") public ResponseEntity> getNotes( @PathVariable String connectionId, diff --git a/backend/src/main/java/com/dbaagent/dto/BrainNoteProposalRequest.java b/backend/src/main/java/com/dbaagent/dto/BrainNoteProposalRequest.java new file mode 100644 index 0000000..11e00ab --- /dev/null +++ b/backend/src/main/java/com/dbaagent/dto/BrainNoteProposalRequest.java @@ -0,0 +1,16 @@ +package com.dbaagent.dto; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@NoArgsConstructor +@AllArgsConstructor +public class BrainNoteProposalRequest { + private String connectionId; + private String question; + private String answer; + /** Previous assistant answer. Required for a proposal — clean first turns stay quiet. */ + private String priorAnswer; +} diff --git a/backend/src/main/java/com/dbaagent/dto/BrainNoteProposalResponse.java b/backend/src/main/java/com/dbaagent/dto/BrainNoteProposalResponse.java new file mode 100644 index 0000000..fe75873 --- /dev/null +++ b/backend/src/main/java/com/dbaagent/dto/BrainNoteProposalResponse.java @@ -0,0 +1,24 @@ +package com.dbaagent.dto; + +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class BrainNoteProposalResponse { + private String scopeType; + private String tableName; + private String columnName; + private String bubbleLabel; + private String excerpt; + private String proposedNoteText; + /** NEW, MERGE, or SKIP (SKIP is omitted from the Agent UI). */ + private String action; + private String existingNoteId; + private String existingNoteText; + private String overlapReason; +} diff --git a/backend/src/main/java/com/dbaagent/service/brain/core/BrainNoteIntentService.java b/backend/src/main/java/com/dbaagent/service/brain/core/BrainNoteIntentService.java new file mode 100644 index 0000000..8154f99 --- /dev/null +++ b/backend/src/main/java/com/dbaagent/service/brain/core/BrainNoteIntentService.java @@ -0,0 +1,443 @@ +package com.dbaagent.service.brain.core; + +import org.springframework.stereotype.Service; + +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Drafts a shared-brain note from an Agent turn and folds it into existing + * context (schema documentation + business rules) so one table/column keeps a + * single intent instead of a pile of overlapping recommendations. + */ +@Service +public class BrainNoteIntentService { + + private static final Pattern BACKTICK_IDENT = Pattern.compile("`([^`]+)`"); + private static final Set STOP_TABLES = Set.of( + "information_schema", "pg_catalog", "mysql", "sys", "performance_schema" + ); + /** + * User follow-ups that teach or correct. Phrase contains() — not a fat + * regex — so a long Agent transcript cannot ReDoS the propose path. + */ + private static final List FEEDBACK_PHRASES = List.of( + "that's wrong", "that is wrong", "that's not", "that is not", + "incorrect", "actually ", "instead", "should be", "should use", + "you should", "don't use", "do not use", "never use", + "always use", "always filter", "always join", "always exclude", + "we use", "we always", "we never", "not that", "not the ", + "pin this", "pin that", "remember this", "remember:", + "save this", "save that", "use this", "use that", + "too high", "too low", "off by", "the right " + ); + + public record ContextItem( + String id, + String tableName, + String columnName, + String text, + String source + ) {} + + public record Proposal( + String scopeType, + String tableName, + String columnName, + String bubbleLabel, + String excerpt, + String proposedNoteText, + String action, + String existingNoteId, + String existingNoteText, + String overlapReason + ) {} + + public Optional proposeFromTurn(String question, String answer, List existing) { + return proposeFromTurn(question, answer, existing, null); + } + + /** + * Only draft a note when the user just corrected or taught after a prior + * Agent answer. A clean first-turn definition is not a recommendation. + */ + public Optional proposeFromTurn( + String question, + String answer, + List existing, + String priorAnswer + ) { + if (!isCorrectionTurn(question, priorAnswer)) { + return Optional.empty(); + } + String cleaned = stripMarkdownNoise(nvl(answer)); + String combined = nvl(question) + "\n" + cleaned; + if (combined.trim().length() < 24) { + return Optional.empty(); + } + + Optional target = resolveTarget(combined); + if (target.isEmpty()) { + return Optional.empty(); + } + String tableName = target.get()[0]; + String columnName = target.get()[1]; + String excerpt = excerpt(nvl(question) + (cleaned.isBlank() ? "" : " " + cleaned)); + String proposed = columnName != null + ? "For " + tableName + "." + columnName + ": " + excerpt + : "For " + tableName + ": " + excerpt; + String label = columnName != null + ? "Save correction: " + columnName + : "Save correction: " + tableName; + + Proposal draft = new Proposal( + columnName != null ? "COLUMN" : "TABLE", + tableName, + columnName, + label, + excerpt, + proposed, + "NEW", + null, + null, + null + ); + return Optional.of(resolveOverlap(draft, existing == null ? List.of() : existing)); + } + + public boolean isCorrectionTurn(String question, String priorAnswer) { + if (priorAnswer == null || priorAnswer.isBlank()) { + return false; + } + return looksLikeUserFeedback(question); + } + + public boolean looksLikeUserFeedback(String question) { + if (question == null || question.isBlank()) { + return false; + } + String q = question.toLowerCase(Locale.ROOT).trim(); + if (q.startsWith("no,") || q.startsWith("no ") || q.startsWith("no-") + || q.startsWith("no—") || q.startsWith("nope")) { + return true; + } + for (String phrase : FEEDBACK_PHRASES) { + if (q.contains(phrase)) { + return true; + } + } + return false; + } + + public Proposal resolveOverlap(Proposal draft, List existing) { + if (draft == null) { + return null; + } + ContextItem match = findOverlap(draft.tableName(), draft.columnName(), existing); + if (match == null) { + return draft; + } + if (sameIntent(match.text(), draft.proposedNoteText())) { + return new Proposal( + draft.scopeType(), + draft.tableName(), + draft.columnName(), + draft.bubbleLabel(), + draft.excerpt(), + match.text(), + "SKIP", + match.id(), + match.text(), + "Already in " + match.source() + " with the same intent" + ); + } + String merged = mergeTexts(match.text(), draft.proposedNoteText()); + // mergeTexts already returns the existing text unchanged when the + // incoming sentence is a subset. Do not call sameIntent(existing, + // merged) here — merged always contains existing, so that check + // would skip every real merge. + if (normalizeText(merged).equals(normalizeText(match.text()))) { + return new Proposal( + draft.scopeType(), + draft.tableName(), + draft.columnName(), + draft.bubbleLabel(), + draft.excerpt(), + match.text(), + "SKIP", + match.id(), + match.text(), + "Existing context already covers this intent" + ); + } + return new Proposal( + draft.scopeType(), + draft.tableName(), + draft.columnName(), + "Update definition: " + (draft.columnName() != null ? draft.columnName() : draft.tableName()), + excerpt(merged), + merged, + "MERGE", + match.id(), + match.text(), + "Overlaps existing " + match.source() + " — keep as one intent" + ); + } + + public String mergeTexts(String existing, String incoming) { + String left = existing == null ? "" : existing.trim(); + String right = incoming == null ? "" : incoming.trim(); + if (left.isEmpty()) { + return right; + } + if (right.isEmpty() || sameIntent(left, right) || containsNormalized(left, right)) { + return left; + } + if (containsNormalized(right, left)) { + return right; + } + return left + (left.endsWith(".") ? " " : ". ") + right; + } + + public boolean sameIntent(String left, String right) { + Set a = tokens(left); + Set b = tokens(right); + if (a.isEmpty() || b.isEmpty()) { + return false; + } + if (containsNormalized(left, right) || containsNormalized(right, left)) { + return true; + } + int inter = 0; + for (String token : a) { + if (b.contains(token)) { + inter++; + } + } + int union = a.size() + b.size() - inter; + return union > 0 && (inter * 1.0 / union) >= 0.72; + } + + private ContextItem findOverlap(String tableName, String columnName, List existing) { + String table = normalizeName(tableName); + String column = normalizeName(columnName); + ContextItem tableMatch = null; + for (ContextItem item : existing) { + if (item == null || !tableMatches(table, normalizeName(item.tableName()))) { + continue; + } + String itemCol = normalizeName(item.columnName()); + if (!column.isEmpty() && column.equals(itemCol)) { + return item; + } + if (column.isEmpty() && itemCol.isEmpty()) { + return item; + } + if (tableMatch == null && itemCol.isEmpty()) { + tableMatch = item; + } + } + return tableMatch; + } + + /** + * Collect every backtick ident first. Returning on the first dotted table + * used to drop a metric already seen ({@code `meditator_count_current`} then + * {@code `marts.dim_person`}). Prefer {@code schema.table.column}, then a + * dotted table plus a distinct {@code _}-containing ident, then a dotted + * table found in prose. + */ + private Optional resolveTarget(String text) { + List idents = new ArrayList<>(); + Matcher ticks = BACKTICK_IDENT.matcher(text); + while (ticks.find()) { + String ident = ticks.group(1).trim(); + if (!ident.isEmpty()) { + idents.add(ident); + } + } + for (String ident : idents) { + String[] parts = ident.split("\\."); + if (parts.length >= 3 && !STOP_TABLES.contains(parts[0].toLowerCase(Locale.ROOT))) { + return Optional.of(new String[] { parts[0] + "." + parts[1], parts[2] }); + } + } + String table = null; + String column = null; + for (String ident : idents) { + if (ident.contains(".")) { + String[] parts = ident.split("\\."); + if (parts.length >= 2 && !STOP_TABLES.contains(parts[0].toLowerCase(Locale.ROOT))) { + table = parts[0] + "." + parts[1]; + } + } else if (column == null && ident.contains("_") && ident.length() > 3) { + column = ident; + } + } + if (table != null) { + return Optional.of(new String[] { table, column }); + } + String[] prose = findFirstQualifiedTable(text); + if (prose != null) { + return Optional.of(new String[] { prose[0] + "." + prose[1], column }); + } + return Optional.empty(); + } + + /** + * Linear scan for {@code schema.table} (not {@code a.b.c}). Avoids the + * backtracking {@code [\w]*\.[\w]*} pattern CodeQL flags as ReDoS on + * attacker-controlled Agent answers. + */ + static String[] findFirstQualifiedTable(String text) { + if (text == null || text.isEmpty()) { + return null; + } + int n = text.length(); + int i = 0; + while (i < n) { + char c = text.charAt(i); + if (!isIdentStart(c)) { + i++; + continue; + } + if (i > 0) { + char prev = text.charAt(i - 1); + if (isIdentPart(prev) || prev == '.') { + i++; + continue; + } + } + int schemaStart = i; + i++; + while (i < n && isIdentPart(text.charAt(i))) { + i++; + } + if (i >= n || text.charAt(i) != '.') { + continue; + } + int schemaEnd = i; + i++; + if (i >= n || !isIdentStart(text.charAt(i))) { + continue; + } + int nameStart = i; + i++; + while (i < n && isIdentPart(text.charAt(i))) { + i++; + } + if (i < n) { + char next = text.charAt(i); + if (isIdentPart(next) || next == '.') { + continue; + } + } + String schema = text.substring(schemaStart, schemaEnd); + if (STOP_TABLES.contains(schema.toLowerCase(Locale.ROOT))) { + continue; + } + return new String[] { schema, text.substring(nameStart, i) }; + } + return null; + } + + private static boolean isIdentStart(char c) { + return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_'; + } + + private static boolean isIdentPart(char c) { + return isIdentStart(c) || (c >= '0' && c <= '9'); + } + + /** + * Exact table match, or one side is a bare name of a schema-qualified + * table. Two different schemas with the same local name stay distinct. + */ + private boolean tableMatches(String left, String right) { + if (left.isEmpty() || right.isEmpty()) { + return false; + } + if (left.equals(right)) { + return true; + } + boolean leftQualified = left.contains("."); + boolean rightQualified = right.contains("."); + if (leftQualified && !rightQualified) { + return left.endsWith("." + right); + } + if (rightQualified && !leftQualified) { + return right.endsWith("." + left); + } + return false; + } + + private static String nvl(String value) { + return value == null ? "" : value; + } + + private String excerpt(String text) { + String compact = text.replaceAll("\\s+", " ").trim(); + if (compact.length() <= 280) { + return compact; + } + int cut = compact.lastIndexOf('.', 280); + if (cut < 80) { + cut = 280; + } + return compact.substring(0, cut).trim(); + } + + private String stripMarkdownNoise(String answer) { + String withoutCode = answer.replaceAll("(?s)```.*?```", " "); + withoutCode = withoutCode.replaceAll("(?m)^\\|.*\\|$", " "); + withoutCode = withoutCode.replaceAll("\\*\\*|__", ""); + return withoutCode.replaceAll("\\s+", " ").trim(); + } + + private boolean containsNormalized(String haystack, String needle) { + String a = normalizeText(haystack); + String b = normalizeText(needle); + return !b.isEmpty() && a.contains(b); + } + + private Set tokens(String text) { + Set out = new LinkedHashSet<>(); + for (String part : normalizeText(text).split(" ")) { + if (part.length() >= 3) { + out.add(part); + } + } + return out; + } + + private String normalizeText(String text) { + if (text == null) { + return ""; + } + return text.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9._\\s]", " ").replaceAll("\\s+", " ").trim(); + } + + private String normalizeName(String name) { + return name == null ? "" : name.trim().toLowerCase(Locale.ROOT); + } + + public List contextFromNotesAndRules( + List notes, + List rules + ) { + List all = new ArrayList<>(); + if (notes != null) { + all.addAll(notes); + } + if (rules != null) { + all.addAll(rules); + } + return all; + } +} diff --git a/backend/src/main/java/com/dbaagent/service/brain/core/BrainNoteProposalService.java b/backend/src/main/java/com/dbaagent/service/brain/core/BrainNoteProposalService.java new file mode 100644 index 0000000..c773c6c --- /dev/null +++ b/backend/src/main/java/com/dbaagent/service/brain/core/BrainNoteProposalService.java @@ -0,0 +1,131 @@ +package com.dbaagent.service.brain.core; + +import com.dbaagent.dto.BrainNoteProposalRequest; +import com.dbaagent.dto.BrainNoteProposalResponse; +import com.dbaagent.model.SchemaDocumentation; +import com.dbaagent.model.brain.BrainNoteRequest; +import com.dbaagent.model.brain.BrainNoteResponse; +import com.dbaagent.model.brain.BrainRule; +import com.dbaagent.repository.SchemaDocumentationRepository; +import com.dbaagent.repository.brain.BrainRuleRepository; +import lombok.RequiredArgsConstructor; +import org.springframework.stereotype.Service; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +@Service +@RequiredArgsConstructor +public class BrainNoteProposalService { + + private final BrainNoteIntentService intentService; + private final BrainNoteService brainNoteService; + private final SchemaDocumentationRepository documentationRepository; + private final BrainRuleRepository brainRuleRepository; + + public Optional proposeFromTurn(BrainNoteProposalRequest request) { + if (request == null || request.getConnectionId() == null || request.getConnectionId().isBlank()) { + return Optional.empty(); + } + List context = loadContext(request.getConnectionId()); + return intentService.proposeFromTurn( + request.getQuestion(), + request.getAnswer(), + context, + request.getPriorAnswer() + ) + .filter(proposal -> !"SKIP".equals(proposal.action())) + .map(this::toResponse); + } + + public BrainNoteResponse accept(BrainNoteRequest request) { + if (request == null) { + throw new IllegalArgumentException("Request is required"); + } + List context = loadContext(request.getConnectionId()); + BrainNoteIntentService.Proposal draft = new BrainNoteIntentService.Proposal( + request.getScopeType(), + request.getTableName(), + request.getColumnName(), + null, + request.getNoteText(), + request.getNoteText(), + "NEW", + null, + null, + null + ); + BrainNoteIntentService.Proposal resolved = intentService.resolveOverlap(draft, context); + if (resolved != null && "MERGE".equals(resolved.action()) && resolved.existingNoteId() != null + && isDocumentationNote(resolved.existingNoteId())) { + BrainNoteRequest update = new BrainNoteRequest(); + update.setScopeType(resolved.scopeType()); + update.setTableName(resolved.tableName()); + update.setColumnName(resolved.columnName()); + update.setNoteText(resolved.proposedNoteText()); + update.setCreatedBy(request.getCreatedBy()); + return brainNoteService.updateNote(resolved.existingNoteId(), update); + } + if (resolved != null && "SKIP".equals(resolved.action()) && resolved.existingNoteId() != null + && isDocumentationNote(resolved.existingNoteId())) { + return brainNoteService.getNotes( + request.getConnectionId(), + resolved.scopeType(), + resolved.tableName(), + resolved.columnName() + ).stream().findFirst().orElseGet(() -> brainNoteService.createNote(request)); + } + if (resolved != null) { + request.setNoteText(resolved.proposedNoteText()); + request.setScopeType(resolved.scopeType()); + request.setTableName(resolved.tableName()); + request.setColumnName(resolved.columnName()); + } + return brainNoteService.createNote(request); + } + + private boolean isDocumentationNote(String id) { + return documentationRepository.findById(id).isPresent(); + } + + private List loadContext(String connectionId) { + List items = new ArrayList<>(); + for (SchemaDocumentation doc : documentationRepository.findByConnectionId(connectionId)) { + if (doc.getObjectType() == SchemaDocumentation.DocumentationType.TABLE) { + items.add(new BrainNoteIntentService.ContextItem( + doc.getId(), doc.getObjectName(), null, doc.getDescription(), "brain note" + )); + } else if (doc.getObjectType() == SchemaDocumentation.DocumentationType.COLUMN) { + items.add(new BrainNoteIntentService.ContextItem( + doc.getId(), doc.getParentObject(), doc.getObjectName(), doc.getDescription(), "brain note" + )); + } + } + for (BrainRule rule : brainRuleRepository.findByConnectionIdAndIsActiveTrueOrderByCreatedAtDesc(connectionId)) { + items.add(new BrainNoteIntentService.ContextItem( + rule.getId(), + rule.getTableName(), + rule.getColumnName(), + rule.getRuleText(), + "business rule" + )); + } + return items; + } + + private BrainNoteProposalResponse toResponse(BrainNoteIntentService.Proposal proposal) { + return BrainNoteProposalResponse.builder() + .scopeType(proposal.scopeType()) + .tableName(proposal.tableName()) + .columnName(proposal.columnName()) + .bubbleLabel(proposal.bubbleLabel()) + .excerpt(proposal.excerpt()) + .proposedNoteText(proposal.proposedNoteText()) + .action(proposal.action()) + .existingNoteId(proposal.existingNoteId()) + .existingNoteText(proposal.existingNoteText()) + .overlapReason(proposal.overlapReason()) + .build(); + } +} diff --git a/backend/src/test/java/com/dbaagent/service/brain/core/BrainNoteIntentServiceTest.java b/backend/src/test/java/com/dbaagent/service/brain/core/BrainNoteIntentServiceTest.java new file mode 100644 index 0000000..6f746a0 --- /dev/null +++ b/backend/src/test/java/com/dbaagent/service/brain/core/BrainNoteIntentServiceTest.java @@ -0,0 +1,148 @@ +package com.dbaagent.service.brain.core; + +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; + +class BrainNoteIntentServiceTest { + + private static final String PRIOR_WRONG = + "There are 12,004 people in dim_person."; + private static final String CORRECTION = + "No, that's wrong — the pinned metric is `meditator_count_current` from `marts.dim_person`."; + private static final String AGENT_AFTER = "290066 meditators on that pinned metric."; + + private final BrainNoteIntentService service = new BrainNoteIntentService(); + + @Test + void proposeFromTurn_staysQuietWhenTheFirstAnswerNeedsNoFeedback() { + Optional proposal = service.proposeFromTurn( + "what is the meditator count?", + "The correct pinned metric is `meditator_count_current` from `marts.dim_person`, totaling 290066 meditators.", + List.of(), + null + ); + + assertThat(proposal).isEmpty(); + } + + @Test + void proposeFromTurn_staysQuietWhenTheUserJustThanksTheAgent() { + Optional proposal = service.proposeFromTurn( + "thanks, that's right", + "Glad it helped.", + List.of(), + "The correct pinned metric is `meditator_count_current` from `marts.dim_person`." + ); + + assertThat(proposal).isEmpty(); + } + + @Test + void proposeFromTurn_offersAfterUserCorrectsTheAgent() { + Optional proposal = service.proposeFromTurn( + CORRECTION, + AGENT_AFTER, + List.of(), + PRIOR_WRONG + ); + + assertThat(proposal).isPresent(); + assertThat(proposal.get().action()).isEqualTo("NEW"); + assertThat(proposal.get().tableName()).isEqualTo("marts.dim_person"); + assertThat(proposal.get().columnName()).isEqualTo("meditator_count_current"); + assertThat(proposal.get().bubbleLabel()).contains("correction"); + assertThat(proposal.get().excerpt()).contains("meditator_count_current"); + assertThat(proposal.get().proposedNoteText()).contains("marts.dim_person"); + } + + @Test + void proposeFromTurn_skipsWhenExistingNoteIsSameIntent() { + String existing = "For marts.dim_person.meditator_count_current: No, that's wrong — the pinned metric is meditator_count_current from marts.dim_person. 290066 meditators on that pinned metric."; + Optional proposal = service.proposeFromTurn( + CORRECTION, + AGENT_AFTER, + List.of(new BrainNoteIntentService.ContextItem( + "note-1", "marts.dim_person", "meditator_count_current", existing, "brain note" + )), + PRIOR_WRONG + ); + + assertThat(proposal).isPresent(); + assertThat(proposal.get().action()).isEqualTo("SKIP"); + assertThat(proposal.get().existingNoteId()).isEqualTo("note-1"); + } + + @Test + void proposeFromTurn_mergesOverlappingContextIntoOneIntent() { + Optional proposal = service.proposeFromTurn( + "No, that's wrong — the pinned metric is `meditator_count_current` from `marts.dim_person`, excluding ambiguous matches.", + AGENT_AFTER, + List.of(new BrainNoteIntentService.ContextItem( + "note-1", + "marts.dim_person", + "meditator_count_current", + "dim_person is the person dimension used for IRC region rollups.", + "brain note" + )), + PRIOR_WRONG + ); + + assertThat(proposal).isPresent(); + assertThat(proposal.get().action()).isEqualTo("MERGE"); + assertThat(proposal.get().proposedNoteText()).contains("person dimension"); + assertThat(proposal.get().proposedNoteText()).contains("pinned metric"); + assertThat(proposal.get().overlapReason()).contains("one intent"); + } + + @Test + void proposeFromTurn_matchesBareTableNameAgainstQualifiedContext() { + Optional proposal = service.proposeFromTurn( + "No, that's wrong — the pinned metric is `meditator_count_current` from `marts.dim_person`, excluding ambiguous matches.", + AGENT_AFTER, + List.of(new BrainNoteIntentService.ContextItem( + "rule-1", + "dim_person", + "meditator_count_current", + "dim_person is the person dimension used for IRC region rollups.", + "business rule" + )), + PRIOR_WRONG + ); + + assertThat(proposal).isPresent(); + assertThat(proposal.get().action()).isEqualTo("MERGE"); + assertThat(proposal.get().overlapReason()).contains("one intent"); + } + + @Test + void proposeFromTurn_ignoresNonDefinitionAnswers() { + Optional proposal = service.proposeFromTurn( + "how many tables are there?", + "There are 42 tables in this database.", + List.of() + ); + assertThat(proposal).isEmpty(); + } + + @Test + void findFirstQualifiedTable_skipsTripleQualifiedAndCatalogSchemas() { + assertThat(BrainNoteIntentService.findFirstQualifiedTable("see marts.dim_person.col then crm.accounts")) + .containsExactly("crm", "accounts"); + assertThat(BrainNoteIntentService.findFirstQualifiedTable("pg_catalog.pg_class")).isNull(); + assertThat(BrainNoteIntentService.findFirstQualifiedTable("A".repeat(20_000) + " marts.dim_person")) + .containsExactly("marts", "dim_person"); + } + + @Test + void mergeTexts_doesNotDuplicateTheSameSentence() { + String merged = service.mergeTexts( + "Always filter cancelled bookings.", + "always filter cancelled bookings" + ); + assertThat(merged).isEqualTo("Always filter cancelled bookings."); + } +} diff --git a/docs/public/cli-and-mcp.md b/docs/public/cli-and-mcp.md index ef87f76..be10761 100644 --- a/docs/public/cli-and-mcp.md +++ b/docs/public/cli-and-mcp.md @@ -179,7 +179,7 @@ The MCP surface mirrors the `deepsql` CLI for almost every read/diagnostic opera | Tool | What it does | |---|---| | `list_connections` | List databases this user can see. **Always call first** — every other tool needs the UUID. | -| `get_current_user` | Authenticated user, role, and bound DeepSQL host. Use to know whether the caller is admin-capable before suggesting DDL. | +| `get_current_user` | Authenticated user, role, and `callerCapabilities`. Read `doNotOffer` before suggesting a write. | | `show_connection` | One connection's saved config with all secret fields masked as `(set)`. | | `test_connection` | Run the privilege report (+ SSH check) using the saved encrypted credentials. No plaintext crosses the agent's wire. | | `reinit_connection_brain` | Trigger a fresh schema scan + brain re-embedding (use after the user reports stale schema knowledge). | @@ -190,7 +190,7 @@ The MCP surface mirrors the `deepsql` CLI for almost every read/diagnostic opera |---|---| | `get_schema` | Fetch cached schema (tables, columns, FKs, types). Fast and cheap — call freely. | | `get_database_objects` | Tables, views, functions, procedures. Use when you need DDL-level objects, not just columns. | -| `get_brain_context` | **Primary retrieval tool.** Returns the tables, columns, FKs, business rules, and anti-patterns most relevant to your question. **Call before generating any non-trivial SQL or DDL.** | +| `get_brain_context` | **Primary retrieval tool.** Returns the tables, columns, FKs, business rules, and anti-patterns most relevant to your question, plus `callerCapabilities`. **Call before generating any non-trivial SQL or DDL. Never offer an action in `doNotOffer`.** | | `list_business_rules` | Active business rules and SQL guardrails. Honor these — they encode domain semantics (e.g. `always_filter_cancelled`). | | `get_relationships` | Inferred + validated foreign keys with confidence scores. Many real-world DBs lack declared FKs; this fills the gap. | diff --git a/mcp/CLAUDE.md b/mcp/CLAUDE.md index 6af38ac..5566f77 100644 --- a/mcp/CLAUDE.md +++ b/mcp/CLAUDE.md @@ -60,7 +60,7 @@ in this version; ask before mid-session admin work. | Tool | Purpose | |---|---| -| `list_connections` | List databases the user has access to. Always call this first — you need IDs for everything else. | +| `list_connections` | List databases the user has access to. Always call this first — you need IDs for everything else. Each row includes `canManageContent`; if false, do not offer writes on that connection. | | `get_schema` | Cached schema metadata (tables, columns, FKs, types). Cheap and fast — call freely. | | `get_database_objects` | Tables, views, functions, procedures. Use when you need DDL-level objects, not just columns. | | `get_brain_context` | **Your primary retrieval tool.** Given a question, returns the tables/columns/FKs/training docs/business rules/anti-patterns most relevant to it. | @@ -68,7 +68,7 @@ in this version; ask before mid-session admin work. | `get_relationships` | Inferred + validated foreign keys with confidence scores. Many real DBs lack declared FKs; this fills the gap. | | `get_anti_patterns` | Schema-level (`kind=table`) or query-level (`kind=query`) anti-patterns. | | `list_brain_recommendations` | The brain's AI-proposed notes to review for a connection (priority, reason, indicators, suggested prompt). The company-context review queue. | -| `save_brain_note` | **Accept/save a fact into the shared brain** — grounds every future answer for the connection. TABLE- or COLUMN-scoped. Admin (manage-content), audited. Personal preferences belong in a DeepSQL skill, not here. | +| `save_brain_note` | **Accept/save a fact into the shared brain** — only when the user asked to remember it AND `callerCapabilities.canWriteSharedBrainNotes` is true. Never volunteer after answering a question. | | `list_brain_notes` | Knowledge already saved to the brain (filter by table/column; can be thousands). | | `analyze_slow_queries` | Recent slow queries with fingerprints, durations, examples. Read-only; doesn't trigger new work. | | `get_slow_query_timeline` | Day-by-day timeline for one query from the 30-day analytics store — call count, mean/max time, regression factor per day. Identify the query by its fingerprint (the `queryId` from `analyze_slow_queries`). Answers "is this query getting slower". | diff --git a/mcp/README.md b/mcp/README.md index 66ec069..edad8ea 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -126,11 +126,11 @@ per-user admin ops (`users`/`access`/`permissions`). |---|---| | `get_schema` | Cached schema metadata (tables, columns, FKs, types) | | `get_database_objects` | Tables, views, functions, procedures | -| `get_brain_context` | Retrieval brain: tables/columns/FKs/training docs/rules for a question | +| `get_brain_context` | Retrieval brain plus `callerCapabilities` (what this user can enforce) | | `list_business_rules` | Active business rules and SQL guardrails for a connection | | `get_relationships` | Inferred + validated foreign keys with confidence scores | -| `list_brain_recommendations` | The brain's AI-proposed notes to review (company-context queue) | -| `save_brain_note` | Accept/save a fact into the shared brain (admin; grounds future answers) | +| `list_brain_recommendations` | The brain's AI-proposed notes to review (do not offer accept unless caller can write) | +| `save_brain_note` | Save a shared brain fact only when the user asked and `canManageContent` is true | | `list_brain_notes` | Knowledge already saved to the brain (filterable) | ### Anti-patterns + daily digest (4) diff --git a/mcp/deepsql-phase1-lib.js b/mcp/deepsql-phase1-lib.js index 5e899ed..5396d52 100644 --- a/mcp/deepsql-phase1-lib.js +++ b/mcp/deepsql-phase1-lib.js @@ -39,7 +39,10 @@ const EXPLAIN_PREFIX_PATTERN = /^EXPLAIN(?:\s*\([^)]*\))?/; const TOOL_DEFINITIONS = [ { name: "list_connections", - description: "List DeepSQL database connections available to this user.", + description: + "List DeepSQL database connections available to this user. Each row includes " + + "canManageContent / canManageConfig — if canManageContent is false, do not " + + "offer save_brain_note, APPLY index changes, or other writes on that connection.", inputSchema: { type: "object", properties: {}, @@ -79,7 +82,10 @@ const TOOL_DEFINITIONS = [ { name: "get_brain_context", description: - "Retrieve DeepSQL's brain context for a question: relevant tables, columns, FKs, training docs, business rules, anti-patterns, and embedding-ranked snippets. Use this to give your own coding agent the same retrieval context the DeepSQL agent uses, then have your agent generate the SQL/answer.", + "Retrieve DeepSQL's brain context for a question: relevant tables, columns, FKs, training docs, business rules, anti-patterns, and embedding-ranked snippets. " + + "The result also includes callerCapabilities for this connection — read doNotOffer " + + "before suggesting any write (shared brain notes, APPLY indexes, SQL DML/DDL). " + + "Never volunteer an action listed there.", inputSchema: { type: "object", properties: { @@ -157,7 +163,10 @@ const TOOL_DEFINITIONS = [ { name: "list_brain_recommendations", description: - "List the brain's AI-proposed recommendations for a connection — high-value tables/columns DeepSQL suggests documenting, each with a priority (P0/P1…), the reason, supporting indicators, and a suggested prompt to explore. This is the company-context review queue: an admin reviews these and accepts the good ones with save_brain_note. Returns { suggestions, totalCount } (totalCount reflects the requested limit, not an absolute total).", + "List the brain's AI-proposed recommendations for a connection — high-value tables/columns DeepSQL suggests documenting, each with a priority (P0/P1…), the reason, supporting indicators, and a suggested prompt to explore. " + + "This is an admin review queue. Do not present it as something the current user can accept " + + "unless callerCapabilities.canWriteSharedBrainNotes is true (from get_brain_context / list_connections). " + + "Returns { suggestions, totalCount } (totalCount reflects the requested limit, not an absolute total).", inputSchema: { type: "object", properties: { @@ -171,7 +180,12 @@ const TOOL_DEFINITIONS = [ { name: "save_brain_note", description: - "Accept/save a fact into the connection's BRAIN — shared, company-level context that grounds EVERY future answer for this connection (not a per-user preference). Use this to accept a recommendation from list_brain_recommendations, or to record any durable fact about a table/column. Scope is TABLE (tableName only) or COLUMN (tableName + columnName). Requires manage-content permission on the connection (admin) — the backend enforces and audits it. NOTE: an individual's personal preference (how *they* like answers formatted, a private shortcut) belongs in a DeepSQL skill on their own profile, NOT here — this writes to the shared brain everyone sees.", + "Accept/save a fact into the connection's BRAIN — shared, company-level context that grounds EVERY future answer for this connection (not a per-user preference). " + + "ONLY call this when the user explicitly asked to remember/pin a team definition AND " + + "list_connections.canManageContent / get_brain_context.callerCapabilities.canWriteSharedBrainNotes is true. " + + "Never volunteer this after answering a data question. Never ask 'should I save this as a shared brain note' " + + "when the caller cannot write. Scope is TABLE (tableName only) or COLUMN (tableName + columnName). " + + "Personal preferences belong in a DeepSQL skill, not here.", inputSchema: { type: "object", properties: { @@ -589,9 +603,9 @@ const TOOL_DEFINITIONS = [ name: "get_current_user", description: "Return the authenticated user behind the current MCP token: username, role, " - + "and the DeepSQL host this MCP server is bound to. Use this when the agent " - + "needs to know whether the caller is admin-capable before suggesting a " - + "DDL/DML run, or when explaining role-based restrictions to the user.", + + "permissions, and callerCapabilities (what this user can actually enforce). " + + "Read doNotOffer before suggesting a write. Role ADMIN can mutate SQL; " + + "shared brain notes and APPLY indexes also need canManageContent on the connection.", inputSchema: { type: "object", properties: {}, additionalProperties: false }, }, { @@ -1359,15 +1373,86 @@ async function callDeepSqlApi(config, path, options = {}) { return payload; } +function asConnectionList(payload) { + if (Array.isArray(payload)) return payload; + if (Array.isArray(payload?.items)) return payload.items; + return []; +} + +function findConnection(payload, connectionId) { + const id = String(connectionId || "").trim(); + if (!id) return null; + return asConnectionList(payload).find((connection) => + (connection.id || connection.connectionId) === id + ) || null; +} + +function isTruthyFlag(value) { + return value === true || value === "true"; +} + +/** + * What the current caller can actually enforce on one connection. + * Fail closed when the connection row is missing: offering a write the + * backend will 403 is worse than staying silent. + */ +function buildCallerCapabilities(connection, user = null) { + const role = user?.role != null ? String(user.role).toUpperCase() : null; + const canManageContent = isTruthyFlag(connection?.canManageContent); + const canManageConfig = isTruthyFlag(connection?.canManageConfig); + const canMutateSql = role === "ADMIN"; + const doNotOffer = []; + if (!canManageContent) { + doNotOffer.push( + "save_brain_note", + "accept list_brain_recommendations", + "apply_index_recommendation APPLY", + "reinit_connection_brain", + ); + } + if (!canManageConfig) { + doNotOffer.push("set_growth_config"); + } + if (!canMutateSql) { + doNotOffer.push("SQL DML", "SQL DDL"); + } + return { + username: user?.username || null, + role, + connectionId: connection?.id || connection?.connectionId || null, + accessLevel: connection?.accessLevel || null, + canManageContent, + canManageConfig, + canWriteSharedBrainNotes: canManageContent, + canMutateSql, + doNotOffer, + }; +} + +function summarizeCallerCapabilities(caps) { + if (!caps) return ""; + if (caps.canWriteSharedBrainNotes && caps.canMutateSql) { + return "Caller can write shared brain notes and mutate SQL on this connection."; + } + const banned = (caps.doNotOffer || []).join("; "); + return ( + `Caller cannot enforce write actions on this connection ` + + `(manage-content=${caps.canManageContent ? "yes" : "no"}, role=${caps.role || "unknown"}). ` + + `Do not offer: ${banned}.` + ); +} + function summarizeConnections(connections) { - const lines = connections.map((connection) => { + const list = asConnectionList(connections); + const lines = list.map((connection) => { const name = connection.connectionName || connection.name || connection.id; const type = connection.dbType || "unknown"; - return `- ${name} (${type}) — ${connection.id}`; + const content = isTruthyFlag(connection.canManageContent) ? "manage-content" : "read-only content"; + return `- ${name} (${type}) — ${connection.id} [${content}]`; }); return lines.length - ? `Found ${connections.length} connection(s):\n${lines.join("\n")}` + ? `Found ${list.length} connection(s):\n${lines.join("\n")}` : "No connections were returned by DeepSQL."; } @@ -1401,7 +1486,8 @@ function summarizeBrainContext(payload) { const total = payload.totalResults ?? (Array.isArray(payload.results) ? payload.results.length : 0); const tableCount = Array.isArray(payload.tablesCovered) ? payload.tablesCovered.length : 0; - return `Retrieved ${total} ranked snippet(s) covering ${tableCount} table(s).`; + const caps = summarizeCallerCapabilities(payload.callerCapabilities); + return `Retrieved ${total} ranked snippet(s) covering ${tableCount} table(s).${caps ? ` ${caps}` : ""}`; } // /training/context rich shape (RetrievedContextResult) const tables = Array.isArray(payload.ragTableNames) @@ -1414,7 +1500,9 @@ function summarizeBrainContext(payload) { : ""; const intent = payload.retrievalIntent || "n/a"; const skipped = payload.skipped ? ` (skipped: ${payload.skipReason || "?"})` : ""; - return `Brain context: intent=${intent}, topK=${payload.retrievalTopK ?? "?"}, results=${payload.resultCount ?? 0}, tables=${tables}${types ? `, types[${types}]` : ""}${skipped}.`; + const caps = summarizeCallerCapabilities(payload.callerCapabilities); + const capLine = caps ? ` ${caps}` : ""; + return `Brain context: intent=${intent}, topK=${payload.retrievalTopK ?? "?"}, results=${payload.resultCount ?? 0}, tables=${tables}${types ? `, types[${types}]` : ""}${skipped}.${capLine}`; } function summarizeBusinessRules(payload) { @@ -1429,13 +1517,35 @@ function summarizeRelationships(payload) { return `${list.length} relationship(s) (${high} high-confidence).`; } +function summarizeCurrentUser(payload) { + if (!payload || typeof payload !== "object") { + return "Current user unavailable."; + } + const name = payload.username || payload.email || "unknown"; + const role = payload.role || "unknown"; + const caps = summarizeCallerCapabilities(payload.callerCapabilities); + if (caps) { + return `Authenticated as ${name} (role ${role}). ${caps}`; + } + return `Authenticated as ${name} (role ${role}). Check list_connections.canManageContent before offering writes.`; +} + function summarizeBrainRecommendations(payload) { const list = (payload && payload.suggestions) || []; - if (!list.length) return "No brain recommendations pending review for this connection."; + const caps = payload && payload.callerCapabilities; + const suffix = caps && !caps.canWriteSharedBrainNotes + ? " This caller cannot accept them — do not offer save_brain_note." + : ""; + if (!list.length) { + return `No brain recommendations pending review for this connection.${suffix}`; + } const top = list .slice(0, 5) .map((s) => `${s.priority || ""} ${s.columnName ? `${s.tableName}.${s.columnName}` : s.tableName}`.trim()); - return `${payload.totalCount ?? list.length} recommendation(s) to review. Top: ${top.join("; ")}. Accept the good ones with save_brain_note.`; + const accept = suffix + ? suffix + : " Accept the good ones with save_brain_note."; + return `${payload.totalCount ?? list.length} recommendation(s) to review. Top: ${top.join("; ")}.${accept}`; } function summarizeBrainNoteSaved(payload) { @@ -1805,6 +1915,9 @@ function buildToolResult(name, payload, extra = {}) { case "list_connections": summary = summarizeConnections(payload); break; + case "get_current_user": + summary = summarizeCurrentUser(payload); + break; case "get_schema": summary = summarizeSchema(payload); break; @@ -1910,9 +2023,19 @@ function buildToolError(message, extra = {}) { // list changes rarely; a 30s TTL keeps it fresh enough. const CONNECTIONS_CACHE_TTL_MS = 30000; let _connectionsCache = null; // { key, ts, payload } +let _currentUserCache = null; // { key, ts, payload } + +function resetToolCaches() { + _connectionsCache = null; + _currentUserCache = null; +} + +function cacheKey(config) { + return `${(config && config.baseUrl) || ""}|${getAuthToken(config)}`; +} async function fetchConnectionsCached(config) { - const key = `${(config && config.baseUrl) || ""}|${getAuthToken(config)}`; + const key = cacheKey(config); if (_connectionsCache && _connectionsCache.key === key && Date.now() - _connectionsCache.ts < CONNECTIONS_CACHE_TTL_MS) { return _connectionsCache.payload; @@ -1922,6 +2045,41 @@ async function fetchConnectionsCached(config) { return payload; } +async function fetchCurrentUserCached(config) { + const key = cacheKey(config); + if (_currentUserCache && _currentUserCache.key === key + && Date.now() - _currentUserCache.ts < CONNECTIONS_CACHE_TTL_MS) { + return _currentUserCache.payload; + } + try { + const payload = await callDeepSqlApi(config, "/auth/me"); + _currentUserCache = { key, ts: Date.now(), payload }; + return payload; + } catch { + return null; + } +} + +async function resolveCallerCapabilities(config, connectionId) { + const [connections, user] = await Promise.all([ + fetchConnectionsCached(config).catch(() => []), + fetchCurrentUserCached(config), + ]); + const connection = findConnection(connections, connectionId); + return buildCallerCapabilities(connection, user); +} + +function attachCallerCapabilities(payload, caps) { + if (payload && typeof payload === "object" && !Array.isArray(payload)) { + payload.callerCapabilities = caps; + return payload; + } + return { + result: payload, + callerCapabilities: caps, + }; +} + async function handleToolCall(config, name, args = {}) { switch (name) { case "list_connections": { @@ -1969,7 +2127,8 @@ async function handleToolCall(config, name, args = {}) { { method: "POST", json: { question } }, ); } - return buildToolResult(name, payload); + const caps = await resolveCallerCapabilities(config, connectionId); + return buildToolResult(name, attachCallerCapabilities(payload, caps)); } case "list_business_rules": { @@ -2018,7 +2177,8 @@ async function handleToolCall(config, name, args = {}) { config, `/brain/notes/suggestions/${encodeURIComponent(connectionId)}?limit=${limit}`, ); - return buildToolResult(name, payload); + const caps = await resolveCallerCapabilities(config, connectionId); + return buildToolResult(name, attachCallerCapabilities(payload, caps)); } case "save_brain_note": { @@ -2029,7 +2189,14 @@ async function handleToolCall(config, name, args = {}) { if (!tableName) return buildToolError("tableName is required."); if (!noteText) return buildToolError("noteText is required."); const columnName = args.columnName ? String(args.columnName).trim() : null; - // Backend enforces manage-content permission (admin) + audits the write. + const caps = await resolveCallerCapabilities(config, connectionId); + if (!caps.canWriteSharedBrainNotes) { + return buildToolError( + "This account cannot write shared DeepSQL brain notes on this connection. " + + "Do not offer to save a shared brain note and do not ask the user if they want one.", + { errorCode: "POLICY_CONTENT_WRITE_DENIED", callerCapabilities: caps }, + ); + } const payload = await callDeepSqlApi(config, "/brain/notes", { method: "POST", json: { @@ -2285,7 +2452,22 @@ async function handleToolCall(config, name, args = {}) { case "get_current_user": { const payload = await callDeepSqlApi(config, "/auth/me"); - return buildToolResult(name, payload); + let connections = []; + try { + connections = asConnectionList(await fetchConnectionsCached(config)); + } catch { + connections = []; + } + const writable = connections.filter((c) => isTruthyFlag(c.canManageContent)); + const caps = buildCallerCapabilities(writable[0] || connections[0] || null, payload); + if (connections.length > 0 && writable.length === 0) { + caps.canWriteSharedBrainNotes = false; + caps.canManageContent = false; + if (!caps.doNotOffer.includes("save_brain_note")) { + caps.doNotOffer.unshift("save_brain_note"); + } + } + return buildToolResult(name, attachCallerCapabilities(payload, caps)); } case "test_connection": { @@ -2553,5 +2735,10 @@ module.exports = { summarizeSlowQueryOptimization, summarizeTableGrowth, summarizeTrackedQueries, + summarizeCallerCapabilities, + summarizeConnections, + summarizeCurrentUser, + buildCallerCapabilities, + resetToolCaches, validateReadOnlySql, }; diff --git a/mcp/deepsql-phase1-lib.test.js b/mcp/deepsql-phase1-lib.test.js index 5ae3ea1..55aad48 100644 --- a/mcp/deepsql-phase1-lib.test.js +++ b/mcp/deepsql-phase1-lib.test.js @@ -17,6 +17,10 @@ const { createConfigFromEnv, getAuthToken, invalidateTokenCache, + buildCallerCapabilities, + summarizeCallerCapabilities, + summarizeConnections, + resetToolCaches, } = require("./deepsql-phase1-lib"); function tmpTokenFile(contents) { @@ -639,6 +643,7 @@ test("handleToolCall(get_growth_anomalies) omits unacknowledgedOnly when not exp }); function makeFakeConfig(callsOut, responses) { + resetToolCaches(); let i = 0; const originalFetch = global.fetch; global.fetch = async (url, init) => { @@ -724,11 +729,14 @@ test("connection-write tools are intentionally NOT exposed (secrets-in-history r test("handleToolCall(get_current_user) hits /auth/me with no args", async () => { const calls = []; - const cfg = makeFakeConfig(calls, [{ username: "alice", role: "ADMIN" }]); + const cfg = makeFakeConfig(calls, [ + { username: "alice", role: "ADMIN" }, + [{ id: "c1", canManageContent: true, canManageConfig: true }], + ]); const result = await handleToolCall(cfg, "get_current_user", {}); - assert.equal(calls.length, 1); assert.match(calls[0].url, /\/auth\/me$/); assert.equal(result.structuredContent.username, "alice"); + assert.equal(result.structuredContent.callerCapabilities.canWriteSharedBrainNotes, true); }); test("handleToolCall(test_connection) sends only id — never plaintext creds", async () => { @@ -863,14 +871,19 @@ test("handleToolCall(list_brain_recommendations) GETs suggestions with the limit test("handleToolCall(save_brain_note) POSTs a COLUMN-scoped note", async () => { const calls = []; - const cfg = makeFakeConfig(calls, [{ tableName: "orders", columnName: "status", noteText: "x" }]); + const cfg = makeFakeConfig(calls, [ + [{ id: "c1", canManageContent: true }], + { username: "admin", role: "ADMIN" }, + { tableName: "orders", columnName: "status", noteText: "x" }, + ]); const result = await handleToolCall(cfg, "save_brain_note", { connectionId: "c1", tableName: "orders", columnName: "status", noteText: "x", }); - assert.equal(calls[0].method, "POST"); - assert.match(calls[0].url, /\/brain\/notes$/); - assert.equal(calls[0].body.scopeType, "COLUMN"); - assert.equal(calls[0].body.tableName, "orders"); + const noteCall = calls.find((c) => /\/brain\/notes$/.test(c.url)); + assert.ok(noteCall, "should POST /brain/notes after the capability check"); + assert.equal(noteCall.method, "POST"); + assert.equal(noteCall.body.scopeType, "COLUMN"); + assert.equal(noteCall.body.tableName, "orders"); assert.match(result.content[0].text, /Saved to brain/); }); @@ -1074,3 +1087,64 @@ test("callDeepSqlApi does not retry a 401 when the token file is unchanged", asy } assert.equal(stub.authHeaders.length, 1, "unchanged token → no pointless retry"); }); + +test("buildCallerCapabilities fail-closes when the connection cannot write notes", () => { + const caps = buildCallerCapabilities( + { id: "c1", canManageContent: false, accessLevel: "CHAT_EDITOR" }, + { username: "marts-editor", role: "DEVELOPER" }, + ); + assert.equal(caps.canWriteSharedBrainNotes, false); + assert.ok(caps.doNotOffer.includes("save_brain_note")); + assert.match(summarizeCallerCapabilities(caps), /Do not offer:.*save_brain_note/); +}); + +test("buildCallerCapabilities allows shared notes when canManageContent is true", () => { + const caps = buildCallerCapabilities( + { id: "c1", canManageContent: true, canManageConfig: true }, + { username: "admin", role: "ADMIN" }, + ); + assert.equal(caps.canWriteSharedBrainNotes, true); + assert.equal(caps.canMutateSql, true); + assert.equal(caps.doNotOffer.includes("save_brain_note"), false); +}); + +test("summarizeConnections surfaces read-only vs manage-content", () => { + const text = summarizeConnections([ + { id: "c1", connectionName: "ACME", dbType: "postgresql", canManageContent: false }, + { id: "c2", connectionName: "Owned", dbType: "postgresql", canManageContent: true }, + ]); + assert.match(text, /ACME.*\[read-only content\]/); + assert.match(text, /Owned.*\[manage-content\]/); +}); + +test("handleToolCall(save_brain_note) refuses before POST when caller cannot write", async () => { + const calls = []; + const cfg = makeFakeConfig(calls, [ + [{ id: "c1", canManageContent: false, accessLevel: "CHAT_EDITOR" }], + { username: "marts-editor", role: "DEVELOPER" }, + ]); + const result = await handleToolCall(cfg, "save_brain_note", { + connectionId: "c1", tableName: "marts.dim_person", noteText: "meditator_count_current", + }); + assert.equal(result.isError, true); + assert.match(result.content[0].text, /cannot write shared DeepSQL brain notes/); + assert.match(result.content[0].text, /Do not offer to save/); + assert.equal(result.structuredContent.errorCode, "POLICY_CONTENT_WRITE_DENIED"); + assert.equal(calls.some((c) => /\/brain\/notes$/.test(c.url) && c.method === "POST"), false); +}); + +test("handleToolCall(get_brain_context) stamps callerCapabilities onto the payload", async () => { + const calls = []; + const cfg = makeFakeConfig(calls, [ + { retrievalIntent: "metric", resultCount: 1, ragTableNames: ["marts.dim_person"] }, + [{ id: "c1", canManageContent: false, accessLevel: "CHAT_EDITOR" }], + { username: "marts-editor", role: "DEVELOPER" }, + ]); + const result = await handleToolCall(cfg, "get_brain_context", { + connectionId: "c1", + question: "how many meditators", + }); + assert.equal(result.structuredContent.callerCapabilities.canWriteSharedBrainNotes, false); + assert.ok(result.structuredContent.callerCapabilities.doNotOffer.includes("save_brain_note")); + assert.match(result.content[0].text, /Do not offer:.*save_brain_note/); +}); diff --git a/mcp/package.json b/mcp/package.json index ad2c339..4daaec7 100644 --- a/mcp/package.json +++ b/mcp/package.json @@ -1,6 +1,6 @@ { "name": "@deepsql/mcp", - "version": "0.27.0", + "version": "0.27.1", "description": "DeepSQL CLI, DBA Agent (thin client), and stdio MCP server for self-hosted deployments", "bin": { "deepsql": "bin/deepsql.js", diff --git a/mcp/skills/SKILL_BODY.md b/mcp/skills/SKILL_BODY.md index 0a44151..6736195 100644 --- a/mcp/skills/SKILL_BODY.md +++ b/mcp/skills/SKILL_BODY.md @@ -91,8 +91,8 @@ usually doesn't know about either; that's exactly why DeepSQL exists. | `list_business_rules(connectionId, question?)` | Rules the SQL must respect. | | `get_relationships(connectionId)` | Foreign keys (declared + inferred-with-confidence). | | `get_anti_patterns(connectionId, kind="table"\|"query")` | Patterns to avoid in this DB. | -| `list_brain_recommendations(connectionId, limit?)` | The brain's AI-proposed things to document (the review queue). Admin accepts good ones with `save_brain_note`. | -| `save_brain_note(connectionId, tableName, noteText, columnName?)` | **Accept/save a fact to the SHARED company brain** — grounds every future answer for this connection. Admin (manage-content). NOT for personal prefs — those go in a DeepSQL skill. | +| `list_brain_recommendations(connectionId, limit?)` | The brain's AI-proposed things to document (the review queue). Only offer accept/save when `callerCapabilities.canWriteSharedBrainNotes` is true. | +| `save_brain_note(connectionId, tableName, noteText, columnName?)` | **Accept/save a fact to the SHARED company brain** — only when the user asked to remember it AND `canWriteSharedBrainNotes` is true. Never volunteer after answering a question. | | `list_brain_notes(connectionId, tableName?, columnName?)` | Knowledge already in the brain; filter before saving a duplicate. | | `analyze_slow_queries(connectionId, thresholdMs?, limit?)` | Snapshot of slow queries from live stats. | | `get_slow_query_timeline(connectionId, fingerprint)` | Day-by-day timeline for one fingerprint: call count, mean/max time, regression factor per day. Answers "is this query getting slower". | @@ -106,7 +106,7 @@ usually doesn't know about either; that's exactly why DeepSQL exists. | `get_growth_anomalies(connectionId, tableName?, unacknowledgedOnly?, days?)` | DeepSQL-flagged sudden growth spikes with severity (CRITICAL/WARNING/INFO), anomaly type, before/after sizes, confidence score. Check this BEFORE walking the user through a slow-query plan — a recent growth anomaly is often the real root cause. | | `execute_sql(connectionId, query, ...)` | Run any SQL — SELECT for everyone, DML/DDL for admins (two-step confirm). | | `analyze_query_plan(connectionId, query, useAnalyze=false)` | AI-enriched plan analysis (issues + index recs + summary). | -| `get_current_user()` | Authenticated user + role. Use to know whether the caller is admin-capable before suggesting DDL. | +| `get_current_user()` | Authenticated user + role + `callerCapabilities`. Read `doNotOffer` before suggesting any write. | | `test_connection(connectionId)` | Validates a saved connection (privilege report + SSH tunnel check). Read-only on the customer's DB. | | `show_connection(connectionId)` | Full saved config with secrets masked. Diagnose host/port/SSL/SSH issues. | | `reinit_connection_brain(connectionId, force?)` | Trigger a fresh schema scan + brain re-embedding. Use after the user reports stale schema knowledge. | diff --git a/src/components/AgentChat/AgentChatPanel.jsx b/src/components/AgentChat/AgentChatPanel.jsx index 0326c6b..f2ec796 100644 --- a/src/components/AgentChat/AgentChatPanel.jsx +++ b/src/components/AgentChat/AgentChatPanel.jsx @@ -3,8 +3,15 @@ import { ArrowUp, Plus, Square, Loader2, Database, Sparkles, Hash, Table2, Clock import { agentChatAPI, withConnectionContext } from '@/lib/api/agentClient' import { agentConversationAPI } from '@/lib/api/client' import { useAuth } from '@/hooks/useAuth' +import { brainAPI } from '@/lib/api/client' import AgentMarkdown from './AgentMarkdown' +import AgentRecommendationBubbles from './AgentRecommendationBubbles' import { sanitizeAssistantAnswer } from './sanitizeAssistantAnswer' +import { + isSuppressedProposal, + proposalTargetKey, + shouldOfferBrainSuggestion, +} from './shouldOfferBrainSuggestion' import styles from './AgentChatPanel.module.css' function updateLast(messages, updater) { @@ -14,6 +21,15 @@ function updateLast(messages, updater) { return copy } +function findPriorAssistant(messages) { + for (let i = messages.length - 3; i >= 0; i--) { + if (messages[i]?.role === 'assistant' && messages[i].content) { + return messages[i] + } + } + return null +} + function toolLabel(d) { const name = (d?.name || '').replace(/^mcp_deepsql_/, '').replace(/^skill_view$/, 'skill') if (d?.args?.query) return `SQL · ${String(d.args.query).replace(/\s+/g, ' ').slice(0, 90)}` @@ -41,7 +57,7 @@ const SUGGESTIONS = [ { icon: Clock, text: 'What are the top slow queries?' }, ] -export default function AgentChatPanel({ connectionId, connectionName }) { +export default function AgentChatPanel({ connectionId, connectionName, canManageContent = false }) { const { username } = useAuth() const [sessionId, setSessionId] = useState(null) const [booting, setBooting] = useState(true) @@ -58,6 +74,7 @@ export default function AgentChatPanel({ connectionId, connectionName }) { const streamIdRef = useRef(null) const listRef = useRef(null) const firstMsgRef = useRef(true) + const suppressedTargetsRef = useRef(new Set()) const profileRef = useRef(null) // resolved agent profile (for new sessions) const convIdRef = useRef(null) // backend conversation id (the per-user index row) const restoredRef = useRef(false) // guards the persist effect until boot finishes @@ -164,7 +181,16 @@ export default function AgentChatPanel({ connectionId, connectionName }) { // tool step in the collapsible activity — so the bubble ends with just the // final answer (the text after the last tool). onTool: (d) => setMessages((m) => updateLast(m, (a) => ({ ...a, content: '', tools: [...(a.tools || []), toolLabel(d)] }))), - onEnd: () => { setMessages((m) => updateLast(m, (a) => ({ ...a, streaming: false }))); setSending(false); esRef.current = null }, + onEnd: () => { + setMessages((m) => updateLast(m, (a) => ({ ...a, streaming: false }))) + setSending(false) + esRef.current = null + // Correction chips load after the turn so the composer is already + // free. Clean first answers stay quiet. Failures never block chat. + if (canManageContent) { + queueMicrotask(() => proposeFromLastTurn()) + } + }, onError: () => { setMessages((m) => updateLast(m, (a) => ({ ...a, streaming: false, error: true }))); setSending(false); esRef.current = null }, }) } catch (e) { @@ -173,6 +199,50 @@ export default function AgentChatPanel({ connectionId, connectionName }) { } } + const proposeFromLastTurn = () => { + setMessages((current) => { + const last = current[current.length - 1] + const user = current[current.length - 2] + if (!last || last.role !== 'assistant' || last.error || last.proposal) { + return current + } + const priorAssistant = findPriorAssistant(current) + const hasUnsavedProposal = current.some((m) => m.proposal && !m.proposal.status) + if (!shouldOfferBrainSuggestion({ + userText: user?.role === 'user' ? user.content : '', + priorAssistantText: priorAssistant?.content || '', + hasUnsavedProposal, + })) { + return current + } + const answer = sanitizeAssistantAnswer(last.content || '') + const question = user?.role === 'user' ? user.content : '' + brainAPI.proposeNoteFromTurn({ + connectionId, + question, + answer, + priorAnswer: priorAssistant.content, + }) + .then((proposal) => { + if (!proposal) return + if (isSuppressedProposal(proposal, [...suppressedTargetsRef.current])) return + setMessages((msgs) => updateLast(msgs, (assistant) => ( + assistant.proposal ? assistant : { ...assistant, proposal } + ))) + }) + .catch(() => { /* no bubble is fine — chat stays usable */ }) + return current + }) + } + + const suppressAndPatch = (proposal, patch) => { + const key = proposalTargetKey(proposal) + if (key) suppressedTargetsRef.current.add(key) + setMessages((m) => updateLast(m, (a) => ( + a.proposal ? { ...a, proposal: { ...a.proposal, ...patch } } : a + ))) + } + const stop = async () => { if (streamIdRef.current) await agentChatAPI.cancel(streamIdRef.current) esRef.current?.close(); esRef.current = null @@ -248,6 +318,14 @@ export default function AgentChatPanel({ connectionId, connectionName }) { ? : m.streaming && } {m.error &&
The agent run ended early.
} + {canManageContent && !m.streaming && m.proposal && ( + suppressAndPatch(m.proposal, { status: 'dismissed' })} + onAccepted={() => suppressAndPatch(m.proposal, { status: 'saved' })} + /> + )} ) : (
{m.content}
diff --git a/src/components/AgentChat/AgentChatPanel.module.css b/src/components/AgentChat/AgentChatPanel.module.css index dd758c4..521f753 100644 --- a/src/components/AgentChat/AgentChatPanel.module.css +++ b/src/components/AgentChat/AgentChatPanel.module.css @@ -266,6 +266,54 @@ .msgError { font-size: 12px; color: #b42318; margin-top: 6px; } +.recWrap { margin-top: 12px; max-width: 520px; } +.recBubbles { display: flex; flex-wrap: wrap; gap: 8px; } +.recBubble { + display: inline-flex; align-items: center; gap: 6px; + border: 1px solid #dcd8f4; background: #f6f5fd; color: #3d3690; + border-radius: 999px; padding: 6px 12px; font-size: 12.5px; font-weight: 550; + cursor: pointer; + transition: background 120ms ease-out, border-color 120ms ease-out, transform 120ms cubic-bezier(0.34, 1.4, 0.64, 1); +} +.recBubble:hover { background: #eeedfe; border-color: #c4bdf0; } +.recBubble:active { transform: scale(0.97); } +.recBubbleOpen { background: #eeedfe; border-color: #534AB7; } +.recCard { + margin-top: 8px; padding: 12px 14px; + border: 1px solid #ececec; border-radius: 12px; background: #fff; + box-shadow: 0 6px 16px -12px rgba(17, 17, 17, 0.2); +} +.recCardHead { display: flex; align-items: center; justify-content: space-between; gap: 8px; } +.recCardKicker { font-size: 11px; font-weight: 600; letter-spacing: 0.02em; text-transform: uppercase; color: #6b668f; } +.recDismiss { + border: none; background: transparent; color: #888; cursor: pointer; padding: 2px; + border-radius: 6px; +} +.recDismiss:hover { background: #f4f4f4; color: #333; } +.recTarget { margin: 6px 0 4px; font-size: 12.5px; font-weight: 600; color: #111; font-family: var(--font-family-mono, ui-monospace, Menlo, monospace); } +.recOverlap { margin: 0 0 8px; font-size: 12px; color: #6b668f; } +.recExcerpt { + margin: 0; padding: 10px 12px; border-left: 3px solid #534AB7; + background: #f7f7fb; color: #333; font-size: 13px; line-height: 1.5; border-radius: 0 8px 8px 0; +} +.recActions { display: flex; gap: 8px; margin-top: 12px; } +.recAccept { + border: none; background: #534AB7; color: #fff; border-radius: 8px; + padding: 7px 12px; font-size: 12.5px; font-weight: 550; cursor: pointer; +} +.recAccept:hover:not(:disabled) { background: #463c9f; } +.recAccept:disabled { background: #d8d5f0; cursor: default; } +.recCancel { + border: 1px solid #e2e2e2; background: #fff; color: #333; border-radius: 8px; + padding: 7px 12px; font-size: 12.5px; font-weight: 500; cursor: pointer; +} +.recCancel:hover { background: #f6f6f6; } +.recError { margin: 8px 0 0; font-size: 12px; color: #b42318; } +@media (prefers-reduced-motion: reduce) { + .recBubble { transition: background 120ms ease-out, border-color 120ms ease-out; } + .recBubble:active { transform: none; } +} + .typing { display: inline-flex; gap: 4px; padding: 4px 0; } .typing span { width: 6px; height: 6px; border-radius: 50%; background: #cfcaf0; animation: blink 1.2s infinite both; } .typing span:nth-child(2) { animation-delay: .2s; } diff --git a/src/components/AgentChat/AgentRecommendationBubbles.jsx b/src/components/AgentChat/AgentRecommendationBubbles.jsx new file mode 100644 index 0000000..480e9c7 --- /dev/null +++ b/src/components/AgentChat/AgentRecommendationBubbles.jsx @@ -0,0 +1,86 @@ +import { useState } from 'react' +import { BookmarkPlus, X } from 'lucide-react' +import { brainAPI } from '@/lib/api/client' +import styles from './AgentChatPanel.module.css' + +/** + * Non-blocking suggestion chips under an Agent answer. Chat stays usable; + * clicking a bubble only opens an excerpt of the shared-brain rule that + * would be created (or merged into an existing one). + */ +export default function AgentRecommendationBubbles({ connectionId, proposal, onDismiss, onAccepted }) { + const [open, setOpen] = useState(false) + const [saving, setSaving] = useState(false) + const [error, setError] = useState(null) + + if (!proposal || proposal.status === 'dismissed' || proposal.status === 'saved') { + return null + } + + const accept = async (event) => { + event.preventDefault() + event.stopPropagation() + if (saving) return + setSaving(true) + setError(null) + try { + await brainAPI.acceptNote({ + connectionId, + scopeType: proposal.scopeType, + tableName: proposal.tableName, + columnName: proposal.columnName || undefined, + noteText: proposal.proposedNoteText, + }) + onAccepted?.() + } catch (e) { + setError(e?.response?.data?.message || e?.message || 'Could not save this definition') + } finally { + setSaving(false) + } + } + + return ( +
+
+ +
+ {open && ( +
+
+ + {proposal.action === 'MERGE' ? 'Merge into existing context' : 'Shared brain note'} + + +
+

+ {proposal.columnName + ? `${proposal.tableName}.${proposal.columnName}` + : proposal.tableName} +

+ {proposal.overlapReason && ( +

{proposal.overlapReason}

+ )} +
{proposal.excerpt || proposal.proposedNoteText}
+
+ + +
+ {error &&

{error}

} +
+ )} +
+ ) +} diff --git a/src/components/AgentChat/AgentRecommendationBubbles.test.js b/src/components/AgentChat/AgentRecommendationBubbles.test.js new file mode 100644 index 0000000..1be189b --- /dev/null +++ b/src/components/AgentChat/AgentRecommendationBubbles.test.js @@ -0,0 +1,33 @@ +import test from 'node:test' +import assert from 'node:assert/strict' + +// The bubble renderer is a React component; this file locks the payload +// contract the Agent panel attaches after a turn so a missing excerpt or +// MERGE action cannot silently drop the admin preview. + +test('proposal payload for a new definition includes excerpt and bubble label', () => { + const proposal = { + scopeType: 'COLUMN', + tableName: 'marts.dim_person', + columnName: 'meditator_count_current', + bubbleLabel: 'Save correction: meditator_count_current', + excerpt: 'The correct pinned metric is meditator_count_current from marts.dim_person', + proposedNoteText: 'For marts.dim_person.meditator_count_current: The correct pinned metric is meditator_count_current from marts.dim_person', + action: 'NEW', + } + assert.ok(proposal.bubbleLabel.startsWith('Save correction:')) + assert.ok(proposal.excerpt.length > 20) + assert.equal(proposal.action, 'NEW') +}) + +test('merge proposal keeps one intent and names the overlap', () => { + const proposal = { + action: 'MERGE', + overlapReason: 'Overlaps existing brain note — keep as one intent', + proposedNoteText: 'Existing region rule. New pinned metric definition.', + } + assert.equal(proposal.action, 'MERGE') + assert.match(proposal.overlapReason, /one intent/) + assert.match(proposal.proposedNoteText, /Existing/) + assert.match(proposal.proposedNoteText, /pinned metric/) +}) diff --git a/src/components/AgentChat/shouldOfferBrainSuggestion.js b/src/components/AgentChat/shouldOfferBrainSuggestion.js new file mode 100644 index 0000000..03a2d50 --- /dev/null +++ b/src/components/AgentChat/shouldOfferBrainSuggestion.js @@ -0,0 +1,72 @@ +/** + * Suggestion chips only after the user corrects or teaches. A clean first + * answer must not spawn a save bubble. + * + * Phrase contains() — not a fat regex — so a long transcript cannot ReDoS. + */ +export const FEEDBACK_PHRASES = [ + "that's wrong", + 'that is wrong', + "that's not", + 'that is not', + 'incorrect', + 'actually ', + 'instead', + 'should be', + 'should use', + 'you should', + "don't use", + 'do not use', + 'never use', + 'always use', + 'always filter', + 'always join', + 'always exclude', + 'we use', + 'we always', + 'we never', + 'not that', + 'not the ', + 'pin this', + 'pin that', + 'remember this', + 'remember:', + 'save this', + 'save that', + 'use this', + 'use that', + 'too high', + 'too low', + 'off by', + 'the right ', +] + +export function looksLikeUserFeedback(userText) { + if (!userText || !String(userText).trim()) return false + const q = String(userText).toLowerCase().trim() + if (q.startsWith('no,') || q.startsWith('no ') || q.startsWith('no-') + || q.startsWith('no—') || q.startsWith('nope')) { + return true + } + return FEEDBACK_PHRASES.some((phrase) => q.includes(phrase)) +} + +export function proposalTargetKey(proposal) { + if (!proposal) return '' + return `${proposal.tableName || ''}::${proposal.columnName || ''}` +} + +export function isSuppressedProposal(proposal, suppressedTargets = []) { + const key = proposalTargetKey(proposal) + return Boolean(key && suppressedTargets.includes(key)) +} + +export function shouldOfferBrainSuggestion({ + userText, + priorAssistantText, + hasUnsavedProposal = false, +} = {}) { + if (hasUnsavedProposal) return false + if (!priorAssistantText || !String(priorAssistantText).trim()) return false + return looksLikeUserFeedback(userText) +} diff --git a/src/components/AgentChat/shouldOfferBrainSuggestion.test.js b/src/components/AgentChat/shouldOfferBrainSuggestion.test.js new file mode 100644 index 0000000..b3f6a11 --- /dev/null +++ b/src/components/AgentChat/shouldOfferBrainSuggestion.test.js @@ -0,0 +1,48 @@ +import test from 'node:test' +import assert from 'node:assert/strict' +import { shouldOfferBrainSuggestion, looksLikeUserFeedback, isSuppressedProposal } from './shouldOfferBrainSuggestion.js' + +test('clean first-turn answers do not earn a chip', () => { + assert.equal(shouldOfferBrainSuggestion({ + userText: 'what is the meditator count?', + priorAssistantText: '', + }), false) + assert.equal(shouldOfferBrainSuggestion({ + userText: 'what is the meditator count?', + priorAssistantText: undefined, + }), false) +}) + +test('thanks after a good answer does not earn a chip', () => { + assert.equal(looksLikeUserFeedback("thanks, that's right"), false) + assert.equal(shouldOfferBrainSuggestion({ + userText: "thanks, that's right", + priorAssistantText: 'The pinned metric is meditator_count_current.', + }), false) +}) + +test('a correction after a prior answer earns a chip', () => { + assert.equal(shouldOfferBrainSuggestion({ + userText: "No, that's wrong — use meditator_count_current", + priorAssistantText: 'There are 12,004 people.', + }), true) + assert.equal(shouldOfferBrainSuggestion({ + userText: 'Always filter cancelled bookings on the bookings table', + priorAssistantText: 'Here are all bookings.', + }), true) +}) + +test('do not stack another chip while one is still open', () => { + assert.equal(shouldOfferBrainSuggestion({ + userText: "No, that's wrong — use meditator_count_current", + priorAssistantText: 'There are 12,004 people.', + hasUnsavedProposal: true, + }), false) +}) + +test('dismissed targets stay quiet for the rest of the session', () => { + assert.equal(isSuppressedProposal( + { tableName: 'marts.dim_person', columnName: 'meditator_count_current' }, + ['marts.dim_person::meditator_count_current'] + ), true) +}) diff --git a/src/components/sections/AgentChatSection.jsx b/src/components/sections/AgentChatSection.jsx index 9ae20fc..de4fd44 100644 --- a/src/components/sections/AgentChatSection.jsx +++ b/src/components/sections/AgentChatSection.jsx @@ -21,6 +21,7 @@ export default function AgentChatSection() { key={`${username || 'anon'}:${connectionId}`} connectionId={connectionId} connectionName={selectedConnection?.connectionName} + canManageContent={Boolean(selectedConnection?.canManageContent)} /> ) } diff --git a/src/lib/api/client.js b/src/lib/api/client.js index 1c8b628..2ac8c02 100644 --- a/src/lib/api/client.js +++ b/src/lib/api/client.js @@ -904,6 +904,14 @@ export const brainAPI = { ); return response.data; }, + proposeNoteFromTurn: async (payload) => { + const response = await apiClient.post("/api/brain/notes/propose", payload); + return response.status === 204 ? null : response.data; + }, + acceptNote: async (payload) => { + const response = await apiClient.post("/api/brain/notes/accept", payload); + return response.data; + }, getKeyColumns: async (connectionId, params = {}) => { const response = await apiClient.get( `/api/brain/key-columns/${connectionId}`,