From 97398d8c3864865acd54e0ee140f5b65dc00eee3 Mon Sep 17 00:00:00 2001 From: Staging-Devin AI <166158716+staging-devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 01:13:28 +0000 Subject: [PATCH] codeowners: allow owner signoff through Comment reviews Owners currently have to approve a pull request to satisfy ownership, even when they only want to sign off on their own areas. Accept a standalone codeowners-approved directive in Comment reviews and apply the existing user/team ownership rules. Keep signoffs out of ordinary approval counts and automatic approval, and invalidate stale signoffs without mutating GitHub reviews. Co-Authored-By: Ethan Wu --- README.md | 37 ++++- internal/app/app.go | 39 ++++- internal/app/signoffs_test.go | 153 +++++++++++++++++ internal/github/gh.go | 1 + internal/github/signoffs.go | 40 +++++ internal/github/signoffs_test.go | 154 ++++++++++++++++++ pkg/directives/directives.go | 73 +++++++++ pkg/directives/directives_test.go | 29 ++++ .../testdata/codeowners_approved.json | 41 +++++ 9 files changed, 565 insertions(+), 2 deletions(-) create mode 100644 internal/app/signoffs_test.go create mode 100644 internal/github/signoffs.go create mode 100644 internal/github/signoffs_test.go create mode 100644 pkg/directives/directives.go create mode 100644 pkg/directives/directives_test.go create mode 100644 pkg/directives/testdata/codeowners_approved.json diff --git a/README.md b/README.md index f9dc06d..19eb61c 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ Code Ownership & Review Assignment Tool - GitHub CODEOWNERS but better [![Go Report Card](https://goreportcard.com/badge/github.com/multimediallc/codeowners-plus)](https://goreportcard.com/report/github.com/multimediallc/codeowners-plus?kill_cache=1) [![Tests](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml/badge.svg)](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml) -![Coverage](https://img.shields.io/badge/Coverage-83.0%25-brightgreen) +![Coverage](https://img.shields.io/badge/Coverage-84.3%25-brightgreen) [![License](https://img.shields.io/badge/License-BSD%203--Clause-blue.svg)](https://opensource.org/licenses/BSD-3-Clause) [![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-2.1-4baaaa.svg)](CODE_OF_CONDUCT.md) @@ -113,6 +113,41 @@ It is recommended to also set up a rerun workflow on `pull_request_review` to re If you plan to have organization teams as code owners, you will need to use a PAT that has organization [read access for Members and Administration](https://docs.github.com/en/rest/authentication/permissions-required-for-fine-grained-personal-access-tokens) as the token. If you do not have organization teams as owners, [GITHUB_TOKEN](https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#using-the-github_token-in-a-workflow) should be sufficient. +### Owner-only signoff + +To satisfy ownership requirements without approving the pull request, submit a +GitHub **Comment** review whose body includes this standalone line: + +```text +codeowners-approved +``` + +The signer satisfies every required owner group their user or team membership +can satisfy under the existing `.codeowners` rules. This does not create an +`APPROVED` review, count toward `min_reviews` or `max_reviews`, or cause +`enforcement.approval` to approve a PR whose ownership depends on the signoff. +Ordinary approving reviews continue to satisfy ownership and review counts. +PR-author handling remains controlled by `allow_self_approval`. + +Recognition is case-insensitive and allows up to three leading spaces and +trailing spaces/tabs. Prose, inline backticks, blockquotes, list items, indented +code, and backtick/tilde fenced code do not count. Leave a blank line after a +quote, list, or HTML paragraph before writing the directive. + +Unrelated later comments preserve a signoff. Editing a review to remove or +invalidate its directive withdraws that review's signoff; any other valid +signoff reviews remain effective. A later **Request changes** or dismissed +review supersedes earlier signoffs by that reviewer. A new Comment review can +sign off again. Signoffs survive pushes with `disable_smart_dismissal = true`; +otherwise, changes to the signer's owned files invalidate them using the same +diff checks as ordinary approvals. Stale Comment reviews are ignored rather +than dismissed through GitHub. + +Integrations must rerun Codeowners Plus on review submission, dismissal, and +body edits adding or removing a valid directive. The parser compatibility +fixtures are in `pkg/directives/testdata/codeowners_approved.json`; webhook +receivers should validate against those same cases. + ## Configuration ### .codeowners File Spec diff --git a/internal/app/app.go b/internal/app/app.go index 69c0b41..0a3345f 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -248,6 +248,12 @@ func (a *App) processApprovalsAndReviewers() (bool, string, []string, error) { return false, message, nil, err } + requiredBeforeSignoffs := len(a.codeowners.AllRequired()) + if err := a.processOwnerSignoffs(); err != nil { + return false, message, nil, err + } + usedOwnerSignoff := len(a.codeowners.AllRequired()) < requiredBeforeSignoffs + // Request reviews from required owners err = a.requestReviews() if err != nil { @@ -349,7 +355,7 @@ func (a *App) processApprovalsAndReviewers() (bool, string, []string, error) { } message = "Codeowners reviews satisfied" - if a.Conf.Enforcement.Approval && tokenOwnerApproval == nil { + if a.Conf.Enforcement.Approval && tokenOwnerApproval == nil && !usedOwnerSignoff { // Approve the PR since all codeowner teams have approved err = a.client.ApprovePR() if err != nil { @@ -515,6 +521,37 @@ func (a *App) processApprovals(ghApprovals []*gh.CurrentApproval) (int, error) { return len(ghApprovals) - len(approvalsToDismiss), nil } +func (a *App) processOwnerSignoffs() error { + signoffs, err := a.client.GetCurrentOwnerSignoffs() + if err != nil { + return fmt.Errorf("GetCurrentOwnerSignoffs Error: %v", err) + } + if len(signoffs) == 0 { + return nil + } + var approvers []codeowners.Slug + var stale []*gh.CurrentApproval + if a.Conf.DisableSmartDismissal { + for _, signoff := range signoffs { + approvers = append(approvers, signoff.Reviewers...) + } + } else { + fileReviewers := f.MapMap(a.codeowners.FileRequired(), func(reviewers codeowners.ReviewerGroups) []string { + return codeowners.NormalizedStrings(reviewers.Flatten()) + }) + approvers, stale = a.client.CheckApprovals(fileReviewers, signoffs, a.gitDiff) + } + for _, signoff := range signoffs { + if !slices.Contains(stale, signoff) { + a.printDebug("Owner signoff: %s (review %d, commit %s) satisfies %s\n", + signoff.GHLogin.Original(), signoff.ReviewID, signoff.CommitID, codeowners.OriginalStrings(signoff.Reviewers)) + } + } + a.printDebug("Stale owner signoffs (ignored): %+v\n", stale) + a.codeowners.ApplyApprovals(approvers) + return nil +} + func (a *App) requestReviews() error { if a.config.Quiet { return nil diff --git a/internal/app/signoffs_test.go b/internal/app/signoffs_test.go new file mode 100644 index 0000000..d21b511 --- /dev/null +++ b/internal/app/signoffs_test.go @@ -0,0 +1,153 @@ +package app + +import ( + "errors" + "io" + "os" + "path/filepath" + "testing" + + "github.com/google/go-github/v63/github" + owners "github.com/multimediallc/codeowners-plus/internal/config" + "github.com/multimediallc/codeowners-plus/internal/git" + gh "github.com/multimediallc/codeowners-plus/internal/github" + "github.com/multimediallc/codeowners-plus/pkg/codeowners" +) + +func (m *mockGitHubClient) GetCurrentOwnerSignoffs() ([]*gh.CurrentApproval, error) { + return nil, nil +} + +type signoffClient struct { + mockGitHubClient + signoffs []*gh.CurrentApproval + signoffsError error + approved bool + dismissed []*gh.CurrentApproval +} + +func (m *signoffClient) GetCurrentOwnerSignoffs() ([]*gh.CurrentApproval, error) { + return m.signoffs, m.signoffsError +} + +func (m *signoffClient) FindUserApproval(user string) (*gh.CurrentApproval, error) { + return nil, nil +} + +func (m *signoffClient) ApprovePR() error { + m.approved = true + return nil +} + +func (m *signoffClient) DismissStaleReviews(approvals []*gh.CurrentApproval) error { + m.dismissed = append(m.dismissed, approvals...) + return nil +} + +func (m *signoffClient) CheckApprovals(fileReviewers map[string][]string, approvals []*gh.CurrentApproval, diff git.Diff) ([]codeowners.Slug, []*gh.CurrentApproval) { + return gh.NewClient("org", "repo", "").CheckApprovals(fileReviewers, approvals, diff) +} + +func newSignoffApp(t *testing.T, client *signoffClient, files []string) *App { + t.Helper() + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, ".codeowners"), []byte("* @org/api @fallback\n&* @org/core\nother.go @other\n"), 0600); err != nil { + t.Fatal(err) + } + diff := mockGitDiff{changes: files} + co, err := codeowners.New(root, diff.AllChanges(), nil, io.Discard) + if err != nil { + t.Fatal(err) + } + return &App{ + config: &Config{Quiet: true, InfoBuffer: io.Discard, WarningBuffer: io.Discard}, + client: client, codeowners: co, gitDiff: diff, + Conf: &owners.Config{ + DisableSmartDismissal: true, + Enforcement: &owners.Enforcement{Approval: true, FailCheck: true}, + }, + } +} + +func TestOwnerSignoffsSatisfyOwnershipOnly(t *testing.T) { + tt := []struct { + name string + reviewers []string + files []string + ordinaryApprovals []*gh.CurrentApproval + minReviews *int + maxReviews *int + expectedSuccess bool + expectedApproval bool + }{ + {name: "all authorized AND/OR groups", reviewers: []string{"@org/api", "@org/core"}, files: []string{"api.go"}, expectedSuccess: true}, + {name: "outsider cannot sign off", files: []string{"api.go"}}, + {name: "unrelated owner remains required", reviewers: []string{"@org/api", "@org/core"}, files: []string{"api.go", "other.go"}}, + {name: "signoff does not meet minimum", reviewers: []string{"@org/api", "@org/core"}, files: []string{"api.go"}, minReviews: github.Int(1)}, + {name: "signoff does not reach maximum", reviewers: []string{"@org/api"}, files: []string{"api.go"}, maxReviews: github.Int(1)}, + {name: "ordinary approval plus signoff", reviewers: []string{"@org/api", "@org/core"}, files: []string{"api.go", "other.go"}, ordinaryApprovals: []*gh.CurrentApproval{{Reviewers: codeowners.NewSlugs([]string{"@other"})}}, minReviews: github.Int(1), expectedSuccess: true}, + {name: "ordinary approvals retain enforcement", files: []string{"api.go"}, ordinaryApprovals: []*gh.CurrentApproval{{Reviewers: codeowners.NewSlugs([]string{"@org/api", "@org/core"})}}, expectedSuccess: true, expectedApproval: true}, + {name: "redundant signoff retains enforcement", reviewers: []string{"@org/api", "@org/core"}, files: []string{"api.go"}, ordinaryApprovals: []*gh.CurrentApproval{{Reviewers: codeowners.NewSlugs([]string{"@org/api", "@org/core"})}}, expectedSuccess: true, expectedApproval: true}, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + client := &signoffClient{mockGitHubClient: mockGitHubClient{currentApprovals: tc.ordinaryApprovals}} + if tc.reviewers != nil { + client.signoffs = []*gh.CurrentApproval{{GHLogin: codeowners.NewSlug("signer"), ReviewID: 10, Reviewers: codeowners.NewSlugs(tc.reviewers), CommitID: "old-head"}} + } + app := newSignoffApp(t, client, tc.files) + app.Conf.MinReviews = tc.minReviews + app.Conf.MaxReviews = tc.maxReviews + success, message, _, err := app.processApprovalsAndReviewers() + if err != nil || success != tc.expectedSuccess { + t.Fatalf("success = %v, expected %v; %s; error = %v", success, tc.expectedSuccess, message, err) + } + if client.approved != tc.expectedApproval { + t.Errorf("GitHub approval created = %v, expected %v", client.approved, tc.expectedApproval) + } + if len(client.dismissed) > 0 { + t.Errorf("unexpected dismissals: %v", client.dismissed) + } + }) + } +} + +func TestOwnerSignoffLifetime(t *testing.T) { + tt := []struct { + name string + disableSmart bool + changes []string + diffError error + expectedSuccess bool + }{ + {name: "persists across pushes when dismissal disabled", disableSmart: true, changes: []string{"api.go"}, expectedSuccess: true}, + {name: "owned changes invalidate", changes: []string{"api.go"}}, + {name: "unrelated changes preserve", changes: []string{"unrelated.go"}, expectedSuccess: true}, + {name: "unchanged commit preserves", expectedSuccess: true}, + {name: "unreadable commit invalidates", diffError: errors.New("missing commit")}, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + client := &signoffClient{signoffs: []*gh.CurrentApproval{{Reviewers: codeowners.NewSlugs([]string{"@org/api", "@org/core"}), CommitID: "reviewed"}}} + app := newSignoffApp(t, client, []string{"api.go"}) + app.Conf.DisableSmartDismissal = tc.disableSmart + app.gitDiff = mockGitDiff{changes: tc.changes, changesSinceError: tc.diffError} + success, message, _, err := app.processApprovalsAndReviewers() + if err != nil || success != tc.expectedSuccess { + t.Fatalf("success = %v, expected %v; %s; error = %v", success, tc.expectedSuccess, message, err) + } + if client.approved || len(client.dismissed) > 0 { + t.Errorf("signoff caused GitHub review mutation: approved=%v dismissed=%v", client.approved, client.dismissed) + } + }) + } +} + +func TestOwnerSignoffReadError(t *testing.T) { + client := &signoffClient{signoffsError: errors.New("review read failed")} + app := newSignoffApp(t, client, []string{"api.go"}) + success, _, _, err := app.processApprovalsAndReviewers() + if err == nil || success { + t.Fatalf("expected failure, got success=%v error=%v", success, err) + } +} diff --git a/internal/github/gh.go b/internal/github/gh.go index bd14108..f6221cc 100644 --- a/internal/github/gh.go +++ b/internal/github/gh.go @@ -38,6 +38,7 @@ type Client interface { AllApprovals() ([]*CurrentApproval, error) FindUserApproval(ghUser string) (*CurrentApproval, error) GetCurrentReviewerApprovals() ([]*CurrentApproval, error) + GetCurrentOwnerSignoffs() ([]*CurrentApproval, error) GetAlreadyReviewed() ([]codeowners.Slug, error) GetCurrentlyRequested() ([]codeowners.Slug, error) DismissStaleReviews(staleApprovals []*CurrentApproval) error diff --git a/internal/github/signoffs.go b/internal/github/signoffs.go new file mode 100644 index 0000000..72f5973 --- /dev/null +++ b/internal/github/signoffs.go @@ -0,0 +1,40 @@ +package gh + +import ( + "strings" + + "github.com/google/go-github/v63/github" + "github.com/multimediallc/codeowners-plus/pkg/directives" +) + +func (gh *GHClient) GetCurrentOwnerSignoffs() ([]*CurrentApproval, error) { + if gh.pr == nil { + return nil, &NoPRError{} + } + if gh.userReviewerMap == nil { + return nil, &UserReviewerMapNotInitError{} + } + if gh.reviews == nil { + if err := gh.InitReviews(); err != nil { + return nil, err + } + } + seen := make(map[string]bool) + signoffs := make([]*github.PullRequestReview, 0) + for _, review := range gh.reviews { + user := strings.ToLower(review.GetUser().GetLogin()) + if seen[user] { + continue + } + switch review.GetState() { + case "CHANGES_REQUESTED", "DISMISSED": + seen[user] = true + case "COMMENTED": + if directives.HasCodeownersApproval(review.GetBody()) { + seen[user] = true + signoffs = append(signoffs, review) + } + } + } + return currentReviewerApprovalsFromReviews(signoffs, gh.userReviewerMap), nil +} diff --git a/internal/github/signoffs_test.go b/internal/github/signoffs_test.go new file mode 100644 index 0000000..1158e68 --- /dev/null +++ b/internal/github/signoffs_test.go @@ -0,0 +1,154 @@ +package gh + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "testing" + + "github.com/google/go-github/v63/github" + "github.com/multimediallc/codeowners-plus/pkg/codeowners" +) + +func ownerReview(login, state, body string, id int64) *github.PullRequestReview { + return &github.PullRequestReview{ + User: &github.User{Login: github.String(login)}, + State: github.String(state), + Body: github.String(body), + ID: github.Int64(id), + CommitID: github.String("reviewed-commit"), + } +} + +func TestCurrentOwnerSignoffs(t *testing.T) { + signoff := ownerReview("OwNeR", "COMMENTED", "codeowners-approved", 1) + tt := []struct { + name string + reviews []*github.PullRequestReview + expected []int64 + }{ + {"signoff", []*github.PullRequestReview{signoff}, []int64{1}}, + {"unrelated comment preserves", []*github.PullRequestReview{ownerReview("owner", "COMMENTED", "Thanks", 2), signoff}, []int64{1}}, + {"request changes revokes", []*github.PullRequestReview{ownerReview("owner", "CHANGES_REQUESTED", "", 2), signoff}, nil}, + {"dismissal revokes", []*github.PullRequestReview{ownerReview("owner", "DISMISSED", "codeowners-approved", 2), signoff}, nil}, + {"new signoff restores", []*github.PullRequestReview{signoff, ownerReview("owner", "CHANGES_REQUESTED", "", 2)}, []int64{1}}, + {"newest signoff per user", []*github.PullRequestReview{ownerReview("owner", "COMMENTED", "codeowners-approved", 2), signoff}, []int64{2}}, + {"ordinary approval is separate", []*github.PullRequestReview{ownerReview("owner", "APPROVED", "codeowners-approved", 2)}, nil}, + {"pending review is ignored", []*github.PullRequestReview{ownerReview("owner", "PENDING", "codeowners-approved", 2)}, nil}, + {"invalid directive", []*github.PullRequestReview{ownerReview("owner", "COMMENTED", "```\ncodeowners-approved\n```", 2)}, nil}, + {"different reviewer cannot revoke", []*github.PullRequestReview{ownerReview("other", "CHANGES_REQUESTED", "", 2), signoff}, []int64{1}}, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + client := &GHClient{ + pr: &github.PullRequest{Number: github.Int(1)}, + reviews: tc.reviews, + userReviewerMap: ghUserReviewerMap{"owner": codeowners.NewSlugs([]string{"@org/api", "@org/core"})}, + } + signoffs, err := client.GetCurrentOwnerSignoffs() + if err != nil { + t.Fatal(err) + } + var ids []int64 + for _, signoff := range signoffs { + ids = append(ids, signoff.ReviewID) + expected := []string{"@org/api", "@org/core"} + if !reflect.DeepEqual(codeowners.OriginalStrings(signoff.Reviewers), expected) { + t.Errorf("reviewers = %v, expected %v", signoff.Reviewers, expected) + } + if signoff.CommitID != "reviewed-commit" { + t.Errorf("commit = %q", signoff.CommitID) + } + } + if !reflect.DeepEqual(ids, tc.expected) { + t.Errorf("signoff IDs = %v, expected %v", ids, tc.expected) + } + }) + } +} + +func TestOwnerSignoffEditAndApprovalSeparation(t *testing.T) { + review := ownerReview("reviewer1", "COMMENTED", "codeowners-approved", 10) + client := setupReviews() + client.reviews = []*github.PullRequestReview{review} + signoffs, err := client.GetCurrentOwnerSignoffs() + if err != nil || len(signoffs) != 1 { + t.Fatalf("signoffs = %v, error = %v", signoffs, err) + } + approvals, err := client.GetCurrentReviewerApprovals() + if err != nil || len(approvals) != 0 { + t.Fatalf("signoff counted as ordinary approval: %v, %v", approvals, err) + } + allApprovals, err := client.AllApprovals() + if err != nil || len(allApprovals) != 0 { + t.Fatalf("signoff counted in AllApprovals: %v, %v", allApprovals, err) + } + approval, err := client.FindUserApproval("reviewer1") + if err != nil || approval != nil { + t.Fatalf("signoff counted by FindUserApproval: %v, %v", approval, err) + } + review.Body = github.String("Withdrawn") + signoffs, err = client.GetCurrentOwnerSignoffs() + if err != nil || len(signoffs) != 0 { + t.Fatalf("removed signoff still counts: %v, %v", signoffs, err) + } +} + +func TestOwnerSignoffMembershipAndAuthorExclusion(t *testing.T) { + reviews := []*github.PullRequestReview{ + ownerReview("author", "COMMENTED", "codeowners-approved", 1), + ownerReview("Teammate", "COMMENTED", "codeowners-approved", 2), + ownerReview("outsider", "COMMENTED", "codeowners-approved", 3), + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/repos/org/repo/pulls/1/reviews" { + t.Errorf("unexpected path %s", r.URL.Path) + http.NotFound(w, r) + return + } + if err := json.NewEncoder(w).Encode(reviews); err != nil { + t.Error(err) + } + })) + defer server.Close() + api := github.NewClient(server.Client()) + api.BaseURL, _ = url.Parse(server.URL + "/") + client := &GHClient{ + ctx: context.Background(), owner: "org", repo: "repo", client: api, + pr: &github.PullRequest{Number: github.Int(1), User: &github.User{Login: github.String("author")}}, + userReviewerMap: makeGHUserReviwerMap([]string{"@org/api", "@org/core", "@teammate"}, func(org, team string) []*github.User { + return []*github.User{{Login: github.String("teammate")}} + }), + warningBuffer: io.Discard, infoBuffer: io.Discard, + } + signoffs, err := client.GetCurrentOwnerSignoffs() + if err != nil { + t.Fatal(err) + } + if len(signoffs) != 2 { + t.Fatalf("signoffs = %v, expected author excluded", signoffs) + } + if len(signoffs[0].Reviewers) != 0 { + t.Errorf("outsider satisfies owners: %v", signoffs[0].Reviewers) + } + expected := []string{"@org/api", "@org/core", "@teammate"} + if !reflect.DeepEqual(codeowners.OriginalStrings(signoffs[1].Reviewers), expected) { + t.Errorf("team/user mapping = %v, expected %v", signoffs[1].Reviewers, expected) + } +} + +func TestOwnerSignoffInitializationErrors(t *testing.T) { + client := &GHClient{} + if _, err := client.GetCurrentOwnerSignoffs(); !errors.As(err, new(*NoPRError)) { + t.Fatalf("expected NoPRError, got %v", err) + } + client.pr = &github.PullRequest{} + if _, err := client.GetCurrentOwnerSignoffs(); !errors.As(err, new(*UserReviewerMapNotInitError)) { + t.Fatalf("expected UserReviewerMapNotInitError, got %v", err) + } +} diff --git a/pkg/directives/directives.go b/pkg/directives/directives.go new file mode 100644 index 0000000..48f7597 --- /dev/null +++ b/pkg/directives/directives.go @@ -0,0 +1,73 @@ +package directives + +import ( + "regexp" + "strings" +) + +var containerLine = regexp.MustCompile(`^(>|[-+*][ \t]|[0-9]+[.)][ \t]|<)`) +var indentedLine = regexp.MustCompile(`^( {4}| {0,3}\t)`) +var htmlCodeBlock = regexp.MustCompile(`^<(pre|script|style|textarea)([ \t>]|$)`) + +// HasCodeownersApproval recognizes an unformatted, standalone directive line. +// Quotes, lists and HTML paragraphs must end with a blank line before a directive. +func HasCodeownersApproval(body string) bool { + var fence byte + fenceLength := 0 + blockedParagraph := false + inComment := false + htmlEnd := "" + for _, raw := range strings.Split(strings.ReplaceAll(body, "\r\n", "\n"), "\n") { + line := strings.Trim(raw, " \t") + lower := strings.ToLower(line) + if fence != 0 { + run := len(line) - len(strings.TrimLeft(line, string(fence))) + if run >= fenceLength && strings.Trim(line[run:], " \t") == "" && + !indentedLine.MatchString(raw) { + fence = 0 + } + continue + } + if htmlEnd == "" && !inComment { + if match := htmlCodeBlock.FindStringSubmatch(lower); match != nil { + htmlEnd = "" + } + } + if htmlEnd != "" { + if strings.Contains(lower, htmlEnd) { + htmlEnd = "" + } + continue + } + if inComment || strings.Contains(line, "") + blockedParagraph = true + continue + } + if line == "" { + blockedParagraph = false + continue + } + if indentedLine.MatchString(raw) { + continue + } + if containerLine.MatchString(line) { + blockedParagraph = true + } + if blockedParagraph { + continue + } + if line[0] == '`' || line[0] == '~' { + run := len(line) - len(strings.TrimLeft(line, line[:1])) + if run >= 3 && (line[0] == '~' || !strings.Contains(line[run:], "`")) { + fence = line[0] + fenceLength = run + continue + } + } + if lower == "codeowners-approved" { + return true + } + } + return false +} diff --git a/pkg/directives/directives_test.go b/pkg/directives/directives_test.go new file mode 100644 index 0000000..97bba46 --- /dev/null +++ b/pkg/directives/directives_test.go @@ -0,0 +1,29 @@ +package directives + +import ( + "encoding/json" + "os" + "testing" +) + +func TestHasCodeownersApproval(t *testing.T) { + data, err := os.ReadFile("testdata/codeowners_approved.json") + if err != nil { + t.Fatal(err) + } + var tt []struct { + Name string `json:"name"` + Body string `json:"body"` + Expected bool `json:"expected"` + } + if err := json.Unmarshal(data, &tt); err != nil { + t.Fatal(err) + } + for _, tc := range tt { + t.Run(tc.Name, func(t *testing.T) { + if actual := HasCodeownersApproval(tc.Body); actual != tc.Expected { + t.Errorf("HasCodeownersApproval(%q) = %v, expected %v", tc.Body, actual, tc.Expected) + } + }) + } +} diff --git a/pkg/directives/testdata/codeowners_approved.json b/pkg/directives/testdata/codeowners_approved.json new file mode 100644 index 0000000..935808e --- /dev/null +++ b/pkg/directives/testdata/codeowners_approved.json @@ -0,0 +1,41 @@ +[ + {"name": "empty", "body": "", "expected": false}, + {"name": "directive", "body": "codeowners-approved", "expected": true}, + {"name": "case and whitespace", "body": " CODEOWNERS-APPROVED \t", "expected": true}, + {"name": "CRLF", "body": "Reviewed.\r\ncodeowners-approved\r\nThanks.", "expected": true}, + {"name": "surrounding prose", "body": "Reviewed my areas.\ncodeowners-approved\nOther owners should review theirs.", "expected": true}, + {"name": "negated prose", "body": "not codeowners-approved yet", "expected": false}, + {"name": "prefix", "body": "not-codeowners-approved", "expected": false}, + {"name": "suffix", "body": "codeowners-approved: looks good", "expected": false}, + {"name": "inline code", "body": "`codeowners-approved`", "expected": false}, + {"name": "blockquote", "body": "> codeowners-approved", "expected": false}, + {"name": "lazy blockquote continuation", "body": "> Example:\ncodeowners-approved", "expected": false}, + {"name": "after blockquote", "body": "> Example\n\ncodeowners-approved", "expected": true}, + {"name": "list", "body": "- codeowners-approved", "expected": false}, + {"name": "ordered list", "body": "1. codeowners-approved", "expected": false}, + {"name": "lazy list continuation", "body": "- Example:\ncodeowners-approved", "expected": false}, + {"name": "after list", "body": "- Example\n\ncodeowners-approved", "expected": true}, + {"name": "indentation", "body": " codeowners-approved", "expected": false}, + {"name": "tab indentation", "body": "\tcodeowners-approved", "expected": false}, + {"name": "mixed indentation", "body": " \tcodeowners-approved", "expected": false}, + {"name": "fenced code", "body": "```\ncodeowners-approved\n```", "expected": false}, + {"name": "fenced language", "body": "```text\ncodeowners-approved\n```", "expected": false}, + {"name": "tilde fence", "body": "~~~text\ncodeowners-approved\n~~~", "expected": false}, + {"name": "unclosed fence", "body": "```\ncodeowners-approved", "expected": false}, + {"name": "short inner fence", "body": "````\n```\ncodeowners-approved\n````", "expected": false}, + {"name": "different inner fence", "body": "```\n~~~\ncodeowners-approved\n```", "expected": false}, + {"name": "closing fence with text", "body": "```\n```example\ncodeowners-approved\n```", "expected": false}, + {"name": "after closed fence", "body": "```\nexample\n```\ncodeowners-approved", "expected": true}, + {"name": "longer closing fence", "body": "~~~\nexample\n~~~~\ncodeowners-approved", "expected": true}, + {"name": "indented fence", "body": " ```\ncodeowners-approved\n ```", "expected": false}, + {"name": "indented closing fence", "body": "```\n ```\ncodeowners-approved", "expected": false}, + {"name": "comment", "body": "", "expected": false}, + {"name": "HTML paragraph", "body": "
\ncodeowners-approved\n
", "expected": false}, + {"name": "HTML code with blank line", "body": "
\n\ncodeowners-approved\n
", "expected": false}, + {"name": "after HTML code", "body": "
\nexample\n
\ncodeowners-approved", "expected": true}, + {"name": "HTML textarea", "body": "", "expected": false}, + {"name": "comment mentioning HTML", "body": "\n\ncodeowners-approved", "expected": true}, + {"name": "after HTML comment", "body": "\n\ncodeowners-approved", "expected": true}, + {"name": "valid after example", "body": "```text\ncodeowners-approved\n```\n\ncodeowners-approved", "expected": true}, + {"name": "non-ASCII whitespace", "body": "\u00a0codeowners-approved", "expected": false} +]