From 81ad68969a3a8c5e638d7c652f5d0b88fe5b9e87 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Fri, 3 Apr 2026 15:30:41 -0700 Subject: [PATCH 1/2] feat: cleanse search response to strip UI-specific fields Strip the SDK's bloated SearchResponse down to only the fields relevant to programmatic consumers, aligning with the RFC for POST /api/search. This is a stopgap until the new API ships. - Add allowlist-based response cleansing (internal/output/cleanse.go) - Filter out empty structured results that have no document or title - Sub-filter author to only name and email - Add --raw flag to bypass cleansing for full SDK response - Warn on stderr when --fields requests a cleansed-away field - Extend WriteNDJSON to handle cleansed map[string]any responses Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/search.go | 30 ++- internal/output/cleanse.go | 185 +++++++++++++++++ internal/output/cleanse_test.go | 345 ++++++++++++++++++++++++++++++++ internal/output/formatter.go | 17 +- 4 files changed, 572 insertions(+), 5 deletions(-) create mode 100644 internal/output/cleanse.go create mode 100644 internal/output/cleanse_test.go diff --git a/cmd/search.go b/cmd/search.go index 323fe2e..0ae45c2 100644 --- a/cmd/search.go +++ b/cmd/search.go @@ -3,6 +3,7 @@ package cmd import ( "encoding/json" "fmt" + "strings" "github.com/gleanwork/api-client-go/models/components" gleanClient "github.com/gleanwork/glean-cli/internal/client" @@ -23,6 +24,7 @@ func NewCmdSearch() *cobra.Command { var outputFormat string var dryRun bool var fields string + var raw bool cmd := &cobra.Command{ Use: "search [query]", @@ -61,7 +63,14 @@ Example: if err != nil { return fmt.Errorf("search request failed: %w", err) } - return output.WriteFormatted(cmd.OutOrStdout(), resp.SearchResponse, outputFormat, nil) + var result any = resp.SearchResponse + if !raw { + result, err = output.CleanseSearchResponse(result) + if err != nil { + return err + } + } + return output.WriteFormatted(cmd.OutOrStdout(), result, outputFormat, nil) } // flag-based path @@ -111,10 +120,24 @@ Example: if err != nil { return err } + var result any = resp + if !raw { + result, err = output.CleanseSearchResponse(result) + if err != nil { + return err + } + } if fields != "" { - return output.ProjectFields(cmd.OutOrStdout(), resp, fields) + if !raw { + if stripped := output.WarnStrippedFields(fields); len(stripped) > 0 { + fmt.Fprintf(cmd.ErrOrStderr(), + "Warning: field(s) %s not available in cleansed output (use --raw for the full response)\n", + strings.Join(stripped, ", ")) + } + } + return output.ProjectFields(cmd.OutOrStdout(), result, fields) } - return output.WriteFormatted(cmd.OutOrStdout(), resp, outputFormat, nil) + return output.WriteFormatted(cmd.OutOrStdout(), result, outputFormat, nil) }, } @@ -122,6 +145,7 @@ Example: cmd.Flags().StringVar(&outputFormat, "output", "json", "Output format: json, ndjson, or text") cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated dot-path fields to include (e.g. results.document.title,results.document.url). Results where all projected fields are missing appear as {}") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Print the request body without sending it") + cmd.Flags().BoolVar(&raw, "raw", false, "Output the full SDK response without cleansing") cmd.Flags().IntVar(&opts.PageSize, "page-size", 10, "Number of results per page") cmd.Flags().IntVar(&opts.MaxSnippetSize, "max-snippet-size", 0, "Maximum size of snippets") cmd.Flags().IntVar(&opts.TimeoutMillis, "timeout", 30000, "Request timeout in milliseconds") diff --git a/internal/output/cleanse.go b/internal/output/cleanse.go new file mode 100644 index 0000000..7ccd5c8 --- /dev/null +++ b/internal/output/cleanse.go @@ -0,0 +1,185 @@ +package output + +import ( + "encoding/json" + "fmt" + "strings" +) + +// CleanseSearchResponse strips UI-specific fields from a search response, +// keeping only the fields relevant to programmatic consumers. +// +// This is a stopgap until POST /api/search ships (see RFC: Search Data +// Retrieval API). Delete this file and its call sites once the new API +// is available. +func CleanseSearchResponse(resp any) (any, error) { + data, err := json.Marshal(resp) + if err != nil { + return nil, fmt.Errorf("cleanse marshal: %w", err) + } + + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + return nil, fmt.Errorf("cleanse unmarshal: %w", err) + } + + result := filterMap(raw, responseAllowlist) + + if results, ok := result["results"].([]any); ok { + result["results"] = filterEmptyResults(results) + } + + return result, nil +} + +type allowlist map[string]allowlist + +var responseAllowlist = allowlist{ + "results": resultAllowlist, + "cursor": nil, + "hasMoreResults": nil, + "requestID": nil, +} + +var resultAllowlist = allowlist{ + "title": nil, + "url": nil, + "snippets": snippetAllowlist, + "document": documentAllowlist, +} + +var documentAllowlist = allowlist{ + "title": nil, + "url": nil, + "datasource": nil, + "docType": nil, + "metadata": metadataAllowlist, +} + +var metadataAllowlist = allowlist{ + "datasource": nil, + "objectType": nil, + "author": personAllowlist, + "updateTime": nil, + "createTime": nil, +} + +var personAllowlist = allowlist{ + "name": nil, + "email": nil, +} + +var snippetAllowlist = allowlist{ + "snippet": nil, + "mimeType": nil, +} + +// WarnStrippedFields checks whether any of the requested --fields paths +// were removed by cleansing. Returns a list of field paths that don't +// exist in the allowlist. +// +// Stopgap — delete with cleanse.go when POST /api/search ships. +func WarnStrippedFields(fields string) []string { + if fields == "" { + return nil + } + var stripped []string + for _, f := range strings.Split(fields, ",") { + f = strings.TrimSpace(f) + if f == "" { + continue + } + if !isAllowedPath(f, responseAllowlist) { + stripped = append(stripped, f) + } + } + return stripped +} + +// isAllowedPath checks whether a dot-separated field path exists in the allowlist tree. +// A path is allowed if every segment resolves in the allowlist. A nil allowlist value +// at any point means "keep everything below here", so all deeper paths are allowed. +func isAllowedPath(path string, al allowlist) bool { + parts := strings.SplitN(path, ".", 2) + key := parts[0] + + childAL, ok := al[key] + if !ok { + return false + } + if len(parts) == 1 { + return true + } + // nil allowlist = keep everything below → any sub-path is valid + if childAL == nil { + return true + } + return isAllowedPath(parts[1], childAL) +} + +// filterMap recursively keeps only keys present in the allowlist. +// If the allowlist value for a key is nil, the entire value is kept as-is. +// If the allowlist value is a nested allowlist, the value is filtered recursively. +func filterMap(m map[string]any, al allowlist) map[string]any { + out := make(map[string]any, len(al)) + for key, childAL := range al { + val, ok := m[key] + if !ok { + continue + } + if childAL == nil { + out[key] = val + continue + } + switch v := val.(type) { + case map[string]any: + out[key] = filterMap(v, childAL) + case []any: + out[key] = filterSlice(v, childAL) + default: + out[key] = val + } + } + return out +} + +// filterEmptyResults removes results that have no meaningful content after cleansing. +// Structured results from the SDK (e.g. knowledge cards) cleanse down to just {"url": ""} +// since they have no document, title, or snippets. +func filterEmptyResults(results []any) []any { + out := make([]any, 0, len(results)) + for _, r := range results { + m, ok := r.(map[string]any) + if !ok { + out = append(out, r) + continue + } + if _, hasDoc := m["document"]; hasDoc { + out = append(out, r) + continue + } + if title, _ := m["title"].(*string); title != nil { + out = append(out, r) + continue + } + if title, ok := m["title"].(string); ok && title != "" { + out = append(out, r) + continue + } + // No document and no title — skip this empty result + } + return out +} + +// filterSlice applies the allowlist to each element in a slice. +func filterSlice(s []any, al allowlist) []any { + out := make([]any, len(s)) + for i, elem := range s { + if m, ok := elem.(map[string]any); ok { + out[i] = filterMap(m, al) + } else { + out[i] = elem + } + } + return out +} diff --git a/internal/output/cleanse_test.go b/internal/output/cleanse_test.go new file mode 100644 index 0000000..2a2af43 --- /dev/null +++ b/internal/output/cleanse_test.go @@ -0,0 +1,345 @@ +package output + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCleanseSearchResponse_StripsUIFields(t *testing.T) { + input := map[string]any{ + "trackingToken": "tok_abc", + "sessionInfo": map[string]any{"sessionId": "sess123"}, + "experimentIds": []any{float64(1), float64(2)}, + "backendTimeMillis": float64(42), + "structuredResults": []any{map[string]any{"foo": "bar"}}, + "generatedQnaResult": map[string]any{ + "question": "what?", + }, + "metadata": map[string]any{"source": "internal"}, + "facetResults": []any{}, + "resultTabs": []any{}, + "resultTabIds": []any{"tab1"}, + "resultsDescription": map[string]any{"text": "showing results"}, + "rewrittenFacetFilters": []any{}, + "requestID": "req_123", + "cursor": "eyJwIjo0", + "hasMoreResults": true, + "results": []any{ + map[string]any{ + "trackingToken": "result_tok", + "structuredResults": []any{}, + "clusteredResults": []any{}, + "backlinkResults": []any{}, + "prominence": "HIGH", + "querySuggestion": map[string]any{}, + "clusterType": "THREAD", + "attachmentCount": float64(3), + "attachments": []any{}, + "pins": []any{}, + "nativeAppUrl": "slack://foo", + "fullText": "some full text", + "fullTextList": []any{"line1"}, + "relatedResults": []any{}, + "allClusteredResults": []any{}, + "title": "Q2 Planning", + "url": "https://example.com/doc", + "snippets": []any{ + map[string]any{ + "snippet": "The platform team will focus on...", + "mimeType": "text/plain", + "ranges": []any{map[string]any{"start": float64(0), "end": float64(5)}}, + }, + }, + "document": map[string]any{ + "id": "doc_123", + "title": "Q2 Planning Doc", + "url": "https://example.com/doc", + "datasource": "confluence", + "docType": "page", + "connectorType": "API_CRAWL", + "content": map[string]any{"fullTextList": []any{"text"}}, + "containerDocument": map[string]any{"id": "parent"}, + "parentDocument": map[string]any{"id": "parent2"}, + "sections": []any{map[string]any{"title": "Intro"}}, + "metadata": map[string]any{ + "datasource": "confluence", + "objectType": "page", + "author": map[string]any{ + "name": "Steve", + "email": "steve@co.com", + "obfuscatedId": "ABC123", + "metadata": map[string]any{"loggingId": "XYZ"}, + }, + "updateTime": "2026-03-28T14:30:00Z", + "createTime": "2026-01-15T09:00:00Z", + "container": "Engineering Space", + "interactions": map[string]any{"numViews": float64(42)}, + "documentCategory": "PUBLISHED_CONTENT", + "pins": []any{}, + "collections": []any{}, + }, + }, + }, + }, + } + + result, err := CleanseSearchResponse(input) + require.NoError(t, err) + + m, ok := result.(map[string]any) + require.True(t, ok) + + // Top-level: only allowed keys + assert.Contains(t, m, "results") + assert.Contains(t, m, "cursor") + assert.Contains(t, m, "hasMoreResults") + assert.Contains(t, m, "requestID") + assert.NotContains(t, m, "trackingToken") + assert.NotContains(t, m, "sessionInfo") + assert.NotContains(t, m, "experimentIds") + assert.NotContains(t, m, "backendTimeMillis") + assert.NotContains(t, m, "structuredResults") + assert.NotContains(t, m, "generatedQnaResult") + assert.NotContains(t, m, "metadata") + assert.NotContains(t, m, "facetResults") + assert.NotContains(t, m, "resultTabs") + assert.NotContains(t, m, "resultTabIds") + assert.NotContains(t, m, "resultsDescription") + assert.NotContains(t, m, "rewrittenFacetFilters") + + // Results array + results, ok := m["results"].([]any) + require.True(t, ok) + require.Len(t, results, 1) + + r := results[0].(map[string]any) + assert.Contains(t, r, "title") + assert.Contains(t, r, "url") + assert.Contains(t, r, "snippets") + assert.Contains(t, r, "document") + assert.NotContains(t, r, "trackingToken") + assert.NotContains(t, r, "structuredResults") + assert.NotContains(t, r, "clusteredResults") + assert.NotContains(t, r, "backlinkResults") + assert.NotContains(t, r, "prominence") + assert.NotContains(t, r, "pins") + assert.NotContains(t, r, "nativeAppUrl") + assert.NotContains(t, r, "fullText") + assert.NotContains(t, r, "attachments") + assert.NotContains(t, r, "clusterType") + + // Snippets: only snippet and mimeType + snippets := r["snippets"].([]any) + require.Len(t, snippets, 1) + snip := snippets[0].(map[string]any) + assert.Contains(t, snip, "snippet") + assert.Contains(t, snip, "mimeType") + assert.NotContains(t, snip, "ranges") + + // Document: only allowed keys + doc := r["document"].(map[string]any) + assert.Contains(t, doc, "title") + assert.Contains(t, doc, "url") + assert.Contains(t, doc, "datasource") + assert.Contains(t, doc, "docType") + assert.Contains(t, doc, "metadata") + assert.NotContains(t, doc, "id") + assert.NotContains(t, doc, "connectorType") + assert.NotContains(t, doc, "content") + assert.NotContains(t, doc, "containerDocument") + assert.NotContains(t, doc, "parentDocument") + assert.NotContains(t, doc, "sections") + + // Document metadata: only allowed keys + meta := doc["metadata"].(map[string]any) + assert.Contains(t, meta, "datasource") + assert.Contains(t, meta, "objectType") + assert.Contains(t, meta, "author") + assert.Contains(t, meta, "updateTime") + assert.Contains(t, meta, "createTime") + assert.NotContains(t, meta, "container") + assert.NotContains(t, meta, "interactions") + assert.NotContains(t, meta, "documentCategory") + assert.NotContains(t, meta, "pins") + assert.NotContains(t, meta, "collections") + + // Author filtered to name and email only + author := meta["author"].(map[string]any) + assert.Equal(t, "Steve", author["name"]) + assert.Equal(t, "steve@co.com", author["email"]) + assert.NotContains(t, author, "metadata") + assert.NotContains(t, author, "obfuscatedId") +} + +func TestCleanseSearchResponse_EmptyResults(t *testing.T) { + input := map[string]any{ + "results": []any{}, + "hasMoreResults": false, + "requestID": "req_456", + } + + result, err := CleanseSearchResponse(input) + require.NoError(t, err) + + m := result.(map[string]any) + assert.Equal(t, []any{}, m["results"]) + assert.Equal(t, false, m["hasMoreResults"]) + assert.Equal(t, "req_456", m["requestID"]) +} + +func TestCleanseSearchResponse_FiltersEmptyResults(t *testing.T) { + input := map[string]any{ + "results": []any{ + // Structured result with no document or title — should be removed + map[string]any{ + "url": "", + "structuredResults": []any{map[string]any{"foo": "bar"}}, + "trackingToken": "tok", + }, + // Real result with document — should be kept + map[string]any{ + "url": "https://example.com/doc", + "title": "Real Doc", + "document": map[string]any{ + "title": "Real Doc", + "datasource": "confluence", + }, + }, + // Result with title but no document — should be kept + map[string]any{ + "url": "https://example.com/other", + "title": "Has Title", + }, + // Empty result, url present but no title/document — should be removed + map[string]any{ + "url": "https://example.com/empty", + }, + }, + "requestID": "req_filter", + } + + result, err := CleanseSearchResponse(input) + require.NoError(t, err) + + m := result.(map[string]any) + results := m["results"].([]any) + require.Len(t, results, 2, "should keep only results with document or non-empty title") + + r0 := results[0].(map[string]any) + assert.Equal(t, "Real Doc", r0["title"]) + + r1 := results[1].(map[string]any) + assert.Equal(t, "Has Title", r1["title"]) +} + +func TestCleanseSearchResponse_MissingOptionalFields(t *testing.T) { + input := map[string]any{ + "results": []any{ + map[string]any{ + "url": "https://example.com", + "title": "Test", + }, + }, + } + + result, err := CleanseSearchResponse(input) + require.NoError(t, err) + + m := result.(map[string]any) + results := m["results"].([]any) + r := results[0].(map[string]any) + assert.Equal(t, "Test", r["title"]) + assert.Equal(t, "https://example.com", r["url"]) + assert.NotContains(t, r, "document") + assert.NotContains(t, r, "snippets") +} + +func TestCleanseSearchResponse_SDKStruct(t *testing.T) { + // Simulate passing an SDK struct (not a map) — verifies the marshal round-trip works. + type fakeResult struct { + Title string `json:"title"` + URL string `json:"url"` + TrackingToken string `json:"trackingToken"` + } + type fakeResponse struct { + Results []fakeResult `json:"results"` + TrackingToken string `json:"trackingToken"` + RequestID string `json:"requestID"` + } + + resp := fakeResponse{ + Results: []fakeResult{ + {Title: "Doc", URL: "https://x.com", TrackingToken: "tok"}, + }, + TrackingToken: "resp_tok", + RequestID: "req_789", + } + + result, err := CleanseSearchResponse(resp) + require.NoError(t, err) + + // Should be a map now + m, ok := result.(map[string]any) + require.True(t, ok) + assert.Contains(t, m, "requestID") + assert.NotContains(t, m, "trackingToken") + + results := m["results"].([]any) + r := results[0].(map[string]any) + assert.Equal(t, "Doc", r["title"]) + assert.NotContains(t, r, "trackingToken") +} + +func TestWarnStrippedFields_AllowedFields(t *testing.T) { + stripped := WarnStrippedFields("results.title,results.url,results.document.datasource") + assert.Empty(t, stripped) +} + +func TestWarnStrippedFields_StrippedFields(t *testing.T) { + stripped := WarnStrippedFields("results.trackingToken,results.title,results.structuredResults") + assert.Equal(t, []string{"results.trackingToken", "results.structuredResults"}, stripped) +} + +func TestWarnStrippedFields_DeepAllowedPath(t *testing.T) { + stripped := WarnStrippedFields("results.document.metadata.author.name") + assert.Empty(t, stripped) +} + +func TestWarnStrippedFields_DeepStrippedPath(t *testing.T) { + stripped := WarnStrippedFields("results.document.content") + assert.Equal(t, []string{"results.document.content"}, stripped) +} + +func TestWarnStrippedFields_TopLevelStripped(t *testing.T) { + stripped := WarnStrippedFields("trackingToken,sessionInfo,requestID") + assert.Equal(t, []string{"trackingToken", "sessionInfo"}, stripped) +} + +func TestWarnStrippedFields_Empty(t *testing.T) { + stripped := WarnStrippedFields("") + assert.Nil(t, stripped) +} + +func TestCleanseSearchResponse_RoundTripsToJSON(t *testing.T) { + input := map[string]any{ + "results": []any{ + map[string]any{ + "title": "Hello", + "url": "https://example.com", + }, + }, + "requestID": "req_1", + } + + result, err := CleanseSearchResponse(input) + require.NoError(t, err) + + // Must be serializable + data, err := json.Marshal(result) + require.NoError(t, err) + assert.Contains(t, string(data), `"title":"Hello"`) + assert.Contains(t, string(data), `"requestID":"req_1"`) +} diff --git a/internal/output/formatter.go b/internal/output/formatter.go index 224b867..14c381d 100644 --- a/internal/output/formatter.go +++ b/internal/output/formatter.go @@ -28,10 +28,11 @@ func WriteJSON(w io.Writer, v any) error { } // WriteNDJSON marshals each element of a slice as a separate JSON line to w. -// For SearchResponse, it emits one result per line instead of the full envelope. +// For SearchResponse (SDK struct or cleansed map), it emits one result per line +// instead of the full envelope. // If v is not a slice, it writes the whole value as a single line. func WriteNDJSON(w io.Writer, v any) error { - // For search responses, emit one result per line + // SDK struct: emit one result per line if sr, ok := v.(*components.SearchResponse); ok && sr != nil { for _, result := range sr.Results { if err := json.NewEncoder(w).Encode(result); err != nil { @@ -41,6 +42,18 @@ func WriteNDJSON(w io.Writer, v any) error { return nil } + // Cleansed map: emit one result per line (stopgap — remove when POST /api/search ships) + if m, ok := v.(map[string]any); ok { + if results, ok := m["results"].([]any); ok { + for _, result := range results { + if err := json.NewEncoder(w).Encode(result); err != nil { + return err + } + } + return nil + } + } + rv := reflect.ValueOf(v) if rv.Kind() == reflect.Ptr { if rv.IsNil() { From 5b9a71565d52707ef9d746140c39d7df3581ec89 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Fri, 3 Apr 2026 15:56:11 -0700 Subject: [PATCH 2/2] style: fix gofmt formatting in cleanse files Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/output/cleanse.go | 8 +++--- internal/output/cleanse_test.go | 46 ++++++++++++++++----------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/internal/output/cleanse.go b/internal/output/cleanse.go index 7ccd5c8..44edeb3 100644 --- a/internal/output/cleanse.go +++ b/internal/output/cleanse.go @@ -35,10 +35,10 @@ func CleanseSearchResponse(resp any) (any, error) { type allowlist map[string]allowlist var responseAllowlist = allowlist{ - "results": resultAllowlist, - "cursor": nil, + "results": resultAllowlist, + "cursor": nil, "hasMoreResults": nil, - "requestID": nil, + "requestID": nil, } var resultAllowlist = allowlist{ @@ -70,7 +70,7 @@ var personAllowlist = allowlist{ } var snippetAllowlist = allowlist{ - "snippet": nil, + "snippet": nil, "mimeType": nil, } diff --git a/internal/output/cleanse_test.go b/internal/output/cleanse_test.go index 2a2af43..6c44a56 100644 --- a/internal/output/cleanse_test.go +++ b/internal/output/cleanse_test.go @@ -29,23 +29,23 @@ func TestCleanseSearchResponse_StripsUIFields(t *testing.T) { "hasMoreResults": true, "results": []any{ map[string]any{ - "trackingToken": "result_tok", - "structuredResults": []any{}, - "clusteredResults": []any{}, - "backlinkResults": []any{}, - "prominence": "HIGH", - "querySuggestion": map[string]any{}, - "clusterType": "THREAD", - "attachmentCount": float64(3), - "attachments": []any{}, - "pins": []any{}, - "nativeAppUrl": "slack://foo", - "fullText": "some full text", - "fullTextList": []any{"line1"}, - "relatedResults": []any{}, + "trackingToken": "result_tok", + "structuredResults": []any{}, + "clusteredResults": []any{}, + "backlinkResults": []any{}, + "prominence": "HIGH", + "querySuggestion": map[string]any{}, + "clusterType": "THREAD", + "attachmentCount": float64(3), + "attachments": []any{}, + "pins": []any{}, + "nativeAppUrl": "slack://foo", + "fullText": "some full text", + "fullTextList": []any{"line1"}, + "relatedResults": []any{}, "allClusteredResults": []any{}, - "title": "Q2 Planning", - "url": "https://example.com/doc", + "title": "Q2 Planning", + "url": "https://example.com/doc", "snippets": []any{ map[string]any{ "snippet": "The platform team will focus on...", @@ -65,14 +65,14 @@ func TestCleanseSearchResponse_StripsUIFields(t *testing.T) { "parentDocument": map[string]any{"id": "parent2"}, "sections": []any{map[string]any{"title": "Intro"}}, "metadata": map[string]any{ - "datasource": "confluence", - "objectType": "page", + "datasource": "confluence", + "objectType": "page", "author": map[string]any{ - "name": "Steve", - "email": "steve@co.com", - "obfuscatedId": "ABC123", - "metadata": map[string]any{"loggingId": "XYZ"}, - }, + "name": "Steve", + "email": "steve@co.com", + "obfuscatedId": "ABC123", + "metadata": map[string]any{"loggingId": "XYZ"}, + }, "updateTime": "2026-03-28T14:30:00Z", "createTime": "2026-01-15T09:00:00Z", "container": "Engineering Space",