From 078b2933e7b24dc2049f6320978b465b2403db0d Mon Sep 17 00:00:00 2001 From: Alex Skatell Date: Thu, 14 May 2026 13:32:03 -0400 Subject: [PATCH] feat(query): resolve --pipeline names client-side in audit/snapshot (v1.6.2) Why - v1.6.1 query audit / query snapshot only accept opaque pipeline IDs. Passing a name (e.g. 'Sales - Flex - Triage') silently returned rows: [], pushing the agent to drop into raw query sql to look up the ID and then re-run audit. That's the v1.6.1 -> v1.6.2 loophole shape: primitive forces a wrapper round-trip the skill rules can't bind. What - New resolvePipelineID() runs before the audit/snapshot SQL: - Opaque 20-char ID (e.g. CLUy1QapsrEeBiNrmQiL) passes through. - Otherwise tokens are AND'd as case-insensitive LIKEs against the pipelines table. - 1 match -> use it, surface matchedId/matchedName/matchedBy. - 0 match -> error lists every available pipeline. - >1 match -> error lists the candidates. - Audit JSON gains pipelineResolution so the agent (and humans) see how the input was matched. matchedBy = "id" | "name". - Skill docs (hermes + claude-code) bumped to v1.6.2: - --pipeline now reads PIPELINE_ID_OR_NAME. - New banned-in-default-flow rule: don't run raw query sql to resolve a pipeline name to its ID. - Pitfall #17 documents the v1.6.1 failure trace and the v1.6.2 fix. Tests - TestQueryAuditResolvesPipelineByName: 'flex triage' -> Triage ID, resolution echo'd in JSON, audit SQL uses opaque ID. - TestQueryAuditUnknownPipelineListsCandidates: 'no such pipeline' errors with the candidate list. - TestQueryAuditAmbiguousPipelineErrors: 'flex' errors with both Flex pipelines and an "ambiguous" hint. - TestQuerySnapshotResolvesPipelineByName: same resolver path for query snapshot. All resolver and existing tests pass; go build ./... clean. --- internal/commands/query.go | 195 ++++++++++++++++++++++++++--- internal/commands/query_test.go | 214 ++++++++++++++++++++++++++++++++ skills/claude-code/SKILL.md | 6 +- skills/hermes/SKILL.md | 6 +- 4 files changed, 400 insertions(+), 21 deletions(-) diff --git a/internal/commands/query.go b/internal/commands/query.go index 9c7f886..5d9ce61 100644 --- a/internal/commands/query.go +++ b/internal/commands/query.go @@ -7,6 +7,8 @@ import ( "io" "net/url" "os" + "regexp" + "sort" "strings" "time" @@ -92,13 +94,17 @@ func printQueryHelp(w io.Writer) { _, _ = fmt.Fprintln(w, " topline query explain --tables contacts,opportunities") _, _ = fmt.Fprintln(w, " topline query sql --sql 'SELECT COUNT(*) AS n FROM contacts'") _, _ = fmt.Fprintln(w, " topline query freshness") - _, _ = fmt.Fprintln(w, " topline query snapshot --pipeline ") - _, _ = fmt.Fprintln(w, " topline query audit --pipeline [--since this-week-et] [--status open]") + _, _ = fmt.Fprintln(w, " topline query snapshot --pipeline ") + _, _ = fmt.Fprintln(w, " topline query audit --pipeline [--since this-week-et] [--status open]") _, _ = fmt.Fprintln(w, "") _, _ = fmt.Fprintln(w, "Composite commands (Phase 3): one CLI call wraps the warehouse views") _, _ = fmt.Fprintln(w, "shipped in Topline-com/os-mcp#1. Use these for pipeline audits instead") _, _ = fmt.Fprintln(w, "of hand-stitched CTEs.") _, _ = fmt.Fprintln(w, "") + _, _ = fmt.Fprintln(w, "--pipeline accepts an opaque 20-char ID (e.g. CLUy1QapsrEeBiNrmQiL)") + _, _ = fmt.Fprintln(w, "or a fuzzy name (e.g. 'flex triage'). On ambiguous or missing names") + _, _ = fmt.Fprintln(w, "the CLI errors with the list of available pipelines.") + _, _ = fmt.Fprintln(w, "") _, _ = fmt.Fprintln(w, "Environment:") _, _ = fmt.Fprintln(w, " TOPLINE_QUERY_TOKEN Connection-bound token from https://os-mcp.topline.com/connect") _, _ = fmt.Fprintln(w, " TOPLINE_QUERY_BASE_URL Defaults to https://os-mcp.topline.com") @@ -260,10 +266,15 @@ func runQueryFreshness(ctx context.Context, client *topline.QueryClient, stdout } func runQuerySnapshot(ctx context.Context, client *topline.QueryClient, flags map[string]string, stdout io.Writer, globals globalOptions) error { - pipelineID := strings.TrimSpace(firstNonEmptyQueryFlag(flags["pipeline"], flags["pipelineId"], flags["pipelineID"])) - if pipelineID == "" { - return errors.New("usage: topline query snapshot --pipeline ") + rawPipeline := strings.TrimSpace(firstNonEmptyQueryFlag(flags["pipeline"], flags["pipelineId"], flags["pipelineID"])) + if rawPipeline == "" { + return errors.New("usage: topline query snapshot --pipeline ") + } + resolution, err := resolvePipelineID(ctx, client, rawPipeline) + if err != nil { + return err } + pipelineID := resolution.MatchedID status := strings.TrimSpace(flags["status"]) if status == "" { status = "open" @@ -274,18 +285,29 @@ func runQuerySnapshot(ctx context.Context, client *topline.QueryClient, flags ma "FROM pipeline_snapshot WHERE pipeline_id = %s%s ORDER BY stage_position", sqlString(pipelineID), queryStatusClause("opportunity_status", status), ) - result, err := executeSQL(ctx, client, sqlText) + rows, err := executeSQL(ctx, client, sqlText) if err != nil { return err } + result := map[string]any{ + "pipelineId": pipelineID, + "pipelineResolution": resolution, + "status": status, + "snapshot": rows, + } return output.WriteJSON(stdout, result, globals.MaskPII) } func runQueryAudit(ctx context.Context, client *topline.QueryClient, flags map[string]string, stdout io.Writer, globals globalOptions) error { - pipelineID := strings.TrimSpace(firstNonEmptyQueryFlag(flags["pipeline"], flags["pipelineId"], flags["pipelineID"])) - if pipelineID == "" { - return errors.New("usage: topline query audit --pipeline [--since this-week-et] [--status open]") + rawPipeline := strings.TrimSpace(firstNonEmptyQueryFlag(flags["pipeline"], flags["pipelineId"], flags["pipelineID"])) + if rawPipeline == "" { + return errors.New("usage: topline query audit --pipeline [--since this-week-et] [--status open]") + } + resolution, err := resolvePipelineID(ctx, client, rawPipeline) + if err != nil { + return err } + pipelineID := resolution.MatchedID start, err := parseAuditTime(flags["since"], time.Now().AddDate(0, 0, -7)) if err != nil { return err @@ -365,18 +387,157 @@ func runQueryAudit(ctx context.Context, client *topline.QueryClient, flags map[s } result := map[string]any{ - "pipelineId": pipelineID, - "window": map[string]string{"since": since, "until": until}, - "status": status, - "freshness": freshness, - "snapshot": snapshot, - "activity": activity, - "deals": deals, - "movement": movement, + "pipelineId": pipelineID, + "pipelineResolution": resolution, + "window": map[string]string{"since": since, "until": until}, + "status": status, + "freshness": freshness, + "snapshot": snapshot, + "activity": activity, + "deals": deals, + "movement": movement, } return output.WriteJSON(stdout, result, globals.MaskPII) } +// pipelineIDPattern matches an opaque 20-char alphanumeric warehouse pipeline ID +// (e.g. "CLUy1QapsrEeBiNrmQiL", "bna6e9DoPgRchNsjeYS3"). Names are typically +// multi-word with spaces/punctuation, so this regex distinguishes the two +// without an extra HTTP round-trip. +var pipelineIDPattern = regexp.MustCompile(`^[A-Za-z0-9]{20}$`) + +// PipelineResolution surfaces how `--pipeline` was interpreted so the agent +// can see whether it passed an opaque ID directly or matched a name. +type PipelineResolution struct { + Input string `json:"input"` + MatchedID string `json:"matchedId"` + MatchedName string `json:"matchedName,omitempty"` + MatchedBy string `json:"matchedBy"` // "id" | "name" + CandidateCnt int `json:"candidateCount,omitempty"` +} + +// resolvePipelineID converts the --pipeline flag value into an opaque pipeline +// ID. If the input already matches the opaque-ID shape it is returned as-is; +// otherwise a case-insensitive LIKE lookup against the `pipelines` table is +// run. On 0 or >1 matches the error message lists candidate pipelines so the +// caller (agent or human) can disambiguate without dropping to raw SQL. +func resolvePipelineID(ctx context.Context, client *topline.QueryClient, input string) (PipelineResolution, error) { + input = strings.TrimSpace(input) + if input == "" { + return PipelineResolution{}, errors.New("--pipeline is required (id or name)") + } + if pipelineIDPattern.MatchString(input) { + return PipelineResolution{Input: input, MatchedID: input, MatchedBy: "id"}, nil + } + tokens := strings.Fields(strings.ToLower(input)) + if len(tokens) == 0 { + return PipelineResolution{}, errors.New("--pipeline value is empty after whitespace trim") + } + clauses := make([]string, len(tokens)) + for i, tok := range tokens { + clauses[i] = fmt.Sprintf("LOWER(name) LIKE %s", sqlString("%"+tok+"%")) + } + matchSQL := fmt.Sprintf( + "SELECT id, name FROM pipelines WHERE %s ORDER BY name", + strings.Join(clauses, " AND "), + ) + matches, err := selectIDNamePairs(ctx, client, matchSQL) + if err != nil { + return PipelineResolution{}, fmt.Errorf("pipeline name resolution failed for %q: %w", input, err) + } + switch len(matches) { + case 1: + return PipelineResolution{ + Input: input, + MatchedID: matches[0].ID, + MatchedName: matches[0].Name, + MatchedBy: "name", + }, nil + case 0: + all, listErr := selectIDNamePairs(ctx, client, "SELECT id, name FROM pipelines ORDER BY name") + if listErr != nil { + return PipelineResolution{}, fmt.Errorf("no pipeline matched %q (and listing failed: %v)", input, listErr) + } + return PipelineResolution{}, fmt.Errorf( + "no pipeline matched %q. Available pipelines: %s", + input, formatPipelineList(all), + ) + default: + return PipelineResolution{}, fmt.Errorf( + "pipeline %q is ambiguous (%d matches). Pass the opaque id or a more specific name. Candidates: %s", + input, len(matches), formatPipelineList(matches), + ) + } +} + +type idNamePair struct { + ID string + Name string +} + +func selectIDNamePairs(ctx context.Context, client *topline.QueryClient, sql string) ([]idNamePair, error) { + raw, err := executeSQL(ctx, client, sql) + if err != nil { + return nil, err + } + columns := stringSliceFromAny(raw["columns"]) + idIdx, nameIdx := -1, -1 + for i, col := range columns { + switch strings.ToLower(col) { + case "id": + idIdx = i + case "name": + nameIdx = i + } + } + if idIdx == -1 || nameIdx == -1 { + return nil, fmt.Errorf("pipeline lookup response missing id/name columns (got %v)", columns) + } + rowsAny, _ := raw["rows"].([]any) + out := make([]idNamePair, 0, len(rowsAny)) + for _, r := range rowsAny { + row, ok := r.([]any) + if !ok || len(row) <= idIdx || len(row) <= nameIdx { + continue + } + id, _ := row[idIdx].(string) + name, _ := row[nameIdx].(string) + id = strings.TrimSpace(id) + name = strings.TrimSpace(name) + if id == "" { + continue + } + out = append(out, idNamePair{ID: id, Name: name}) + } + sort.SliceStable(out, func(i, j int) bool { return strings.ToLower(out[i].Name) < strings.ToLower(out[j].Name) }) + return out, nil +} + +func stringSliceFromAny(v any) []string { + raw, ok := v.([]any) + if !ok { + return nil + } + out := make([]string, 0, len(raw)) + for _, entry := range raw { + if s, ok := entry.(string); ok { + out = append(out, s) + } + } + return out +} + +func formatPipelineList(pairs []idNamePair) string { + if len(pairs) == 0 { + return "(none found)" + } + parts := make([]string, len(pairs)) + for i, p := range pairs { + parts[i] = fmt.Sprintf("%s (%s)", p.Name, p.ID) + } + return strings.Join(parts, ", ") +} + // sqlString quotes a value as a SQLite single-quoted literal. Used because the // query API doesn't yet accept bind parameters — pipeline IDs are caller-controlled // CRM identifiers, not user input, but we still escape embedded single-quotes. diff --git a/internal/commands/query_test.go b/internal/commands/query_test.go index 9641dde..8ceab77 100644 --- a/internal/commands/query_test.go +++ b/internal/commands/query_test.go @@ -3,6 +3,7 @@ package commands import ( "bytes" "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -373,3 +374,216 @@ func TestQueryAuditRequiresPipeline(t *testing.T) { t.Fatalf("expected --pipeline usage error, got %v", err) } } + +// extractLikeNeedles pulls every lowercased LIKE '%needle%' literal out of sql. +// Used by the fixture server below to mimic SQLite's LIKE behaviour without +// pulling in a real database for tests. +func extractLikeNeedles(sql string) []string { + out := []string{} + rest := sql + for { + idx := strings.Index(rest, "LIKE '%") + if idx < 0 { + return out + } + rest = rest[idx+len("LIKE '%"):] + end := strings.Index(rest, "%'") + if end < 0 { + return out + } + out = append(out, strings.ToLower(rest[:end])) + rest = rest[end+2:] + } +} + +// pipelineLookupServer answers pipelines-resolution SELECTs from a fixture and +// records every SQL call so tests can assert ordering and content. +type pipelineLookupServer struct { + t *testing.T + pipelines []idNamePair // fixture pipelines table + sqls []string +} + +func (s *pipelineLookupServer) handle(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/query/api/execute-sql" { + s.t.Fatalf("path = %q", r.URL.Path) + } + var body struct { + SQL string `json:"sql"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + s.sqls = append(s.sqls, body.SQL) + + w.Header().Set("Content-Type", "application/json") + sql := body.SQL + if strings.Contains(sql, "FROM pipelines") { + // Parse out every LIKE '%token%' fragment and require all to match + // (case-insensitively), mirroring SQLite's behaviour for our resolver SQL. + needles := extractLikeNeedles(sql) + matches := make([]idNamePair, 0, len(s.pipelines)) + for _, p := range s.pipelines { + lname := strings.ToLower(p.Name) + ok := true + for _, n := range needles { + if !strings.Contains(lname, n) { + ok = false + break + } + } + if ok { + matches = append(matches, p) + } + } + rows := make([]string, 0, len(matches)) + for _, p := range matches { + rows = append(rows, fmt.Sprintf(`["%s","%s"]`, p.ID, p.Name)) + } + _, _ = fmt.Fprintf(w, `{"columns":["id","name"],"rows":[%s]}`, strings.Join(rows, ",")) + return + } + _, _ = w.Write([]byte(`{"columns":[],"rows":[]}`)) +} + +func TestQueryAuditResolvesPipelineByName(t *testing.T) { + t.Setenv("TOPLINE_QUERY_TOKEN", "signed-query-token") + + srv := &pipelineLookupServer{ + t: t, + pipelines: []idNamePair{ + {ID: "bna6e9DoPgRchNsjeYS3", Name: "Sales - Flex - Triage"}, + {ID: "CLUy1QapsrEeBiNrmQiL", Name: "Sales - Flex - Qualified"}, + }, + } + server := httptest.NewServer(http.HandlerFunc(srv.handle)) + defer server.Close() + t.Setenv("TOPLINE_QUERY_BASE_URL", server.URL) + + var stdout bytes.Buffer + err := Execute( + []string{"query", "audit", "--pipeline", "flex triage", "--since", "2026-05-11", "--until", "2026-05-13"}, + &stdout, io.Discard, + ) + if err != nil { + t.Fatalf("Execute returned error: %v", err) + } + if len(srv.sqls) != 6 { + t.Fatalf("expected 6 SQL calls (1 resolver + 5 audit), got %d:\n%s", len(srv.sqls), strings.Join(srv.sqls, "\n---\n")) + } + if !strings.Contains(srv.sqls[0], "FROM pipelines") { + t.Fatalf("first SQL should resolve pipeline name; got %q", srv.sqls[0]) + } + // Every pipeline-scoped audit SQL should bind the resolved ID. The freshness + // query is global (no pipeline filter), so skip it. + pipelineScoped := 0 + for _, q := range srv.sqls[1:] { + if strings.Contains(q, "FROM warehouse_freshness") { + continue + } + pipelineScoped++ + if !strings.Contains(q, "'bna6e9DoPgRchNsjeYS3'") { + t.Fatalf("audit SQL should bind resolved ID; got %q", q) + } + } + if pipelineScoped != 4 { + t.Fatalf("expected 4 pipeline-scoped audit SQLs (snapshot/activity/deals/movement), got %d", pipelineScoped) + } + out := map[string]any{} + if err := json.Unmarshal(stdout.Bytes(), &out); err != nil { + t.Fatalf("decode audit output: %v", err) + } + resolution, ok := out["pipelineResolution"].(map[string]any) + if !ok { + t.Fatalf("audit output missing pipelineResolution: %s", stdout.String()) + } + if got, _ := resolution["matchedBy"].(string); got != "name" { + t.Fatalf("pipelineResolution.matchedBy = %q, want \"name\"", got) + } + if got, _ := resolution["matchedId"].(string); got != "bna6e9DoPgRchNsjeYS3" { + t.Fatalf("pipelineResolution.matchedId = %q", got) + } + if got, _ := resolution["matchedName"].(string); got != "Sales - Flex - Triage" { + t.Fatalf("pipelineResolution.matchedName = %q", got) + } + if got, _ := out["pipelineId"].(string); got != "bna6e9DoPgRchNsjeYS3" { + t.Fatalf("audit pipelineId = %q, want resolved id", got) + } +} + +func TestQueryAuditUnknownPipelineListsCandidates(t *testing.T) { + t.Setenv("TOPLINE_QUERY_TOKEN", "signed-query-token") + + srv := &pipelineLookupServer{ + t: t, + pipelines: []idNamePair{ + {ID: "bna6e9DoPgRchNsjeYS3", Name: "Sales - Flex - Triage"}, + {ID: "CLUy1QapsrEeBiNrmQiL", Name: "Sales - Flex - Qualified"}, + }, + } + server := httptest.NewServer(http.HandlerFunc(srv.handle)) + defer server.Close() + t.Setenv("TOPLINE_QUERY_BASE_URL", server.URL) + + var stdout bytes.Buffer + err := Execute([]string{"query", "audit", "--pipeline", "nonexistent"}, &stdout, io.Discard) + if err == nil { + t.Fatalf("expected error for unknown pipeline name, got nil; stdout=%s", stdout.String()) + } + if !strings.Contains(err.Error(), "no pipeline matched") { + t.Fatalf("error should mention no match; got %v", err) + } + if !strings.Contains(err.Error(), "Sales - Flex - Triage") || !strings.Contains(err.Error(), "Sales - Flex - Qualified") { + t.Fatalf("error should list available pipelines; got %v", err) + } +} + +func TestQueryAuditAmbiguousPipelineErrors(t *testing.T) { + t.Setenv("TOPLINE_QUERY_TOKEN", "signed-query-token") + + srv := &pipelineLookupServer{ + t: t, + pipelines: []idNamePair{ + {ID: "bna6e9DoPgRchNsjeYS3", Name: "Sales - Flex - Triage"}, + {ID: "CLUy1QapsrEeBiNrmQiL", Name: "Sales - Flex - Qualified"}, + }, + } + server := httptest.NewServer(http.HandlerFunc(srv.handle)) + defer server.Close() + t.Setenv("TOPLINE_QUERY_BASE_URL", server.URL) + + var stdout bytes.Buffer + err := Execute([]string{"query", "audit", "--pipeline", "flex"}, &stdout, io.Discard) + if err == nil { + t.Fatalf("expected ambiguity error, got nil; stdout=%s", stdout.String()) + } + if !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("error should call out ambiguity; got %v", err) + } + if !strings.Contains(err.Error(), "Sales - Flex - Triage") || !strings.Contains(err.Error(), "Sales - Flex - Qualified") { + t.Fatalf("error should list both candidates; got %v", err) + } +} + +func TestQuerySnapshotResolvesPipelineByName(t *testing.T) { + t.Setenv("TOPLINE_QUERY_TOKEN", "signed-query-token") + + srv := &pipelineLookupServer{ + t: t, + pipelines: []idNamePair{ + {ID: "bna6e9DoPgRchNsjeYS3", Name: "Sales - Flex - Triage"}, + }, + } + server := httptest.NewServer(http.HandlerFunc(srv.handle)) + defer server.Close() + t.Setenv("TOPLINE_QUERY_BASE_URL", server.URL) + + var stdout bytes.Buffer + if err := Execute([]string{"query", "snapshot", "--pipeline", "triage"}, &stdout, io.Discard); err != nil { + t.Fatalf("Execute returned error: %v", err) + } + if len(srv.sqls) != 2 { + t.Fatalf("expected 2 SQL calls (resolver + snapshot), got %d", len(srv.sqls)) + } + if !strings.Contains(srv.sqls[1], "'bna6e9DoPgRchNsjeYS3'") { + t.Fatalf("snapshot SQL should bind resolved ID; got %q", srv.sqls[1]) + } +} diff --git a/skills/claude-code/SKILL.md b/skills/claude-code/SKILL.md index 3a96bd6..14d7ca5 100644 --- a/skills/claude-code/SKILL.md +++ b/skills/claude-code/SKILL.md @@ -1,7 +1,7 @@ --- name: topline-os-cli description: Use the Topline OS CLI for SQL-first CRM analytics, pipeline audits, deal briefs, and agent-safe sales operations. Default to the composite `topline --agent query audit|snapshot|freshness` commands for standard analytics; use REST-backed commands for live drilldowns and approved writes. Triggers on Topline OS, CRM pipeline, opportunity, deal, sales activity, and `topline` CLI questions. -version: 1.6.1 +version: 1.6.2 --- # Topline OS CLI Skill @@ -13,12 +13,13 @@ Use the `topline` CLI for Topline OS CRM workflows. For broad CRM analytics, **p For any "what happened in pipeline X over window W" question, run **exactly** this sequence and stop: 1. `topline --agent query doctor` — readiness probe. JSON: `queryTokenPresent`, `schemaReachable`, `tableCount`, `missingTables`, `recommendation`. If `queryTokenPresent` is false, `schemaReachable` is false, or any expected table is missing, stop and report the readiness gap. Missing tables/views are `os-mcp` coverage bugs; surface them in the final answer. -2. `topline --agent query audit --pipeline PIPELINE_ID --since WINDOW --status open` — one composite call returning `freshness`, `snapshot`, `activity` (with `unique_touches`), `deals`, and `movement`. Default `--since this-week-et` for "this week"; the CLI resolves the window. +2. `topline --agent query audit --pipeline PIPELINE_ID_OR_NAME --since WINDOW --status open` — one composite call returning `freshness`, `snapshot`, `activity` (with `unique_touches`), `deals`, `movement`, and `pipelineResolution`. `--pipeline` accepts an opaque 20-char ID **or** a fuzzy name (e.g. `'flex triage'`); on unknown/ambiguous names the CLI errors with the list of available pipelines. Default `--since this-week-et` for "this week"; the CLI resolves the window. 3. Answer. **Hard ceiling: 3 tool calls after skills load** (doctor + audit + answer). Banned in the default flow: - Raw `topline --agent query sql --sql ...` for standard pipeline audits — `query audit` already covers the shape. +- Raw `query sql` to resolve a pipeline name to its opaque ID. `query audit` / `query snapshot` accept names directly (v1.6.2+) and emit `pipelineResolution`. - `query schema`, `query explain`, or per-table freshness SQL before `query audit`. The audit payload's `freshness` field is enough. - `pipeline audit`, `opportunities search`, `conversations search`, per-conversation message fetches. - `python3` / `execute_code` / any subprocess wrapper around `topline`. The CLI returns JSON; parse it directly. @@ -104,3 +105,4 @@ Prefer `--agent` for token-efficient, PII-masked output. Never print PIT or quer - **Treating current pipeline as historical origin.** The `opportunities` warehouse table exposes current pipeline/stage state; a won Qualified opportunity is not proof the deal originated in Triage. For Flex conversion questions, answer current-state first (e.g. current pipeline = `Sales - Flex - Qualified`, status = won, created/closed in window), then add a lineage confidence caveat unless a history/audit table or activity event records the move. - **Counting automated workflow touches as rep effort.** For manual outreach audits, exclude workflow/app automation — in the hosted warehouse, `raw_payload.source = 'app'` on `messages` is the automation exclusion signal. Break out calls/email/SMS separately and report contact counts. - **Mislabeling SQL/native disagreements as "sync lag".** Only call it lag when `_synced_at` proves lag. Missing UNION branches or coverage gaps are `os-mcp` bugs, not lag — disclose and stop. +- **Resolving pipeline names to IDs via raw SQL between `query audit` calls.** Pass the name straight to `--pipeline`; the CLI handles the lookup and emits `pipelineResolution` showing `matchedId` / `matchedName`. On 0 or >1 matches the CLI errors with the available pipelines list — don't paper over it with raw SQL. diff --git a/skills/hermes/SKILL.md b/skills/hermes/SKILL.md index 21fd983..c7a5afb 100644 --- a/skills/hermes/SKILL.md +++ b/skills/hermes/SKILL.md @@ -1,7 +1,7 @@ --- name: topline-os-cli description: Use the Topline OS CLI for SQL-first CRM analytics, pipeline audits, token-efficient reads, deal briefs, and agent-safe sales operations. Default to the composite `topline --agent query audit|snapshot|freshness` commands for standard analytics; use REST-backed commands for live drilldowns and approved writes. -version: 1.6.1 +version: 1.6.2 --- # Topline OS CLI @@ -35,12 +35,13 @@ date '+%Y-%m-%d %H:%M:%S %Z (%z); %A; ISO week %V' For any "what happened in pipeline X over window W" question, run **exactly** this sequence and stop: 1. `topline --agent query doctor` — readiness probe. If `queryTokenPresent` is false, `schemaReachable` is false, or any expected table is missing, stop and report the readiness gap. Missing tables/views are `os-mcp` coverage bugs; surface them in the final answer. -2. `topline --agent query audit --pipeline PIPELINE_ID --since WINDOW --status open` — one composite call returning `freshness`, `snapshot`, `activity` (with `unique_touches`), `deals` (per-deal touch breakdowns), and `movement`. Default `--since this-week-et` for "this week"; the CLI resolves the window so a separate `date` call is not needed for that phrase. +2. `topline --agent query audit --pipeline PIPELINE_ID_OR_NAME --since WINDOW --status open` — one composite call returning `freshness`, `snapshot`, `activity` (with `unique_touches`), `deals` (per-deal touch breakdowns), `movement`, and `pipelineResolution` (echoing how `--pipeline` was matched). `--pipeline` accepts the opaque 20-char ID **or** a fuzzy name (e.g. `'flex triage'`, `'qualified'`). On unknown or ambiguous names the CLI errors with the list of available pipelines — no need to do your own name→ID lookup via raw SQL. Default `--since this-week-et` for "this week"; the CLI resolves the window so a separate `date` call is not needed for that phrase. 3. Answer. **Hard ceiling: 3 tool calls after skills load** (doctor + audit + answer). The following are explicitly banned in the default flow: - Raw `topline --agent query sql --sql ...` for standard pipeline audits — the composite `query audit` already covers the shape. +- **Raw `query sql` to resolve a pipeline name to its opaque ID** (`SELECT id FROM pipelines WHERE name LIKE ...`). `query audit` / `query snapshot` accept names directly and emit `pipelineResolution` so the agent never needs to do the lookup itself. - `query schema`, `query explain`, or per-table freshness SQL before `query audit`. Use `query doctor` for readiness and rely on the audit payload's `freshness` field. - `pipeline audit`, `opportunities search`, `conversations search`, per-conversation message fetches. - `python3` / `execute_code` / any subprocess wrapper around `topline` calls. The CLI already returns JSON; parse it in the answer, not in a Python loop. @@ -172,6 +173,7 @@ topline raw request GET /opportunities/search --query '{"pipelineId":"PIPELINE_I 14. **Doing math on the audit JSON after the fact.** Real failure mode: agent runs `query doctor` + `query audit` cleanly, then opens a `python3 - <<'PY' vals=[...] PY` heredoc (or `jq` / `awk` / bash arithmetic) to compute averages/totals over the audit's `activity.by_stage`, `deals`, or `snapshot` rollups before answering. The audit response already carries those rollups — `activity.total_messages`, `activity.by_stage[*]`, `deals.open_count`, `deals.open_value_total`, `snapshot.avg_days_in_stage`, `movement.advances`/`movement.regresses`/`movement.stalls`. If a question genuinely needs a number not in the payload (e.g. p95 instead of avg), express it as `topline --agent query sql --sql ...` and disclose that it is non-standard analytics. Computing in Python/jq/bash on the audit JSON is the same anti-pattern as wrapping the CLI in Python — it just moves the violation one step past the CLI boundary. 15. **Treating current pipeline as historical origin.** The `opportunities` warehouse table exposes current pipeline/stage state; it does not, by itself, prove that a won Qualified opportunity started in Triage. For Flex conversion questions, answer in two layers: (a) direct/current-state query (current pipeline = `Sales - Flex - Qualified`, status = won, created/closed in window); (b) lineage confidence caveat unless a history/audit table or activity event explicitly records the pipeline move. Do not report "originated in Triage" as proven just because the deal is now in Qualified. See `references/flex-crm-lineage-and-manual-outreach.md`. 16. **Counting automated workflow touches as rep effort.** For manual outreach/activity audits, exclude workflow/app automation. In the hosted warehouse, use message/activity metadata such as `raw_payload.source = 'app'` as the automation exclusion signal when available, then break out calls/email/SMS separately and report contact counts. +17. **Resolving pipeline names to IDs via raw SQL between `query audit` calls.** Real failure mode: agent runs `query audit --pipeline 'Sales - Flex - Triage'`, gets `rows: []` because v1.6.1 passed the literal name string straight into the view, then runs `topline --agent query sql --sql "SELECT id, name FROM pipelines WHERE LOWER(name) LIKE '%flex%'"` to find the opaque ID, then re-runs `query audit` with the resolved ID. As of v1.6.2 the CLI does this resolution for you: pass the name (e.g. `--pipeline 'flex triage'`) and let the resolver match it. On 0 or >1 matches the CLI errors with the list of available pipelines; do not paper over that error with raw SQL — paste the candidate list into the answer or ask the user to disambiguate. ## Reporting rule of thumb