diff --git a/docs/CEILINGS.md b/docs/CEILINGS.md index 97dd24e1..5f9f5b23 100644 --- a/docs/CEILINGS.md +++ b/docs/CEILINGS.md @@ -295,3 +295,22 @@ another option also does. On Linux it covers `run` and `create` only. Replace this when apple/container#1449 lands: set the property in setup and drop the injection. + +## Cwd as the owner of busy unshaped work + +`internal/admission/uncovered.go` attributes uncovered heavy work to a +repository by the process's cwd. A process that matches no build, test or +install shape and is caught only by the CPU rule no longer blocks admission as +unknown ownership when its cwd is observed to lie outside every repository +(#294): an app launched from `/`, or a tool's state directory such as +claude-mem's observer sessions. A job whose cwd could not be observed, or whose +resolution failed for any other reason, still blocks. + +So unshaped work that builds or tests a repository from a directory outside it, +such as a Gradle daemon under `~/.gradle`, a VM process running a container +build, or a script runner started from `~`, is not deferred. A single such +process can also stay under the pressure thresholds on a many-core host. Native +compiler children (`rustc`, `clang`, `swift-frontend`, `ld`) are still +name-shaped and still block. Revisit this when such unshaped builds cause a +missed deferral: attribute work through the process tree's repository +arguments, or add the tool's name to the shape rules. diff --git a/docs/releases/v2.70.49.md b/docs/releases/v2.70.49.md new file mode 100644 index 00000000..f1ba1947 --- /dev/null +++ b/docs/releases/v2.70.49.md @@ -0,0 +1,6 @@ +`dot admit` no longer defers your builds and tests because of a busy app or background tool that is not working in any repository. + +### Fixed +- `dot admit` deferred heavy work with "uncovered heavyweight work ... repo=" whenever a busy desktop app, a system service or a background tool (for example a menu-bar app's own commands, or claude-mem's observer) was running outside every repository. Such a process now counts only as host load. Builds, tests and installs outside a repository, and processes whose working directory cannot be read, still defer as before. (#295) + +**Full changelog**: https://github.com/STAIxBWLB/dotfiles/compare/v2.70.48...v2.70.49 diff --git a/internal/admission/uncovered.go b/internal/admission/uncovered.go index 395652fa..11873f92 100644 --- a/internal/admission/uncovered.go +++ b/internal/admission/uncovered.go @@ -10,6 +10,7 @@ import ( "bufio" "bytes" "context" + "errors" "fmt" "os" osexec "os/exec" @@ -28,6 +29,10 @@ type HeavyJob struct { Directory string `json:"directory,omitempty"` Repository string `json:"repository,omitempty"` Maintenance bool `json:"maintenance,omitempty"` + // CPUOnly: only the CPU rule matched, not a build, test or install shape. + CPUOnly bool `json:"cpu_only,omitempty"` + // OutsideRepository: the cwd was observed and lies outside every repository. + OutsideRepository bool `json:"outside_repository,omitempty"` } // Describe renders a job for a defer reason. @@ -56,7 +61,12 @@ func FindUncovered(ctx context.Context, projectDir string, maintenance bool, lea func filterUncovered(jobs []HeavyJob, project string, maintenance bool) []string { var out []string for _, j := range jobs { - if j.Repository == "" || (!maintenance && j.Repository == project) || (maintenance && j.Maintenance) { + // A busy process with no heavy shape, in no repository, is host load, not + // work of unknown ownership (#294). + // ponytail: known ceiling. See docs/CEILINGS.md (cwd as the owner of busy unshaped work). + unknown := j.Repository == "" && (!j.CPUOnly || !j.OutsideRepository) + sameRepository := j.Repository != "" && j.Repository == project + if unknown || (!maintenance && sameRepository) || (maintenance && j.Maintenance) { out = append(out, j.Describe()) } } @@ -144,8 +154,9 @@ func unownedHeavyJobs(rows []processRow, parents map[int]int, self int, leased [ if ancestors[r.pid] || r.ppid == self || underLease(r.pid) { continue } - if heavyweight(r.name, r.args) || r.cpu >= 100 { - found = append(found, HeavyJob{PID: r.pid, ParentPID: r.ppid, Name: r.name, Maintenance: containsWord(r.args, "install", "update", "upgrade", "sync", "ci")}) + shaped := heavyweight(r.name, r.args) + if shaped || r.cpu >= 100 { + found = append(found, HeavyJob{PID: r.pid, ParentPID: r.ppid, Name: r.name, CPUOnly: !shaped, Maintenance: containsWord(r.args, "install", "update", "upgrade", "sync", "ci")}) if len(found) > 20 { return nil, fmt.Errorf("heavy process inventory exceeds 20 jobs; ownership cannot be established safely") } @@ -250,18 +261,24 @@ func resolveJobDirectories(ctx context.Context, jobs []HeavyJob) []HeavyJob { jobs[i].Directory = cwd[jobs[i].PID] } } - cache := map[string]string{} + type identity struct { + repo string + outside bool + } + cache := map[string]identity{} for i := range jobs { dir := jobs[i].Directory if dir == "" { continue } - repo, ok := cache[dir] + id, ok := cache[dir] if !ok { - repo, _ = repositoryIdentity(dir) - cache[dir] = repo + repo, err := repositoryIdentity(dir) + id = identity{repo: repo, outside: errors.Is(err, errNotRepository)} + cache[dir] = id } - jobs[i].Repository = repo + jobs[i].Repository = id.repo + jobs[i].OutsideRepository = id.outside } return jobs } @@ -286,6 +303,10 @@ func lsofDirectories(out string, probeErr error) map[int]string { return cwd } +// errNotRepository: the walk reached the filesystem root without a .git. Any +// other resolution failure leaves ownership unknown. +var errNotRepository = errors.New("not inside a Git repository") + // repositoryIdentity resolves dir to its repository's common Git directory // with bounded filesystem reads, never Git subprocesses, so every worktree of // one repository has the same identity. @@ -332,11 +353,11 @@ func repositoryIdentity(dir string) (string, error) { } parent := filepath.Dir(current) if parent == current { - break + return "", fmt.Errorf("%w: %s", errNotRepository, dir) } current = parent } - return "", fmt.Errorf("repository ownership unknown for %s", dir) + return "", fmt.Errorf("repository ownership unknown for %s: deeper than 64 levels", dir) } // readGitPointer reads a Git indirection file: a single short path, never diff --git a/internal/admission/uncovered_test.go b/internal/admission/uncovered_test.go index 36a04eff..d36af78f 100644 --- a/internal/admission/uncovered_test.go +++ b/internal/admission/uncovered_test.go @@ -2,8 +2,11 @@ package admission import ( "context" + "errors" "os" + "os/exec" "path/filepath" + "runtime" "strings" "testing" "time" @@ -207,3 +210,117 @@ func TestBackgroundSyncLeaseCoversItsRsyncChild(t *testing.T) { } } } + +// A busy process with no build or test shape, whose cwd is known and lies +// outside every repository, is host load: it never blocks as unknown ownership +// (#294). Everything else that is unknown still blocks. +func TestCPUOnlyWorkOutsideRepositoriesDoesNotBlock(t *testing.T) { + for _, tc := range []struct { + name string + job HeavyJob + maintenance bool + want int + }{ + {"busy app process outside any repository", HeavyJob{PID: 1, CPUOnly: true, OutsideRepository: true}, false, 0}, + {"busy process with unobserved cwd", HeavyJob{PID: 2, CPUOnly: true}, false, 1}, + {"build-shaped work outside any repository", HeavyJob{PID: 3, OutsideRepository: true}, false, 1}, + {"maintenance-shaped busy process outside any repository", HeavyJob{PID: 4, CPUOnly: true, OutsideRepository: true, Maintenance: true}, true, 1}, + {"busy process outside any repository, maintenance class", HeavyJob{PID: 5, CPUOnly: true, OutsideRepository: true}, true, 0}, + } { + if got := filterUncovered([]HeavyJob{tc.job}, "/a/.git", tc.maintenance); len(got) != tc.want { + t.Errorf("%s: blockers=%v, want %d", tc.name, got, tc.want) + } + } +} + +func TestRepositoryIdentityReportsNotRepository(t *testing.T) { + if _, err := repositoryIdentity(tempOutsideRepositories(t)); !errors.Is(err, errNotRepository) { + t.Fatalf("plain directory: err=%v, want errNotRepository", err) + } + broken := t.TempDir() + if err := os.WriteFile(filepath.Join(broken, ".git"), []byte("not a pointer\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := repositoryIdentity(broken); err == nil || errors.Is(err, errNotRepository) { + t.Fatalf("invalid Git pointer: err=%v, want an unknown-ownership error", err) + } +} + +// tempOutsideRepositories returns a temporary directory, or skips when the +// host's TMPDIR itself lies inside a repository (some agent sandboxes). +func tempOutsideRepositories(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if _, err := repositoryIdentity(dir); !errors.Is(err, errNotRepository) { + t.Skipf("TMPDIR %s is not outside every repository: %v", dir, err) + } + return dir +} + +// spawnSleeper starts a child whose cwd is dir and stops it at cleanup. +func spawnSleeper(t *testing.T, dir string) int { + t.Helper() + c := exec.Command("/bin/sleep", "30") + c.Dir = dir + if err := c.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = c.Process.Kill(); _ = c.Wait() }) + return c.Process.Pid +} + +func TestResolveJobDirectoriesMarksOutsideRepository(t *testing.T) { + if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.Skip("cwd resolution is implemented for linux and darwin") + } + outside := spawnSleeper(t, tempOutsideRepositories(t)) + gone := t.TempDir() + removed := spawnSleeper(t, gone) + if err := os.Remove(gone); err != nil { + t.Fatal(err) + } + jobs := resolveJobDirectories(context.Background(), []HeavyJob{{PID: os.Getpid()}, {PID: outside}, {PID: removed}}) + if jobs[0].Repository == "" || jobs[0].OutsideRepository { + t.Errorf("own process (in this repository): %+v, want a repository", jobs[0]) + } + if !jobs[1].OutsideRepository || jobs[1].Repository != "" { + t.Errorf("child in a plain temporary directory: %+v, want OutsideRepository", jobs[1]) + } + if jobs[2].OutsideRepository { + t.Errorf("child whose cwd was removed: %+v, want unknown ownership, not outside", jobs[2]) + } +} + +func TestRepositoryIdentityMissingPathIsUnknown(t *testing.T) { + if _, err := repositoryIdentity(filepath.Join(t.TempDir(), "missing")); err == nil || errors.Is(err, errNotRepository) { + t.Fatalf("nonexistent path: err=%v, want an unknown-ownership error", err) + } +} + +// A caller outside every repository has project "": an exempted job must not +// match it as "the same repository" (#294 D5). +func TestExemptJobDoesNotMatchAnEmptyProject(t *testing.T) { + exempt := HeavyJob{PID: 7, CPUOnly: true, OutsideRepository: true} + if got := filterUncovered([]HeavyJob{exempt}, "", false); len(got) != 0 { + t.Fatalf("caller outside every repository blocked on exempt work: %v", got) + } + if got := filterUncovered([]HeavyJob{{PID: 8, CPUOnly: true}}, "", false); len(got) != 1 { + t.Fatalf("unknown ownership no longer blocks a caller outside every repository: %v", got) + } +} + +func TestUnownedHeavyJobsMarksCPUOnly(t *testing.T) { + rows, parents, err := parseProcessTable("" + + " 400 1 150.0 /Users/x/.local/bin/claude --max-turns=1 --model=fast\n" + + " 401 1 150.0 /opt/homebrew/bin/cargo build --release\n") + if err != nil { + t.Fatal(err) + } + jobs, err := unownedHeavyJobs(rows, parents, 999, nil) + if err != nil || len(jobs) != 2 { + t.Fatalf("jobs=%+v err=%v", jobs, err) + } + if !jobs[0].CPUOnly || jobs[1].CPUOnly { + t.Fatalf("CPUOnly: claude=%t cargo=%t, want true and false", jobs[0].CPUOnly, jobs[1].CPUOnly) + } +} diff --git a/internal/syncer/ceilings_doc_test.go b/internal/syncer/ceilings_doc_test.go index c539f951..0cc637b4 100644 --- a/internal/syncer/ceilings_doc_test.go +++ b/internal/syncer/ceilings_doc_test.go @@ -72,6 +72,7 @@ func TestCeilingsDoc(t *testing.T) { "internal/syncer/peer_commands.go", "internal/config/state.go", "internal/syncer/sync_cmd_ops.go", + "internal/admission/uncovered.go", } { if !strings.Contains(doc, path) { t.Errorf("document does not name %q", path) @@ -95,6 +96,7 @@ func TestCeilingsDoc(t *testing.T) { {"internal/syncer/gitrescue.go", "ponytail: known ceiling. See docs/CEILINGS.md (rewrite twins by author, date and subject)."}, {"internal/container/translate.go", "ponytail: known ceiling. See docs/CEILINGS.md (container translation table)."}, {"internal/container/host.go", "ponytail: known ceiling. See docs/CEILINGS.md (container DNS injection)."}, + {"internal/admission/uncovered.go", "ponytail: known ceiling. See docs/CEILINGS.md (cwd as the owner of busy unshaped work)."}, } { data, err := os.ReadFile(filepath.Join("..", "..", want.path)) if err != nil {