Compare commits

..

1 Commits

Author SHA1 Message Date
u80864958
2bde5dff47 feat(pierre): sanity check 2026-02-13 17:27:53 +01:00
5 changed files with 14 additions and 173 deletions

View File

@@ -3,10 +3,8 @@ package main
import ( import (
"context" "context"
"log" "log"
"log/slog"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter" "git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter"
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/gitadapters/bitbucket" "git.schreifuchs.ch/schreifuchs/pierre-bot/internal/gitadapters/bitbucket"
@@ -48,13 +46,6 @@ type ReviewConfig struct {
} }
type Config struct { type Config struct {
LogLevel string `help:"Log verbosity: debug, info, warn, error"`
// Deprecated flags (no prefix). Retained for backward compatibility.
// These will be mapped to the embedded ReviewConfig if provided.
MaxChunkSize int `help:"Deprecated: use --review-max-chunk-size"`
Guidelines []string `help:"Deprecated: use --review-guidelines" sep:","`
DisableComments bool `help:"Deprecated: use --review-disable-comments"`
// Embedding ReviewConfig with a prefix changes flag names to `--review-…`. // Embedding ReviewConfig with a prefix changes flag names to `--review-…`.
// Existing configuration files using the old flag names will need to be updated. // Existing configuration files using the old flag names will need to be updated.
// Consider keeping backwards compatibility if required. // Consider keeping backwards compatibility if required.
@@ -68,7 +59,6 @@ type Config struct {
} }
func main() { func main() {
cfg := &Config{} cfg := &Config{}
home, err := os.UserHomeDir() home, err := os.UserHomeDir()
if err != nil { if err != nil {
@@ -85,32 +75,6 @@ func main() {
kong.Configuration(kongyaml.Loader, "config.yaml", defaultConfig), kong.Configuration(kongyaml.Loader, "config.yaml", defaultConfig),
) )
// Configure global slog logger based on the --log-level flag
lvl := slog.LevelInfo
switch strings.ToLower(cfg.LogLevel) {
case "debug":
lvl = slog.LevelDebug
case "info":
lvl = slog.LevelInfo
case "warn":
lvl = slog.LevelWarn
case "error":
lvl = slog.LevelError
}
logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: lvl}))
slog.SetDefault(logger)
// Backwards compatibility: map deprecated flag values (if any) to the embedded ReviewConfig.
if cfg.MaxChunkSize != 0 {
cfg.Review.MaxChunkSize = cfg.MaxChunkSize
}
if len(cfg.Guidelines) > 0 {
cfg.Review.Guidelines = cfg.Guidelines
}
if cfg.DisableComments {
cfg.Review.DisableComments = cfg.DisableComments
}
// Auto-detect provider // Auto-detect provider
provider := cfg.GitProvider provider := cfg.GitProvider
if provider == "" { if provider == "" {

View File

@@ -1,80 +0,0 @@
package bitbucket
import (
"context"
"net/http"
"net/http/httptest"
"testing"
)
func TestBitbucketGetFileContentSuccess(t *testing.T) {
const expected = "file content"
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Verify path structure
if r.URL.Path != "/rest/api/1.0/projects/owner/repos/repo/raw/path/to/file.go" {
t.Fatalf("unexpected URL path: %s", r.URL.Path)
}
w.WriteHeader(http.StatusOK)
w.Write([]byte(expected))
}))
defer server.Close()
// Trim trailing slash handling done in NewBitbucket
adapter := NewBitbucket(server.URL, "")
content, err := adapter.GetFileContent(context.Background(), "owner", "repo", "path/to/file.go", "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if content != expected {
t.Fatalf("expected %q, got %q", expected, content)
}
}
func TestBitbucketGetFileContentError(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
w.Write([]byte("not found"))
}))
defer server.Close()
adapter := NewBitbucket(server.URL, "")
_, err := adapter.GetFileContent(context.Background(), "owner", "repo", "missing.go", "")
if err == nil {
t.Fatalf("expected error for non200 response")
}
}
func TestBitbucketGetPRHeadSHASuccess(t *testing.T) {
const sha = "deadbeef"
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/rest/api/1.0/projects/owner/repos/repo/pull-requests/42" {
t.Fatalf("unexpected URL: %s", r.URL.Path)
}
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"toRef":{"latestCommit":"` + sha + `"}}`))
}))
defer server.Close()
adapter := NewBitbucket(server.URL, "")
got, err := adapter.GetPRHeadSHA(context.Background(), "owner", "repo", 42)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got != sha {
t.Fatalf("expected sha %s, got %s", sha, got)
}
}
func TestBitbucketGetPRHeadSHAError(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte("error"))
}))
defer server.Close()
adapter := NewBitbucket(server.URL, "")
_, err := adapter.GetPRHeadSHA(context.Background(), "owner", "repo", 1)
if err == nil {
t.Fatalf("expected error for non200 response")
}
}

View File

@@ -5,7 +5,6 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
"log/slog"
"net/http" "net/http"
"strconv" "strconv"
"strings" "strings"
@@ -14,7 +13,6 @@ import (
) )
func (b *BitbucketAdapter) GetDiff(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (diff io.ReadCloser, err error) { func (b *BitbucketAdapter) GetDiff(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (diff io.ReadCloser, err error) {
slog.Debug("Bitbucket GetDiff start", "project", projectKey, "repo", repositorySlug, "pr", pullRequestID)
r, err := b.CreateRequest( r, err := b.CreateRequest(
ctx, ctx,
http.MethodGet, http.MethodGet,
@@ -41,7 +39,6 @@ func (b *BitbucketAdapter) GetDiff(ctx context.Context, projectKey, repositorySl
} }
func (b *BitbucketAdapter) GetPR(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (pr PullRequest, err error) { func (b *BitbucketAdapter) GetPR(ctx context.Context, projectKey, repositorySlug string, pullRequestID int) (pr PullRequest, err error) {
slog.Debug("Bitbucket GetPR start", "project", projectKey, "repo", repositorySlug, "pr", pullRequestID)
r, err := b.CreateRequest( r, err := b.CreateRequest(
ctx, ctx,
http.MethodGet, http.MethodGet,
@@ -50,23 +47,17 @@ func (b *BitbucketAdapter) GetPR(ctx context.Context, projectKey, repositorySlug
) )
response, err := http.DefaultClient.Do(r) response, err := http.DefaultClient.Do(r)
defer response.Body.Close() // Add this
if err != nil { if err != nil {
return return
} }
defer response.Body.Close() // Add this
err = json.NewDecoder(response.Body).Decode(&pr) err = json.NewDecoder(response.Body).Decode(&pr)
if err != nil {
slog.Error("Bitbucket GetPR decode error", "err", err)
return
}
slog.Info("Bitbucket GetPR success", "id", pullRequestID)
return return
} }
func (b *BitbucketAdapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) (err error) { func (b *BitbucketAdapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) (err error) {
slog.Debug("Bitbucket AddComment start", "owner", owner, "repo", repo, "pr", prID, "file", comment.File, "line", comment.Line)
// pr, err := b.GetPR(ctx, owner, repo, prID) // pr, err := b.GetPR(ctx, owner, repo, prID)
// if err != nil { // if err != nil {
// return // return
@@ -95,18 +86,15 @@ func (b *BitbucketAdapter) AddComment(ctx context.Context, owner, repo string, p
} }
response, err := http.DefaultClient.Do(r) response, err := http.DefaultClient.Do(r)
defer response.Body.Close() // Add this
if err != nil { if err != nil {
return err return err
} }
defer response.Body.Close() // Add this
if response.StatusCode >= 300 || response.StatusCode < 200 { if response.StatusCode >= 300 || response.StatusCode < 200 {
sb := &strings.Builder{} sb := &strings.Builder{}
io.Copy(sb, response.Body) io.Copy(sb, response.Body)
err = fmt.Errorf("error while creating comment staus %d, body %s", response.StatusCode, sb.String()) err = fmt.Errorf("error while creating comment staus %d, body %s", response.StatusCode, sb.String())
slog.Error("Bitbucket AddComment failed", "status", response.StatusCode, "err", err)
} else {
slog.Info("Bitbucket AddComment succeeded", "pr", prID)
} }
return err return err

View File

@@ -5,7 +5,6 @@ import (
"context" "context"
"fmt" "fmt"
"io" "io"
"log/slog"
"code.gitea.io/sdk/gitea" "code.gitea.io/sdk/gitea"
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/pierre" "git.schreifuchs.ch/schreifuchs/pierre-bot/internal/pierre"
@@ -26,19 +25,15 @@ func New(baseURL, token string) (*Adapter, error) {
} }
func (g *Adapter) GetDiff(ctx context.Context, owner, repo string, prID int) (io.ReadCloser, error) { func (g *Adapter) GetDiff(ctx context.Context, owner, repo string, prID int) (io.ReadCloser, error) {
slog.Debug("Gitea GetDiff start", "owner", owner, "repo", repo, "pr", prID)
g.client.SetContext(ctx) g.client.SetContext(ctx)
diff, _, err := g.client.GetPullRequestDiff(owner, repo, int64(prID), gitea.PullRequestDiffOptions{}) diff, _, err := g.client.GetPullRequestDiff(owner, repo, int64(prID), gitea.PullRequestDiffOptions{})
if err != nil { if err != nil {
return nil, err return nil, err
} }
rc := io.NopCloser(bytes.NewReader(diff)) return io.NopCloser(bytes.NewReader(diff)), nil
slog.Info("Gitea GetDiff success", "owner", owner, "repo", repo, "pr", prID, "bytes", len(diff))
return rc, nil
} }
func (g *Adapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) error { func (g *Adapter) AddComment(ctx context.Context, owner, repo string, prID int, comment pierre.Comment) error {
slog.Debug("Gitea AddComment start", "owner", owner, "repo", repo, "pr", prID, "file", comment.File, "line", comment.Line)
g.client.SetContext(ctx) g.client.SetContext(ctx)
opts := gitea.CreatePullReviewOptions{ opts := gitea.CreatePullReviewOptions{
State: gitea.ReviewStateComment, State: gitea.ReviewStateComment,
@@ -51,30 +46,22 @@ func (g *Adapter) AddComment(ctx context.Context, owner, repo string, prID int,
}, },
} }
_, _, err := g.client.CreatePullReview(owner, repo, int64(prID), opts) _, _, err := g.client.CreatePullReview(owner, repo, int64(prID), opts)
if err != nil {
slog.Error("Gitea AddComment failed", "err", err)
return err return err
} }
slog.Info("Gitea AddComment succeeded", "pr", prID)
return nil
}
// GetFileContent returns the file content at a given path and ref (commit SHA). // GetFileContent returns the file content at a given path and ref (commit SHA).
func (g *Adapter) GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error) { func (g *Adapter) GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error) {
slog.Debug("Gitea GetFileContent start", "owner", owner, "repo", repo, "path", path, "ref", ref)
g.client.SetContext(ctx) g.client.SetContext(ctx)
// The SDK's GetFile returns the raw bytes of the file. // The SDK's GetFile returns the raw bytes of the file.
data, _, err := g.client.GetFile(owner, repo, ref, path) data, _, err := g.client.GetFile(owner, repo, ref, path)
if err != nil { if err != nil {
return "", err return "", err
} }
slog.Info("Gitea GetFileContent success", "bytes", len(data))
return string(data), nil return string(data), nil
} }
// GetPRHeadSHA fetches the pull request and returns the head commit SHA. // GetPRHeadSHA fetches the pull request and returns the head commit SHA.
func (g *Adapter) GetPRHeadSHA(ctx context.Context, owner, repo string, prID int) (string, error) { func (g *Adapter) GetPRHeadSHA(ctx context.Context, owner, repo string, prID int) (string, error) {
slog.Debug("Gitea GetPRHeadSHA start", "owner", owner, "repo", repo, "pr", prID)
g.client.SetContext(ctx) g.client.SetContext(ctx)
// GetPullRequest returns the detailed PR information. // GetPullRequest returns the detailed PR information.
pr, _, err := g.client.GetPullRequest(owner, repo, int64(prID)) pr, _, err := g.client.GetPullRequest(owner, repo, int64(prID))
@@ -84,6 +71,5 @@ func (g *Adapter) GetPRHeadSHA(ctx context.Context, owner, repo string, prID int
if pr == nil || pr.Head == nil { if pr == nil || pr.Head == nil {
return "", fmt.Errorf("pull request %d has no head information", prID) return "", fmt.Errorf("pull request %d has no head information", prID)
} }
slog.Info("Gitea GetPRHeadSHA success", "sha", pr.Head.Sha)
return pr.Head.Sha, nil return pr.Head.Sha, nil
} }

View File

@@ -1,8 +1,6 @@
package pierre package pierre
import ( import (
"log/slog"
"context" "context"
"fmt" "fmt"
"io" "io"
@@ -21,7 +19,6 @@ type Comment struct {
} }
func (s *Service) judgePR(ctx context.Context, diff io.Reader) (comments []Comment, err error) { func (s *Service) judgePR(ctx context.Context, diff io.Reader) (comments []Comment, err error) {
slog.Info("judgePR started")
diffBytes, err := io.ReadAll(diff) diffBytes, err := io.ReadAll(diff)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to read diff: %w", err) return nil, fmt.Errorf("failed to read diff: %w", err)
@@ -84,7 +81,6 @@ If project guidelines are provided, treat them as hard rules that must be respec
for _, v := range unique { for _, v := range unique {
comments = append(comments, v) comments = append(comments, v)
} }
slog.Info("judgePR finished", "comments", len(comments))
return return
} }
@@ -92,10 +88,6 @@ If project guidelines are provided, treat them as hard rules that must be respec
// It tries to split on file boundaries ("diff --git") first, then on hunk boundaries (@@), // It tries to split on file boundaries ("diff --git") first, then on hunk boundaries (@@),
// and finally on a hard byte limit. // and finally on a hard byte limit.
func splitDiffIntoChunks(diff []byte, maxSize int) []string { func splitDiffIntoChunks(diff []byte, maxSize int) []string {
// Preserve the file header for each chunk when a single file's diff is split across multiple chunks.
// The header is the portion before the first hunk marker "@@" (including the "diff --git" line).
// When we need to split by hunks, we prepend this header to every resulting subchunk.
if len(diff) <= maxSize { if len(diff) <= maxSize {
return []string{string(diff)} return []string{string(diff)}
} }
@@ -115,31 +107,22 @@ func splitDiffIntoChunks(diff []byte, maxSize int) []string {
current.Reset() current.Reset()
} }
if len(seg) > maxSize { if len(seg) > maxSize {
// Determine if there is a hunk marker. If not, fall back to simple sizebased chunking. // Split further by hunks
headerEnd := strings.Index(seg, "\n@@ ") hunks := strings.Split(seg, "\n@@ ")
if headerEnd == -1 { for j, h := range hunks {
// No hunk marker split purely by size. var hseg string
remaining := seg if j == 0 {
for len(remaining) > maxSize { // First hunk segment already contains the preceding content (including any needed newline)
chunks = append(chunks, remaining[:maxSize]) hseg = h
remaining = remaining[maxSize:] } else {
// Subsequent hunks need the leading newline and "@@ " marker restored
hseg = "\n@@ " + h
} }
current.WriteString(remaining)
continue
}
// Preserve the header up to the first hunk.
header := seg[:headerEnd+1] // include newline before "@@"
// Split the rest of the segment into hunks (excluding the header part).
hunks := strings.Split(strings.TrimPrefix(seg, header), "\n@@ ")
for _, h := range hunks {
// Reconstruct each hunk with its header and "@@ " prefix.
hseg := header + "@@ " + h
if current.Len()+len(hseg) > maxSize && current.Len() > 0 { if current.Len()+len(hseg) > maxSize && current.Len() > 0 {
chunks = append(chunks, current.String()) chunks = append(chunks, current.String())
current.Reset() current.Reset()
} }
if len(hseg) > maxSize { if len(hseg) > maxSize {
// If a single hunk exceeds maxSize, split it further.
for len(hseg) > maxSize { for len(hseg) > maxSize {
chunks = append(chunks, hseg[:maxSize]) chunks = append(chunks, hseg[:maxSize])
hseg = hseg[maxSize:] hseg = hseg[maxSize:]