refactor(engine): groups 的上游复制段归零

engine/UPSTREAM.md

@@ -31,25 +31,25 @@
 **9,621**（对照 `f4c50506`）、**9,659**（对照 `be48afbe`），生产 3,852、
 测试 5,758。
 
-2026-11-12 的 ports 与 killer 重写在此基础上继续下降：对照 `26dbcae` 为 **8,689**
-行 / 636 段，分布在 131 个文件（生产 3,478、测试 5,211）。
+2026-11-12 的 ports、killer 与 groups 重写在此基础上继续下降：对照 `26dbcae` 为
+**7,847** 行 / 535 段，分布在 100 个文件（生产 3,142、测试 4,705）。
 
-按包（对照 `26dbcae`，ports 与 killer 重写后）：
+按包（对照 `26dbcae`，ports、killer 与 groups 重写后）：
 
 | 包 | 生产 | 测试 | 合计 |
 | --- | ---: | ---: | ---: |
 | `internal/daemon` | 2,073 | 3,457 | 5,530 |
 | `internal/cmd` | 837 | 703 | 1,540 |
-| `internal/groups` | 336 | 506 | 842 |
+| `internal/groups` | 0 | 0 | 0 |
 | `internal/scanner` | 220 | 540 | 760 |
 | `internal/ports` | 0 | 0 | 0 |
 | `internal/killer` | 0 | 0 | 0 |
 | `internal/store` | 12 | 5 | 17 |
 
