diff --git a/CODEOWNERS b/CODEOWNERS index 3ed626464..29f356e30 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,23 +1,12 @@ # CODEOWNERS - defines who is responsible for code in this repository # See: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners -# Default owners for everything in the repo -* @josealekhine -* @parlakisik -* @CoderMungan -* @hamzaerbay -* @bilersan - -# Core CLI implementation -/cmd/ @josealekhine -/internal/ @josealekhine - -# Documentation -*.md @josealekhine -/docs/ @josealekhine -/specs/ @josealekhine - -# Build and CI -/hack/ @josealekhine -/.github/ @josealekhine -Makefile @josealekhine +# All five maintainers own the whole repository; any one of them can +# give the code-owner approval the main ruleset requires. +# +# Keep them on ONE line. GitHub applies only the last pattern that +# matches a file, so separate `*` lines override one another (the last +# one wins), and any narrower pattern added below this line (e.g. +# `/internal/ @someone`) would replace everyone else's ownership of +# that path instead of adding to it. See specs/codeowners-full-ownership.md. +* @josealekhine @parlakisik @CoderMungan @hamzaerbay @bilersan diff --git a/internal/compliance/codeowners_test.go b/internal/compliance/codeowners_test.go new file mode 100644 index 000000000..7fe03f81a --- /dev/null +++ b/internal/compliance/codeowners_test.go @@ -0,0 +1,74 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package compliance + +import ( + "bufio" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestCodeownersSingleRule verifies that CODEOWNERS holds exactly +// one rule, `*`, naming every maintainer. +// +// GitHub applies only the last CODEOWNERS pattern that matches a +// file. Appending `* @new-maintainer` on its own line therefore +// replaces the previous owners instead of adding one, and any +// narrower pattern (`/internal/ @someone`) strips everyone else's +// ownership of that path. Both happened: five separate `*` lines +// left only the last name as owner of most of the repo, and eight +// path rules left a single owner of cmd/, internal/, docs/, specs/, +// hack/, .github/, and the Makefile. +// +// The policy is full ownership for every maintainer, so the file +// must be one `*` line. Owner names are not pinned here; adding a +// maintainer means appending a handle to that line. +// +// See specs/codeowners-full-ownership.md. +func TestCodeownersSingleRule(t *testing.T) { + root := projectRoot(t) + + //nolint:gosec // constructed from test constants + f, openErr := os.Open(filepath.Clean(filepath.Join(root, "CODEOWNERS"))) + if openErr != nil { + t.Fatalf("open CODEOWNERS: %v", openErr) + } + defer func() { _ = f.Close() }() + + var rules []string + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + rules = append(rules, line) + } + if scanErr := scanner.Err(); scanErr != nil { + t.Fatalf("read CODEOWNERS: %v", scanErr) + } + + if len(rules) != 1 { + t.Fatalf("CODEOWNERS has %d rules, want exactly 1 (`* @a @b ...`):\n\t%s", + len(rules), strings.Join(rules, "\n\t")) + } + + fields := strings.Fields(rules[0]) + if fields[0] != "*" { + t.Errorf("CODEOWNERS rule pattern is %q, want %q", fields[0], "*") + } + if len(fields) < 2 { + t.Errorf("CODEOWNERS `*` rule names no owners") + } + for _, owner := range fields[1:] { + if !strings.HasPrefix(owner, "@") { + t.Errorf("CODEOWNERS owner %q is not an @handle", owner) + } + } +} diff --git a/specs/codeowners-full-ownership.md b/specs/codeowners-full-ownership.md new file mode 100644 index 000000000..db5fce3c9 --- /dev/null +++ b/specs/codeowners-full-ownership.md @@ -0,0 +1,53 @@ +# CODEOWNERS: Full Ownership for Every Maintainer + +The `main` ruleset requires a code-owner approval on every pull +request. The intent is that each of the five maintainers +(@josealekhine, @parlakisik, @CoderMungan, @hamzaerbay, @bilersan) +owns the whole repository, so any one of them can approve. The +CODEOWNERS file did not say that. + +## Problem + +GitHub applies only the **last** CODEOWNERS pattern that matches a +file. The file had: + +- **Five separate `* @user` lines**, one appended per maintainer + (all on 2026-03-08). Each overrode the one above it, so for most + of the repo only `* @bilersan`, the last line, counted. +- **Eight path rules naming only @josealekhine** (`/cmd/`, + `/internal/`, `*.md`, `/docs/`, `/specs/`, `/hack/`, `/.github/`, + `Makefile`). Being later in the file, they replaced the `*` owners + for those paths instead of adding to them. + +Net effect: @bilersan was sole owner of everything outside those +paths, @josealekhine was sole owner of everything inside them, and +the other three owned nothing. GitHub's CODEOWNERS error check +reported nothing, because the file was syntactically valid. + +## Solution + +- **One rule:** `* @josealekhine @parlakisik @CoderMungan + @hamzaerbay @bilersan`. A comment above it explains the + last-match rule, so the next maintainer added goes onto this line. +- **Path rules removed.** Each one could only narrow ownership, + which contradicts the policy. +- **Guard:** `internal/compliance/codeowners_test.go` requires + exactly one non-comment rule, with pattern `*` and at least one + `@handle`. It doesn't pin the names, so adding a maintainer stays a + one-line edit, but appending a second line fails `go test`. + +All five accounts have write access, which GitHub requires before a +code owner counts. + +## Verification + +- `TestCodeownersSingleRule` passes on the new file, and fails on + the old one (13 rules). +- After merge, `gh api repos/ActiveMemory/ctx/codeowners/errors` + reports no errors. + +## Non-Goals + +- Per-area ownership. If areas get dedicated owners later, the + policy changes and so must this guard; every rule must then repeat + all the owners it intends.