Merge pull request #131 from NDDev-OpenNetwork/feat/classify-by-account-not-by-fork

feat(estate): classify a repository by its account, not by being a fork

Author: rldyourmnd

.gds/bundle.lock.yaml

@@ -5,14 +5,14 @@ bundle:
   version: "0.9.0-dev"
   release_sequence: 0
   channel: "development"
-  source_tree_digest: "sha256:e88904c3b4b3654f40b30843ea28f1ab33bb4ee45db6d6b0f0f89076857ba54f"
-  digest: "sha256:1e6b426bb6796d36de00d07037fbfc2591f2f6af30d63f2bd65ae091dbf82973"
+  source_tree_digest: "sha256:4f651053830e5752c664d7f08be88f18164c6f4cd57c7450631e459250bac15b"
+  digest: "sha256:e587a8a3ae46d44ccb7d1385eeba75a89463d9e5642c7e05b914c42a386e6ae4"
 
 projection:
-  input_digest: "sha256:c939fde7254664217bc73f2b990ce7e73db38359ae62bcc8ca690ab6ca066b92"
-  output_digest: "sha256:0f5697325f6623916cb575aa8e1b2e0a3422635de596a1fd048d2776deda131c"
+  input_digest: "sha256:470d0746c588cb1f506c95548a99d4304424f27cc728e4d9fc559a81d4453d88"
+  output_digest: "sha256:d3f7deef13fb002b68c913edf9e838842f56acf1624510a89c2dd4a3b6eea2ea"
   files:
     - path: ".gds/compiled-policy.json"
       digest: "sha256:7ba2962e8afdf99eaf32582cd866f394abff2e24728b0644f013ec67d478b521"
     - path: ".github/workflows/gds-ci.yml"
-      digest: "sha256:a6a19aa0ac91644efe450663924f229a7f474d016e58764be2b561608b4bd8b4"
+      digest: "sha256:56d79046a6adea23115ff848ad8de960e4b7c1baf33ebb08d50c81a1246522bc"

.github/workflows/gds-ci.yml

@@ -1,8 +1,8 @@
 # GENERATED FILE - DO NOT EDIT DIRECTLY
 # generator: gds
 # bundle: 0.9.0-dev
-# source-tree-digest: sha256:e88904c3b4b3654f40b30843ea28f1ab33bb4ee45db6d6b0f0f89076857ba54f
-# input-digest: sha256:c939fde7254664217bc73f2b990ce7e73db38359ae62bcc8ca690ab6ca066b92
+# source-tree-digest: sha256:4f651053830e5752c664d7f08be88f18164c6f4cd57c7450631e459250bac15b
+# input-digest: sha256:470d0746c588cb1f506c95548a99d4304424f27cc728e4d9fc559a81d4453d88
 # output-digest: sha256:8c045e745cc69b731bc695a4a9d58a48c10f1ab7dd85b7354db7bfd0e072711c
 # edit-source:
 #   - .gds/repository.yaml

core/assurance/fixtures.go