-`internal/claims`、`internal/display`、`internal/testenv`、`internal/ports`、
-`internal/killer` 与 `internal/store/claims*.go` 已经归零；`internal/docker` 只剩
-结构体字段与表驱动用例数据。`internal/store` 已整包重写：生产代码从 1,076 行降到
-199 行。
+`internal/claims`、`internal/display`、`internal/testenv`、`internal/groups`、
+`internal/ports`、`internal/killer` 与 `internal/store/claims*.go` 已经归零；
+`internal/docker` 只剩结构体字段与表驱动用例数据。`internal/store` 已整包重写：
+生产代码从 1,076 行降到 199 行。
 
 ```sh
 git clone https://github.com/raskrebs/sonar /tmp/sonar

engine/internal/groups/attribute.go

@@ -7,19 +7,17 @@ import (
 	"github.com/sheathedsharp/option-berth/engine/internal/state"
 )
 
-// Attribute runs the resolver over a direct scan. It builds an index from what
-// the scan saw — every `oberth.yaml` above a process cwd, every Compose
-// project's working directory, plus the config in the caller's own working
-// directory — resolves the group of every port, writes Group, GroupSource and
-// ProjectRoot back onto the scanner rows, and returns the resolved contract
-// rows together with the index.
+// Attribute resolves groups for a direct scan. The index it builds covers what
+// the scan itself saw — each `oberth.yaml` above a process cwd, each Compose
+// project's working directory — plus the config in the caller's own directory.
+// Every port gets Group, GroupSource and ProjectRoot written back onto its
+// scanner row; the returned slice is the contract rows and the index.
 //
-// This is the no-daemon path: `oberth status`, `oberth kill` and `oberth init`
-// call it. Run ownership is resolved here too, from runs.json and the process
-// tree (see DirectRuns): the collection layer reports what the OS shows, and
-// this layer decides which worktree, service and run it belongs to. The
-// daemon's scanner calls AttributeWith instead, with a long-lived index and
-// the `oberth start` registry.
+// `oberth status`, `oberth kill` and `oberth init` take this no-daemon path.
+// Run ownership is settled here too, from runs.json and the ancestry walk
+// (DirectRuns): the collector reports what the OS shows, this layer decides
+// the worktree, service and run. The daemon's scanner calls AttributeWith
+// instead, with a long-lived index and the `oberth start` registry.
 func Attribute(pp []ports.ListeningPort) ([]state.Port, *Index) {
 	index := NewIndex()
 	if wd, err := os.Getwd(); err == nil {
@@ -100,11 +98,11 @@ func deref[T ~string](s *T) string {
 	return string(*s)
 }
 
-// stampSessions puts the agent session of the owning run onto every port it
-// owns, from the same registry and the same ancestry walk that decided the
-// run. It runs after Resolve because it stamps the contract rows, not the
-// scanner rows: `session` is a daemon concept with no place in `runs.json`, so
-// the direct-scan path has nothing to stamp and this is a no-op there.
+// stampSessions writes the owning run's agent session onto every port that run
+// owns. The registry and the ancestry walk are the same ones that decided the
+// run, and the stamp lands on contract rows rather than scanner rows — which
+// is why it runs after Resolve. `session` is a daemon concept that runs.json
+// does not carry, so the direct-scan path has nothing to stamp.
 func stampSessions(resolved []state.Port, runs Registry) {
 	registry, ok := runs.(SessionRegistry)
 	if !ok {

engine/internal/groups/attribute_session_test.go

@@ -8,18 +8,18 @@ import (
 	"github.com/sheathedsharp/option-berth/engine/internal/state"
 )
 
-// sessionRunSet is a Registry that also implements SessionRegistry, which is
-// what the daemon's run registry looks like once a run carries a session.
+// sessionRunSet is a Registry that also implements SessionRegistry: the shape
+// the daemon's run registry takes once a run carries a session.
 type sessionRunSet struct {
 	runSet
 	session state.Session
 }
 
 func (s sessionRunSet) Session(p state.Port) (state.Session, bool) {
-	if p.Port != s.port {
-		return state.Session{}, false
+	if p.Port == s.port {
+		return s.session, true
 	}
-	return s.session, true
+	return state.Session{}, false
 }
 
 func requireResolvedPort(t *testing.T, got, want state.Port) {
@@ -31,8 +31,8 @@ func requireResolvedPort(t *testing.T, got, want state.Port) {
 
 func TestAttributeWithStampsTheSession(t *testing.T) {
 	want := state.Session{
-		ID: "claude-code:abc", Tool: "claude-code",
-		Worktree: "feature-x", Branch: "feature/x", Detected: true,
+		ID: "claude-code:def", Tool: "claude-code",
+		Worktree: "feature-y", Branch: "feature/y", Detected: true,
 	}
 	pp := []ports.ListeningPort{
 		{Port: 3000, PID: 11, Process: "node", Command: "node server.js"},

engine/internal/groups/attribute_test.go

@@ -8,19 +8,22 @@ import (
 	"github.com/sheathedsharp/option-berth/engine/internal/state"
 )
 
-// runSet is a Registry that answers for one port number.
+// runSet is a Registry holding the single run these tests know about, reachable
+// through one port number.
 type runSet struct {
 	port  int
-	id    string
 	group string
+	id    string
 	name  string
 }
 
 func (r runSet) Run(p state.Port) (state.Run, bool) {
 	if p.Port != r.port {
 		return state.Run{}, false
 	}
-	return state.Run{ID: r.id, Group: r.group, Name: r.name, RootPID: p.PID}, true
+	run := state.Run{ID: r.id, Group: r.group, Name: r.name}
+	run.RootPID = p.PID
+	return run, true
 }
 
 func requireAttributedPort(t *testing.T, got, want state.Port) {

engine/internal/groups/build.go

@@ -108,9 +108,8 @@ func GroupsWith(pp []state.Port, index *Index, reg Registry, narrow bool) []stat
 		// here too, not a second group with the main checkout's name.
 		name := pass.groupOf(cfg)
 		g := table.get(name)
-		path, dir := cfg.Path, cfg.Dir
-		g.ConfigPath = &path
-		g.RootDir = &dir
+		configPath, rootDir := cfg.Path, cfg.Dir
+		g.ConfigPath, g.RootDir = &configPath, &rootDir
 		if rank(state.SourceFile) > rank(g.Source) {
 			g.Source = state.SourceFile
 		}
@@ -208,10 +207,9 @@ func (r *resolutionPass) describeCheckout(g *state.Group) {
 	}
 }
 
-// ServiceRow adapts one `oberth.yaml` service to the published contract row,
-// without any knowledge of what is running. `groups.config.get` returns the
-// file through it, so what the editor reads back is exactly what the resolver
-// publishes.
+// ServiceRow adapts one `oberth.yaml` service into the published contract row
+// without asking what is running. `groups.config.get` reads the file back
+// through it, so the editor sees exactly what the resolver publishes.
 func ServiceRow(s Service) state.Service {
 	svc := state.Service{
 		Name:          s.Name,
@@ -247,20 +245,20 @@ func nonNilStrings(in []string) []string {
 
 // ServiceRows adapts a whole config's services list.
 func ServiceRows(cfg *Config) []state.Service {
-	out := make([]state.Service, 0, len(cfg.Services))
+	rows := make([]state.Service, 0, len(cfg.Services))
 	for _, s := range cfg.Services {
-		out = append(out, ServiceRow(s))
+		rows = append(rows, ServiceRow(s))
 	}
-	return out
+	return rows
 }
 
 // optional turns an empty metadata string into the contract's null.
 func optional(s string) *string {
-	if s == "" {
-		return nil
+	if s != "" {
+		v := s
+		return &v
 	}
-	v := s
-	return &v
+	return nil
 }
 
 // machineRows renders a config's `machine:` references against what is
@@ -282,9 +280,9 @@ func machineRows(cfg *Config, ports map[int]struct{}, narrow bool) []state.Machi
 	return out
 }
 
-// status is running when everything the group declares is up, stopped when
-// nothing is, and partial in between. A group with no services is running as
-// long as it has a listening port.
+// status is running when every declared service is up, stopped when none is,
+// and partial otherwise; a group that declares no services is running while it
+// has a listening port.
 func status(g state.Group) string {
 	declared := len(g.Services)
 	if declared == 0 {

engine/internal/groups/build_exit_test.go

@@ -29,6 +29,15 @@ func declaredService(s Service) state.Service {
 	return row
 }
 
+// shopConfig is the manifest these tests publish: the services they care about,
+// in a project called "shop" that exists on no disk.
+func shopConfig(services ...Service) *Config {
+	return &Config{
+		Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName,
+		Services: services,
+	}
+}
+
 func shopGroup(status string, running int, services ...state.Service) []state.Group {
 	root := "/nowhere/shop"
 	cfgPath := "/nowhere/shop/" + ConfigName
@@ -47,31 +56,25 @@ func requireGroups(t *testing.T, got, want []state.Group) {
 	}
 }
 
-// TestGroupsCarryTheLastExitOfAServiceThatIsDown: a stopped service says how
+// TestGroupsCarryTheLastExitOfAServiceThatIsDown: a stopped service reports how
 // it ended, so a client can show "crashed" rather than only "not running".
 func TestGroupsCarryTheLastExitOfAServiceThatIsDown(t *testing.T) {
-	index := NewIndex()
-	index.Add(&Config{
-		Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName,
-		Services: []Service{{Name: "api", Cmd: "run-api"}, {Name: "web", Cmd: "run-web"}},
-	})
+	idx := NewIndex()
+	idx.Add(shopConfig(Service{Name: "api", Cmd: "run-api"}, Service{Name: "web", Cmd: "run-web"}))
 
-	gg := GroupsWith(nil, index, exitRegistry{}, false)
+	built := GroupsWith(nil, idx, exitRegistry{}, false)
 	api := declaredService(Service{Name: "api", Cmd: "run-api"})
 	api.LastExit = &state.ServiceExit{Code: 1, Reason: "crashed", At: "2026-09-13T10:00:00Z", RunID: "a"}
-	requireGroups(t, gg, shopGroup("stopped", 0, api, declaredService(Service{Name: "web", Cmd: "run-web"})))
+	requireGroups(t, built, shopGroup("stopped", 0, api, declaredService(Service{Name: "web", Cmd: "run-web"})))
 }
 
-// TestGroupsWithoutARegistryHaveNoExits: the direct-scan path has no registry
-// at all, and must still build its groups.
+// TestGroupsWithoutARegistryHaveNoExits: the direct-scan path has no registry at
+// all, and must still build its groups.
 func TestGroupsWithoutARegistryHaveNoExits(t *testing.T) {
-	index := NewIndex()
-	index.Add(&Config{
-		Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName,
-		Services: []Service{{Name: "api", Cmd: "run-api"}},
-	})
-	gg := Groups(nil, index)
-	requireGroups(t, gg, shopGroup("stopped", 0, declaredService(Service{Name: "api", Cmd: "run-api"})))
+	idx := NewIndex()
+	idx.Add(shopConfig(Service{Name: "api", Cmd: "run-api"}))
+	built := Groups(nil, idx)
+	requireGroups(t, built, shopGroup("stopped", 0, declaredService(Service{Name: "api", Cmd: "run-api"})))
 }
 
 // liveRegistry knows which services option-berth has a live run of.
@@ -94,10 +97,7 @@ func (liveRegistry) ServiceFacts() servicefacts.Snapshot {
 // port for it would be a different lie.
 func TestGroupsSeeAServiceThatHoldsNoPort(t *testing.T) {
 	index := NewIndex()
-	index.Add(&Config{
-		Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName,
-		Services: []Service{{Name: "worker", Cmd: "run-worker"}, {Name: "api", Cmd: "run-api"}},
-	})
+	index.Add(shopConfig(Service{Name: "worker", Cmd: "run-worker"}, Service{Name: "api", Cmd: "run-api"}))
 
 	gg := GroupsWith(nil, index, liveRegistry{}, false)
 	worker := declaredService(Service{Name: "worker", Cmd: "run-worker"})
@@ -119,10 +119,7 @@ func (driftRegistry) ServiceFacts() servicefacts.Snapshot {
 
 func TestGroupsExposeManifestRuntimeMismatch(t *testing.T) {
 	index := NewIndex()
-	index.Add(&Config{
-		Name: "shop", Dir: "/nowhere/shop", Path: "/nowhere/shop/" + ConfigName,
-		Services: []Service{{Name: "api", Cmd: "old-api"}},
-	})
+	index.Add(shopConfig(Service{Name: "api", Cmd: "old-api"}))
 	gg := GroupsWith(nil, index, driftRegistry{}, false)
 	api := declaredService(Service{Name: "api", Cmd: "old-api"})
 	api.Running = true

engine/internal/groups/build_test.go

@@ -14,19 +14,19 @@ import (
 
 func TestGroupsFromPortsOnly(t *testing.T) {
 	f := newFixture(t)
-	index := NewIndex()
-	index.Observe(f.repoSub)
+	idx := NewIndex()
+	idx.Observe(f.repoSub)
 
-	pp := Resolve([]state.Port{
+	ports := Resolve([]state.Port{
 		nativePort(8123, f.repoSub),
 		nativePort(8124, f.repo),
 		nativePort(3000, ""),
-	}, NoRuns{}, index)
-	if len(pp) != 3 {
-		t.Fatalf("Resolve returned %d ports, want 3", len(pp))
+	}, NoRuns{}, idx)
+	if len(ports) != 3 {
+		t.Fatalf("Resolve returned %d ports, want 3", len(ports))
 	}
 
-	requireOneGroup(t, Groups(pp, index), state.Group{
+	requireOneGroup(t, Groups(ports, idx), state.Group{
 		Name: "option-berth", Repo: "option-berth",
 		Identity: WorktreeToken(f.repo), Source: state.SourceAuto,
 		RootDir: &f.repo, Status: "running", Members: []int{8123, 8124},
@@ -83,7 +83,7 @@ func TestGroupsStatusFromServices(t *testing.T) {
 	webHash := "e3hr8249vm80p51bph2fn45bl5ll5u7j72ti7e3gk8dsnludrlb0"
 	workerHash := "qebi5bgj4l1b94b9nnd6330hrsntb3kvplijkhqle2ctmpeqvf70"
 
-	tests := []struct {
+	cases := []struct {
 		name             string
 		listening        []state.Port
 		status           string
@@ -116,16 +116,16 @@ func TestGroupsStatusFromServices(t *testing.T) {
 			worker: publishedService("worker", nil, nil, nil, workerHash, true, intPtr(9000)),
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			index := NewIndex()
-			index.Observe(f.repo)
-			requireOneGroup(t, Groups(Resolve(tt.listening, NoRuns{}, index), index), state.Group{
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			idx := NewIndex()
+			idx.Observe(f.repo)
+			requireOneGroup(t, Groups(Resolve(tc.listening, NoRuns{}, idx), idx), state.Group{
 				Name: "option-berth", Repo: "option-berth",
 				Identity: WorktreeToken(f.repo), Source: state.SourceFile,
-				RootDir: &f.repo, ConfigPath: &config, Status: tt.status,
-				Members: tt.members, Services: []state.Service{tt.api, tt.web, tt.worker},
-				Machine: []state.MachineRef{}, RunningServices: tt.running,
+				RootDir: &f.repo, ConfigPath: &config, Status: tc.status,
+				Members: tc.members, Services: []state.Service{tc.api, tc.web, tc.worker},
+				Machine: []state.MachineRef{}, RunningServices: tc.running,
 			})
 		})
 	}
@@ -230,23 +230,23 @@ func requireOneGroup(t *testing.T, got []state.Group, want state.Group) {
 
 func compileDefinition(t *testing.T, name string) *jsonschema.Schema {
 	t.Helper()
-	path := filepath.Join("..", "..", "docs", "schema", "protocol.schema.json")
-	f, err := os.Open(path)
-	if err != nil {
-		t.Fatalf("%v (run `go generate ./...`)", err)
+	schemaPath := filepath.Join("..", "..", "docs", "schema", "protocol.schema.json")
+	file, openErr := os.Open(schemaPath)
+	if openErr != nil {
+		t.Fatalf("%v (run `go generate ./...`)", openErr)
 	}
-	defer f.Close()
-	doc, err := jsonschema.UnmarshalJSON(f)
-	if err != nil {
-		t.Fatal(err)
+	defer file.Close()
+	document, parseErr := jsonschema.UnmarshalJSON(file)
+	if parseErr != nil {
+		t.Fatalf("protocol.schema.json: %v", parseErr)
 	}
-	c := jsonschema.NewCompiler()
-	if err := c.AddResource("protocol.schema.json", doc); err != nil {
-		t.Fatal(err)
+	compiler := jsonschema.NewCompiler()
+	if addErr := compiler.AddResource("protocol.schema.json", document); addErr != nil {
+		t.Fatalf("add protocol.schema.json: %v", addErr)
 	}
-	sch, err := c.Compile("protocol.schema.json#/definitions/" + name)
-	if err != nil {
-		t.Fatal(err)
+	sch, compileErr := compiler.Compile("protocol.schema.json#/definitions/" + name)
+	if compileErr != nil {
+		t.Fatalf("compile %s: %v", name, compileErr)
 	}
 	return sch
 }