- {plugin.result}
-
- ) : (
- hi
' }, + }; + expect(resolvePluginBody(plugin)).toEqual({ + body_type: 'html', + body_content: 'hi
', + }); + }); + + test('falls back to the raw result as markdown when no body is present', () => { + // A server predating the response contract only sends result/status. + const plugin: PluginResult = { result: 'plain text summary', status: 'success' }; + expect(resolvePluginBody(plugin)).toEqual({ + body_type: 'markdown', + body_content: 'plain text summary', + }); + }); + + test('falls back to the raw result when body_type is unrecognized', () => { + const plugin = { + result: 'plain text summary', + status: 'success', + body: { body_type: 'plaintext', body_content: 'ignored' }, + } as unknown as PluginResult; + expect(resolvePluginBody(plugin)).toEqual({ + body_type: 'markdown', + body_content: 'plain text summary', + }); + }); + + test('normalizes body_type casing', () => { + const plugin = { + result: '', + status: 'success', + body: { body_type: 'HTML', body_content: 'x' }, + } as unknown as PluginResult; + expect(resolvePluginBody(plugin).body_type).toBe('html'); + }); + + test('keeps an empty body when the plugin only produced annotations', () => { + // An annotations-only plugin's raw result is the contract JSON, which + // must not leak into the rendered body. + const plugin: PluginResult = { + result: '{"body":{"body_type":"markdown","body_content":""},"annotations":[]}', + status: 'success', + body: { body_type: 'markdown', body_content: '' }, + annotations: [{ filename: 'a.py', line: 4, severity: 'warning', content: 'x' }], + }; + expect(resolvePluginBody(plugin).body_content).toBe(''); + }); + + test('handles a pending plugin with no output at all', () => { + const plugin: PluginResult = { result: '', status: 'pending' }; + expect(resolvePluginBody(plugin)).toEqual({ body_type: 'markdown', body_content: '' }); + }); +}); + +describe('sortedAnnotations', () => { + test('groups by filename then orders by line', () => { + const plugin: PluginResult = { + result: '', + status: 'success', + body: { body_type: 'markdown', body_content: '' }, + annotations: [ + { filename: 'b.py', line: 2, severity: 'info', content: 'b2' }, + { filename: 'a.py', line: 75, severity: 'warning', content: 'a75' }, + { filename: 'a.py', line: 7, severity: 'error', content: 'a7' }, + ], + }; + expect(sortedAnnotations(plugin).map(a => `${a.filename}:${a.line}`)).toEqual([ + 'a.py:7', + 'a.py:75', + 'b.py:2', + ]); + }); + + test('returns an empty list when annotations are absent', () => { + expect(sortedAnnotations({ result: 'x', status: 'success' })).toEqual([]); + }); + + test('does not mutate the original array', () => { + const annotations = [ + { filename: 'z.py', line: 1, severity: '', content: 'z' }, + { filename: 'a.py', line: 1, severity: '', content: 'a' }, + ]; + sortedAnnotations({ result: '', status: 'success', annotations }); + expect(annotations[0].filename).toBe('z.py'); + }); +}); + +describe('annotationSeverityVariant', () => { + test('maps known severities', () => { + expect(annotationSeverityVariant('error')).toBe('danger'); + expect(annotationSeverityVariant('CRITICAL')).toBe('danger'); + expect(annotationSeverityVariant('warning')).toBe('warning'); + expect(annotationSeverityVariant('warn')).toBe('warning'); + expect(annotationSeverityVariant('info')).toBe('info'); + expect(annotationSeverityVariant('note')).toBe('info'); + }); + + test('falls back to neutral for free-form severities', () => { + expect(annotationSeverityVariant('nitpick')).toBe('neutral'); + expect(annotationSeverityVariant('')).toBe('neutral'); + }); +}); + +describe('totalAnnotationCount', () => { + test('sums annotations across plugins and ignores plugins without any', () => { + const plugins: Recordhi
"}}` + resp := ParsePluginOutput(raw) + + if resp.Body.BodyType != BodyTypeHTML { + t.Errorf("expected body_type %q (normalized), got %q", BodyTypeHTML, resp.Body.BodyType) + } + if resp.Body.BodyContent != "hi
" { + t.Errorf("unexpected body_content: %q", resp.Body.BodyContent) + } + if len(resp.Annotations) != 0 { + t.Errorf("expected no annotations, got %d", len(resp.Annotations)) + } +} + +func TestParsePluginOutput_LegacyPlainTextFallsBackToMarkdown(t *testing.T) { + raw := "Just some plain text summary.\nWith multiple lines." + resp := ParsePluginOutput(raw) + + if resp.Body.BodyType != BodyTypeMarkdown { + t.Errorf("expected fallback body_type markdown, got %q", resp.Body.BodyType) + } + if resp.Body.BodyContent != raw { + t.Errorf("expected raw output preserved verbatim, got %q", resp.Body.BodyContent) + } + if len(resp.Annotations) != 0 { + t.Errorf("expected no annotations on fallback, got %d", len(resp.Annotations)) + } +} + +func TestParsePluginOutput_JSONWithoutBodyFallsBack(t *testing.T) { + raw := `{"summary": "valid JSON but not the contract"}` + resp := ParsePluginOutput(raw) + + if resp.Body.BodyType != BodyTypeMarkdown || resp.Body.BodyContent != raw { + t.Errorf("expected whole output wrapped as markdown, got %+v", resp.Body) + } +} + +func TestParsePluginOutput_UnknownBodyTypeFallsBack(t *testing.T) { + raw := `{"body": {"body_type": "plaintext", "body_content": "hello"}}` + resp := ParsePluginOutput(raw) + + if resp.Body.BodyType != BodyTypeMarkdown || resp.Body.BodyContent != raw { + t.Errorf("expected whole output wrapped as markdown, got %+v", resp.Body) + } +} + +func TestParsePluginOutput_DropsUnanchoredAnnotations(t *testing.T) { + raw := `{ + "body": {"body_type": "markdown", "body_content": "ok"}, + "annotations": [ + {"filename": "", "line": 10, "content": "no filename"}, + {"filename": "a.go", "line": 0, "content": "no line"}, + {"filename": "a.go", "line": -3, "content": "negative line"}, + {"filename": "keep.go", "line": 1, "content": "kept"} + ] + }` + resp := ParsePluginOutput(raw) + + if len(resp.Annotations) != 1 { + t.Fatalf("expected 1 annotation to survive, got %d: %+v", len(resp.Annotations), resp.Annotations) + } + if resp.Annotations[0].Filename != "keep.go" { + t.Errorf("unexpected surviving annotation: %+v", resp.Annotations[0]) + } +} + +func TestCollectPluginAnnotations_OnlySuccessfulPluginsTagged(t *testing.T) { + db := setupTestDB(t) + config.SetC(config.Config{DB: db}) + + conforming := `{ + "body": {"body_type": "markdown", "body_content": "found issues"}, + "annotations": [ + {"filename": "test.py", "line": 75, "severity": "warning", "content": "this line looks wrong"} + ] + }` + if err := db.UpsertPluginResult("owner", "repo", 1, "b-linter", conforming, "success", "sha1"); err != nil { + t.Fatalf("upsert failed: %v", err) + } + // Legacy plain-text plugin: contributes no annotations. + if err := db.UpsertPluginResult("owner", "repo", 1, "a-summarizer", "plain summary", "success", "sha1"); err != nil { + t.Fatalf("upsert failed: %v", err) + } + // Failed plugin with annotation-shaped output must be excluded. + if err := db.UpsertPluginResult("owner", "repo", 1, "c-broken", conforming, "error", "sha1"); err != nil { + t.Fatalf("upsert failed: %v", err) + } + + annotations, err := collectPluginAnnotations("owner", "repo", 1) + if err != nil { + t.Fatalf("collectPluginAnnotations failed: %v", err) + } + if len(annotations) != 1 { + t.Fatalf("expected 1 annotation, got %d: %+v", len(annotations), annotations) + } + a := annotations[0] + if a.Plugin != "b-linter" { + t.Errorf("expected annotation tagged with plugin name, got %q", a.Plugin) + } + if a.Filename != "test.py" || a.Line != 75 || a.Severity != "warning" { + t.Errorf("unexpected annotation: %+v", a) + } +} + +func TestCollectPluginAnnotations_NoResults(t *testing.T) { + db := setupTestDB(t) + config.SetC(config.Config{DB: db}) + + annotations, err := collectPluginAnnotations("owner", "repo", 99) + if err != nil { + t.Fatalf("collectPluginAnnotations failed: %v", err) + } + if len(annotations) != 0 { + t.Errorf("expected no annotations, got %+v", annotations) + } +} + +func TestParsePluginOutputs_KeepsRawResultAlongsideParsed(t *testing.T) { + db := setupTestDB(t) + raw := `{"body": {"body_type": "html", "body_content": "done"}}` + if err := db.UpsertPluginResult("owner", "repo", 2, "renderer", raw, "success", "sha2"); err != nil { + t.Fatalf("upsert failed: %v", err) + } + results, err := db.GetPluginResults("owner", "repo", 2) + if err != nil { + t.Fatalf("get results failed: %v", err) + } + + outputs := parsePluginOutputs(results) + out, ok := outputs["renderer"] + if !ok { + t.Fatal("expected output for plugin 'renderer'") + } + if out.Result != raw { + t.Errorf("expected raw result preserved for legacy clients, got %q", out.Result) + } + if out.Status != "success" { + t.Errorf("expected status success, got %q", out.Status) + } + if out.Body.BodyType != BodyTypeHTML || out.Body.BodyContent != "done" { + t.Errorf("unexpected parsed body: %+v", out.Body) + } +} diff --git a/server/server.go b/server/server.go index 1ae9cc0..c55eee0 100644 --- a/server/server.go +++ b/server/server.go @@ -3,7 +3,6 @@ package server import ( "context" "crs/config" - "crs/database" "crs/git_tools" "crs/llm" "crs/utils" @@ -148,15 +147,16 @@ type GetPRstructArgs struct { // state. Reply structs embed it so all methods expose the same fields, and // populate() is the single place they get filled in and normalized. type PRPayload struct { - Okay bool `json:"okay"` - Content string `json:"content"` - Metadata *PRMetadata `json:"metadata"` - Diff string `json:"diff"` - Comments []CommentJSON `json:"comments"` - OutdatedComments []CommentJSON `json:"outdated_comments"` - Reviews []ReviewJSON `json:"reviews"` - Commits []CommitJSON `json:"commits"` - Feedback string `json:"feedback"` + Okay bool `json:"okay"` + Content string `json:"content"` + Metadata *PRMetadata `json:"metadata"` + Diff string `json:"diff"` + Comments []CommentJSON `json:"comments"` + OutdatedComments []CommentJSON `json:"outdated_comments"` + Reviews []ReviewJSON `json:"reviews"` + Commits []CommitJSON `json:"commits"` + Feedback string `json:"feedback"` + Annotations []PRAnnotation `json:"annotations"` } // populate fills the payload from fetched PR details, normalizing nil slices @@ -188,6 +188,15 @@ func (p *PRPayload) populate(details *PRDetails, content string, owner, repo str slog.Warn("Error loading feedback for PR reply", "repo", repo, "pr", number, "error", err) } p.Feedback = feedback + + annotations, err := collectPluginAnnotations(owner, repo, number) + if err != nil { + slog.Warn("Error loading plugin annotations for PR reply", "repo", repo, "pr", number, "error", err) + } + if annotations == nil { + annotations = []PRAnnotation{} + } + p.Annotations = annotations } type GetPRReply struct { @@ -1140,7 +1149,7 @@ type GetPluginOutputArgs struct { } type GetPluginOutputReply struct { - Output map[string]database.PluginResult `json:"output"` + Output map[string]PluginOutput `json:"output"` } // GetPluginOutput returns all stored plugin outputs for the given PR @@ -1165,7 +1174,7 @@ func (h *RPCHandler) GetPluginOutput(args *GetPluginOutputArgs, reply *GetPlugin }() } - reply.Output = results + reply.Output = parsePluginOutputs(results) return nil } @@ -1177,9 +1186,9 @@ type RerunPluginsArgs struct { } type RerunPluginsReply struct { - Okay bool `json:"okay"` - Message string `json:"message"` - Output map[string]database.PluginResult `json:"output"` + Okay bool `json:"okay"` + Message string `json:"message"` + Output map[string]PluginOutput `json:"output"` } // RerunPlugins forces reexecution of plugins for a given PR, bypassing SHA cache checks. @@ -1234,7 +1243,7 @@ func (h *RPCHandler) RerunPlugins(args *RerunPluginsArgs, reply *RerunPluginsRep } // Return empty output for now (plugins running async) - reply.Output = make(map[string]database.PluginResult) + reply.Output = make(map[string]PluginOutput) return nil }