@@ -54,10 +54,10 @@ func (fixtureAudit) Record(
 
 func buildFixtureRepositories(options Options) []fixtureRepository {
 	result := make([]fixtureRepository, 0, options.RepositoryCount)
-	// Forks are restricted to owners that carry explicit fork selectors in the
-	// canonical estate (personal and organization); the other organization
-	// installations have no fork selector, so a fork owned there would compile
-	// unassigned.
+	// Two owners carry the fork half of the fixture. The restriction used to
+	// be forced — only these two had fork selectors — and is now only a stable
+	// distribution the bounded-evidence counts are calibrated against. A fork
+	// is classified by the account that holds it, like any other repository.
 	forkOwners := []string{"example-user", "example-org"}
 	sourceOwners := []string{"example-user", "example-org", "example-media", "NDDev-OpenNetwork"}
 	for index := 0; index < options.RepositoryCount; index++ {

core/assurance/runner.go

@@ -89,10 +89,15 @@ func Run(ctx context.Context, options Options, schemas *validation.Set) (Report,
 			len(compiled.Repositories), findingCodes(findings),
 		)
 	}
-	forkAssignments := countForkAssignments(compiled)
+	// Forks are no longer a classification of their own, so the invariant is
+	// no longer "N repositories landed in a fork portfolio" — a count that
+	// would now be zero forever and prove nothing. What must hold is that a
+	// fork is classified exactly like any other repository of its account:
+	// matched by a selector and carrying a portfolio.
+	forkAssignments := countClassifiedForks(fixtures, compiled)
 	if forkAssignments != options.ForkCount {
 		return Report{}, fmt.Errorf(
-			"fork assignment mismatch: got %d want %d", forkAssignments, options.ForkCount,
+			"classified fork mismatch: got %d want %d", forkAssignments, options.ForkCount,
 		)
 	}
 
@@ -267,13 +272,24 @@ func resolveRoot(path string) (string, error) {
 	return filepath.Clean(resolved), nil
 }
 
-func countForkAssignments(inventory estate.CompiledInventory) int {
-	count := 0
+func countClassifiedForks(
+	fixtures []fixtureRepository,
+	inventory estate.CompiledInventory,
+) int {
+	assignments := make(map[int64]estate.Assignment, len(inventory.Repositories))
 	for _, assignment := range inventory.Repositories {
-		if assignment.MatchedSelector == "personal-forks" ||
-			assignment.MatchedSelector == "organization-forks" {
-			count++
+		assignments[assignment.ProviderID] = assignment
+	}
+	count := 0
+	for _, fixture := range fixtures {
+		if !fixture.Observed.Fork {
+			continue
+		}
+		assignment, found := assignments[fixture.Observed.ProviderID]
+		if !found || assignment.MatchedSelector == "" || len(assignment.Portfolios) == 0 {
+			continue
 		}
+		count++
 	}
 	return count
 }

core/estate/compiler.go

@@ -85,11 +85,12 @@ func Compile(
 			assignment.RolloutRing = selected.Assign.RolloutRing
 			assignment.MatchedSelector = selected.Selector.ID
 		} else {
-			if repository.Fork {
-				assignment.Portfolios = []string{owner.Classification.ForkPortfolio}
-			} else {
-				assignment.Portfolios = []string{owner.Classification.SourcePortfolio}
-			}
+			// A repository belongs to the account that holds it. Whether GitHub
+			// calls it a fork is a property of how it was created, not of who
+			// is responsible for it, and it used to send unmatched forks to a
+			// portfolio of their own. Owner classification is now the single
+			// fallback; `fork_portfolio` is deprecated and no longer read.
+			assignment.Portfolios = []string{owner.Classification.SourcePortfolio}
 		}
 		sort.Strings(assignment.Portfolios)
 		sort.Strings(assignment.PolicyProfiles)
@@ -121,9 +122,6 @@ func matchedSelectors(selectors []Selector, ownerID string, repository ObservedR
 			!matchesAnyNamePrefix(repository.Name, selector.Match.NamePrefixes) {
 			continue
 		}
-		if selector.Match.Fork != nil && *selector.Match.Fork != repository.Fork {
-			continue
-		}
 		if selector.Match.Archived != nil && *selector.Match.Archived != repository.Archived {
 			continue
 		}

core/estate/compiler_test.go

@@ -16,7 +16,7 @@ func TestLoadCanonicalControlledMutationEstate(t *testing.T) {
 	if config.Root.Rollout.MutationMode != "pull-request" ||
 		config.Root.Discovery.DefaultManagementMode != "observe-only" ||
 		len(config.Installations) != 5 || len(config.Mutations) != 4 ||
-		len(config.Owners) != 5 || len(config.Selectors) != 9 {
+		len(config.Owners) != 5 || len(config.Selectors) != 7 {
 		t.Fatalf("config = %#v", config)
 	}
 }
@@ -40,7 +40,6 @@ func TestCompileTwoThousandRepositoriesAndForksDeterministically(t *testing.T) {
 	if len(findings) != 0 || len(compiled.Repositories) != 2000 {
 		t.Fatalf("repositories=%d findings=%#v", len(compiled.Repositories), findings)
 	}
-	forks := 0
 	managed := 0
 	for index, assignment := range compiled.Repositories {
 		if assignment.ProviderID != int64(index+1000) ||
@@ -55,16 +54,22 @@ func TestCompileTwoThousandRepositoriesAndForksDeterministically(t *testing.T) {
 		} else if assignment.ManagementMode != "observe-only" {
 			t.Fatalf("observe-only assignment[%d] = %#v", index, assignment)
 		}
-		if assignment.MatchedSelector == "personal-forks" ||
-			assignment.MatchedSelector == "organization-forks" {
-			forks++
+		// Half the observations are forks. None of them is classified as
+		// one: a repository belongs to the account that holds it, so a fork
+		// lands in exactly the selector its non-fork sibling would.
+		if assignment.Owner == "example-user" && assignment.MatchedSelector != "personal-sources" {
+			t.Fatalf("personal assignment[%d] = %#v", index, assignment)
+		}
+		if assignment.Owner == "example-org" && assignment.MatchedSelector != "organization-sources" {
+			t.Fatalf("organization assignment[%d] = %#v", index, assignment)
 		}
 	}
-	if forks != 1000 {
-		t.Fatalf("fork assignments = %d, want 1000", forks)
-	}
-	if managed != 500 {
-		t.Fatalf("managed assignments = %d, want 500", managed)
+	// Every organization repository is managed now, forks included. That is
+	// the consequence of dropping fork classification: a managed account
+	// manages everything it holds, and a fork stops being a way to sit
+	// outside that. Half of these observations are forks.
+	if managed != 1000 {
+		t.Fatalf("managed assignments = %d, want 1000", managed)
 	}
 
 	for left, right := 0, len(repositories)-1; left < right; left, right = left+1, right-1 {
@@ -81,7 +86,7 @@ func TestCompileTwoThousandRepositoriesAndForksDeterministically(t *testing.T) {
 func TestCompileRejectsSelectorConflictAndUnknownOwner(t *testing.T) {
 	t.Parallel()
 	config := loadCanonical(t)
-	conflict := organizationForksSelector(t, config)
+	conflict := organizationSourcesSelector(t, config)
 	conflict.Selector.ID = "conflicting-selector"
 	config.Selectors = append(config.Selectors, conflict)
 	_, findings := Compile(config, []ObservedRepository{{
@@ -147,8 +152,9 @@ func TestCompileRoutesServerRepositoriesByNamePrefix(t *testing.T) {
 		len(got.Portfolios) != 1 || got.Portfolios[0] != "portfolio:servers" {
 		t.Fatalf("personal server repository = %#v", got)
 	}
-	if got := byID[13]; got.MatchedSelector != "organization-forks" ||
-		!containsString(got.Portfolios, "portfolio:forks") {
+	// The name prefix decides, and being a fork no longer overrides it.
+	if got := byID[13]; got.MatchedSelector != "organization-servers" ||
+		!containsString(got.Portfolios, "portfolio:servers") {
 		t.Fatalf("server-named organization fork repository = %#v", got)
 	}
 }
@@ -196,13 +202,13 @@ func TestCompilePreservesArchivedObservation(t *testing.T) {
 	}
 }
 
-func organizationForksSelector(t *testing.T, config Config) Selector {
+func organizationSourcesSelector(t *testing.T, config Config) Selector {
 	t.Helper()
 	for _, selector := range config.Selectors {
-		if selector.Selector.ID == "organization-forks" {
+		if selector.Selector.ID == "organization-sources" {
 			return selector
 		}
 	}
-	t.Fatalf("organization-forks selector not found in %#v", config.Selectors)
+	t.Fatalf("organization-sources selector not found in %#v", config.Selectors)
 	return Selector{}
 }

core/validation/estate.go

@@ -176,7 +176,7 @@ func (set *Set) ValidateEstateTree(root string) (EstateSummary, []domain.Finding
 	}
 
 	ownerByID := map[string]estateDocument{}
-	ownerPortfolio := map[string]map[bool]string{}
+	ownerPortfolio := map[string]string{}
 	for _, document := range owners {
 		identity := nestedObject(document.value, "owner")
 		id := stringField(identity, "id")
@@ -211,10 +211,7 @@ func (set *Set) ValidateEstateTree(root string) (EstateSummary, []domain.Finding
 			})
 		}
 		classification := nestedObject(document.value, "classification")
-		ownerPortfolio[id] = map[bool]string{
-			false: stringField(classification, "source_portfolio"),
-			true:  stringField(classification, "fork_portfolio"),
-		}
+		ownerPortfolio[id] = stringField(classification, "source_portfolio")
 	}
 
 	selectorIDs := map[string]estateDocument{}
@@ -237,7 +234,6 @@ func (set *Set) ValidateEstateTree(root string) (EstateSummary, []domain.Finding
 			})
 			continue
 		}
-		fork, hasFork := match["fork"].(bool)
 		for _, rawProfile := range arrayField(nestedObject(document.value, "assign"), "policy_profiles") {
 			profile, _ := rawProfile.(string)
 			if _, found := policyByID[profile]; !found {
@@ -248,10 +244,15 @@ func (set *Set) ValidateEstateTree(root string) (EstateSummary, []domain.Finding
 				})
 			}
 		}
-		if !hasFork || selectorHasSpecializedMatch(match) {
+		// A selector that narrows by name, visibility or lifecycle is
+		// deliberately routing a subset elsewhere. Every other selector is the
+		// owner's general one and must assign the portfolio the owner declares.
+		// This rule used to be keyed on `match.fork` being present, which meant
+		// dropping that field would have silently retired the check.
+		if selectorHasSpecializedMatch(match) {
 			continue
 		}
-		expectedPortfolio := ownerPortfolio[ownerID][fork]
+		expectedPortfolio := ownerPortfolio[ownerID]
 		assigned := arrayField(nestedObject(document.value, "assign"), "portfolios")
 		if !stringArrayContains(assigned, expectedPortfolio) {
 			findings = append(findings, domain.Finding{

core/validation/estate_test.go

@@ -17,7 +17,7 @@ func TestCanonicalEstateTreePasses(t *testing.T) {
 	summary, findings := set.ValidateEstateTree(repositoryRoot(t))
 	if len(findings) != 0 || summary.Installations != 5 || summary.Mutations != 4 ||
 		summary.Owners != 5 ||
-		summary.Selectors != 9 || summary.Devices != 3 {
+		summary.Selectors != 7 || summary.Devices != 3 {
 		t.Fatalf("summary=%#v findings=%#v", summary, findings)
 	}
 }
@@ -58,13 +58,13 @@ func TestEstateTreeRejectsCanonicalSelectorPortfolioMismatch(t *testing.T) {
 	sourceRoot := repositoryRoot(t)
 	copyEstateTree(t, filepath.Join(sourceRoot, "estate"), filepath.Join(root, "estate"))
 	copyEstateTree(t, filepath.Join(sourceRoot, "policies"), filepath.Join(root, "policies"))
-	selectorPath := filepath.Join(root, "estate", "selectors", "personal-forks.yaml")
+	selectorPath := filepath.Join(root, "estate", "selectors", "guild-sources.yaml")
 	raw, err := os.ReadFile(selectorPath)
 	if err != nil {
 		t.Fatal(err)
 	}
 	raw = []byte(strings.Replace(
-		string(raw), "portfolio:forks", "portfolio:personal-servers", 1,
+		string(raw), "portfolio:organization-projects", "portfolio:servers", 1,
 	))
 	if err := os.WriteFile(selectorPath, raw, 0o600); err != nil {
 		t.Fatal(err)
@@ -87,7 +87,7 @@ func TestEstateTreeRejectsUnknownSelectorOwner(t *testing.T) {
 	sourceRoot := repositoryRoot(t)
 	copyEstateTree(t, filepath.Join(sourceRoot, "estate"), filepath.Join(root, "estate"))
 	copyEstateTree(t, filepath.Join(sourceRoot, "policies"), filepath.Join(root, "policies"))
-	selectorPath := filepath.Join(root, "estate", "selectors", "personal-forks.yaml")
+	selectorPath := filepath.Join(root, "estate", "selectors", "personal-sources.yaml")
 	raw, err := os.ReadFile(selectorPath)
 	if err != nil {
 		t.Fatal(err)
@@ -318,7 +318,7 @@ func TestEstateTreeRejectsPolicyReferencesThatResolveToNothing(t *testing.T) {
 		},
 		{
 			name: "portfolio",
-			old:  `    - "portfolio:forks"`,
+			old:  `    - "portfolio:servers"`,
 			new:  `    - "portfolio:no-selector-assigns-this"`,
 			code: "GDS_ESTATE_POLICY_PORTFOLIO_MISSING",
 		},

estate/devices/example-user-mac2.yaml

@@ -16,17 +16,13 @@ device:
 
 workspace_roots:
   projects: "${HOME}/Developer/example-user"
-  forks: "${HOME}/Developer/forks"
 
 materialization:
   default_mode: "absent"
   include:
     - selector: "portfolio:personal-projects"
       workspace_root: "projects"
       mode: "active"
-    - selector: "portfolio:forks"
-      workspace_root: "forks"
-      mode: "active"
 
 harnesses:
   - "claude-code"

estate/devices/example-user-ubuntu-1.yaml

@@ -17,17 +17,13 @@ device:
 
 workspace_roots:
   projects: "${HOME}/Developer/projects"
-  forks: "${HOME}/Developer/forks"
 
 materialization:
   default_mode: "absent"
   include:
     - selector: "portfolio:organization-projects"
       workspace_root: "projects"
       mode: "active"
-    - selector: "portfolio:forks"
-      workspace_root: "forks"
-      mode: "active"
 
 harnesses:
   - "claude-code"