From 6442917de600b84dfeb1eb4cc7a6fabf423135b2 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 06:04:25 -0400 Subject: [PATCH 1/6] session: a git status with no directory is refused where it is built git -C "" is a no-op, so worktreeDirtIn and UnsavedEditsNote built their status command by hand and ran it in the process's own directory when the directory was empty, past the refusal in gitWith. Both go through gitWith now, and gitWith is a thin wrapper of a new gitContext that carries the caller's own context; the empty-directory refusal and the pinned environment live in that one place. Co-Authored-By: codeaf --- internal/session/task_run.go | 19 ++++++++++++----- internal/session/taskgitdir_test.go | 32 +++++++++++++++++++++++++++++ internal/session/taskpreflight.go | 5 ++--- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/internal/session/task_run.go b/internal/session/task_run.go index f3ef6300ef..533f28b916 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -6596,15 +6596,15 @@ func worktreeDirt(dir string) string { return worktreeDirtIn(context.Background( // worktreeDirtIn is [worktreeDirt] under a context, for a reading somebody may // have to stop ([treeWatch.close]). func worktreeDirtIn(ctx context.Context, dir string) string { - args := []string{"-C", dir, "--no-optional-locks", "status", "--porcelain", "--untracked-files=all", "--", "."} + args := []string{"--no-optional-locks", "status", "--porcelain", "--untracked-files=all", "--", "."} for _, dropping := range taskDroppingNames() { args = append(args, ":(exclude)"+dropping) } - out, err := exec.CommandContext(ctx, "git", args...).Output() + out, err := gitContext(ctx, dir, nil, args...) if err != nil { return "" } - sum := sha256.Sum256(out) + sum := sha256.Sum256([]byte(out)) return string(sum[:8]) } @@ -8959,13 +8959,22 @@ func git(dir string, args ...string) (string, error) { return gitWith(dir, nil, args...) } -// gitWith is [git] with something extra in its environment, and it exists for +// gitWith is [gitContext] with [context.Background] in place of a caller's own +// context, kept as its own name because nearly every call here is that. // exactly one caller: the ground ladder stages a parent's tree into AN INDEX OF // ITS OWN so that handing out a child never moves the parent's index // ([sealGroundWork]). GIT_INDEX_FILE is the only way to say that to git, and a // second copy of the pager and editor settings beside it would be the drift the // one-source-of-truth law forbids. func gitWith(dir string, environment []string, args ...string) (string, error) { + return gitContext(context.Background(), dir, environment, args...) +} + +// gitContext is [gitWith] under a context, for a command somebody may have to +// stop before git answers ([worktreeDirtIn]'s watch). The refusal of an empty +// directory and the pinned environment live here, so there is one copy of +// each; every other spelling of a git call in this package is a wrapper of it. +func gitContext(ctx context.Context, dir string, environment []string, args ...string) (string, error) { // A COMMAND WITH NO DIRECTORY RUNS WHEREVER THE PROCESS HAPPENS TO BE. // // exec.Cmd reads an empty Dir as "the calling process's working directory", @@ -8984,7 +8993,7 @@ func gitWith(dir string, environment []string, args ...string) (string, error) { if strings.TrimSpace(dir) == "" { return "", errors.New("git: no directory to run in") } - command := exec.Command("git", args...) + command := exec.CommandContext(ctx, "git", args...) command.Dir = dir // A pager or an editor in the middle of a merge would hang a node forever on // a terminal it does not have. diff --git a/internal/session/taskgitdir_test.go b/internal/session/taskgitdir_test.go index 3ed374555a..96607cae52 100644 --- a/internal/session/taskgitdir_test.go +++ b/internal/session/taskgitdir_test.go @@ -1,6 +1,7 @@ package session import ( + "context" "os" "path/filepath" "strings" @@ -17,6 +18,37 @@ import ( // wrote "task: Rewrite", "task: Measure" and "task: Paint" onto the branch of // the worktree it was launched from. Every call site names a directory, so one // that answers "" is a defect, and it fails here rather than somewhere else. +// TestStatusReadsWithNoDirectoryNeverRuns is the same proof for the two call +// sites that built `git -C ` themselves instead of asking [gitWith]: +// [worktreeDirtIn]'s dirty fingerprint and [UnsavedEditsNote]'s status reading. +// `git -C ""` is a no-op — git runs where the process is standing — so a direct +// exec.Command with an empty directory slipped past the refusal in gitWith. +func TestStatusReadsWithNoDirectoryNeverRuns(t *testing.T) { + repo := t.TempDir() + for _, args := range [][]string{{"init", "-q", "-b", "main"}, {"config", "user.name", "codeaf"}} { + if _, err := git(repo, args...); err != nil { + t.Skipf("no usable git here: %v", err) + } + } + writeFile(t, filepath.Join(repo, "note.md"), "work\n") + t.Chdir(repo) + + if got := worktreeDirtIn(context.Background(), ""); got != "" { + t.Errorf("worktreeDirtIn with no directory returned %q, so a git command ran", got) + } + if got := worktreeDirt(""); got != "" { + t.Errorf("worktreeDirt with no directory returned %q, so a git command ran", got) + } + if got := UnsavedEditsNote(""); got != "" { + t.Errorf("UnsavedEditsNote with no directory returned %q, so a git command ran", got) + } + if staged, problem := stagedPaths(repo); problem != "" { + t.Fatalf("read the index: %s", problem) + } else if len(staged) > 0 { + t.Errorf("a refused command staged %v in the repository the process was standing in", staged) + } +} + func TestAGitCommandWithNoDirectoryNeverRuns(t *testing.T) { // The proof that it never RAN, rather than merely never committed: this is // a real repository holding a real change, and a `git add` that reached it diff --git a/internal/session/taskpreflight.go b/internal/session/taskpreflight.go index 54a1098c74..0a500ea356 100644 --- a/internal/session/taskpreflight.go +++ b/internal/session/taskpreflight.go @@ -43,7 +43,6 @@ package session // an overlap worth a line ([hotFiles]). import ( - "os/exec" "path" "path/filepath" "strconv" @@ -153,8 +152,8 @@ func UnsavedEditsNote(root string) string { if strings.TrimSpace(root) == "" { return "" } - out, err := exec.Command("git", "-C", root, "status", "--porcelain", "--untracked-files=no").Output() - if err != nil || strings.TrimSpace(string(out)) == "" { + out, err := git(root, "status", "--porcelain", "--untracked-files=no") + if err != nil || strings.TrimSpace(out) == "" { return "" } return unsavedEditsWord From 4146dbfc347044735a35471ac4dd3fae49f067ce Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 06:06:14 -0400 Subject: [PATCH 2/6] session: releaseLanded refuses a tree with no ground and takes the root lock The mirror road releaseKept already closes: a tree whose root or directory never resolved has nowhere a worktree is registered and no repository to ask, and its removal and branch deletion would have run wherever the process happened to be, unlocked. Co-Authored-By: codeaf --- internal/session/groundladder.go | 12 ++++ internal/session/task_release_landed_test.go | 69 ++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 internal/session/task_release_landed_test.go diff --git a/internal/session/groundladder.go b/internal/session/groundladder.go index cf16c1a929..e60b8d8f79 100644 --- a/internal/session/groundladder.go +++ b/internal/session/groundladder.go @@ -1147,6 +1147,18 @@ func (t taskTree) carryBranchHomeLocked() (string, error) { // session's own and goes with the work it carried; what furrow is told is that // the record may be forgotten ([taskTree.dropUniverse]). func (t taskTree) releaseLanded() { + // THE SAME DOOR [taskTree.releaseKept] closes: a tree whose ground was never + // made has nowhere a worktree could be registered and no repository to ask, + // and a command with no directory runs in the process's own directory — so + // the removal below would take a working copy out of somebody else's + // checkout and the branch deletion would name a branch in it. + if strings.TrimSpace(t.root) == "" || strings.TrimSpace(t.dir) == "" { + return + } + // THE REMOVAL AND THE DELETION ARE ONE SPAN OF GIT COMMANDS against the + // ground repository, so they take its lock the way every landing does + // ([taskTree.releaseKept] and [taskTree.carryBranchHome]). + defer lockGitRoot(t.place, t.root)() if t.ownRepository() { _ = os.RemoveAll(t.dir) } else if _, err := git(t.root, "worktree", "remove", t.dir); err != nil { diff --git a/internal/session/task_release_landed_test.go b/internal/session/task_release_landed_test.go new file mode 100644 index 0000000000..4a815758b6 --- /dev/null +++ b/internal/session/task_release_landed_test.go @@ -0,0 +1,69 @@ +package session + +import ( + "os" + "strings" + "testing" +) + +// A TREE WITH NO GROUND IS LEFT ALONE BY ITS RELEASE. +// +// [taskTree.releaseKept] refuses a tree with an empty root or directory, for +// the reason gitWith does: a command with no directory runs wherever the +// process happens to be, and a removal and a branch deletion pointed there +// take apart the checkout the process is standing in. [taskTree.releaseLanded] +// is the mirror image of releaseKept and was missing both the refusal and the +// root lock; these tests hold the refusal in place. + +// TestReleaseLandedLeavesAnEmptyTreeAlone proves the empty case never runs git +// at all: the tree is released while the process stands inside a real +// repository, and neither that repository's worktrees nor its branches move. +func TestReleaseLandedLeavesAnEmptyTreeAlone(t *testing.T) { + repo := newTestRepo(t) + for _, tree := range []taskTree{ + {}, // nothing was ever grounded + {dir: repo}, // a directory with no root + {root: repo}, // a root with no directory + {root: " ", dir: " ", branch: "task/x"}, // whitespace is emptiness too + } { + before := gitOut(t, repo, "worktree", "list", "--porcelain") + branches := gitOut(t, repo, "branch", "--list", "--format=%(refname:short)") + + t.Chdir(repo) + tree.releaseLanded() + + if after := gitOut(t, repo, "worktree", "list", "--porcelain"); after != before { + t.Errorf("releasing %+v changed the registered worktrees:\n%s", tree, after) + } + if after := gitOut(t, repo, "branch", "--list", "--format=%(refname:short)"); after != branches { + t.Errorf("releasing %+v changed the branches:\n%s", tree, after) + } + if _, err := os.Stat(repo); err != nil { + t.Fatalf("releasing %+v disturbed the repository: %v", tree, err) + } + } +} + +// TestReleaseLandedRemovesItsWorktreeAndBranch is the ordinary case beside it: +// a released landing still unregisters its worktree and deletes its branch, +// so the guard above cannot be read as the release having gone quiet. +func TestReleaseLandedRemovesItsWorktreeAndBranch(t *testing.T) { + repo := newTestRepo(t) + place := Place{Dir: t.TempDir(), Workspace: repo} + tree, err := prepareTaskTree(place, repo, "abcd1234abcd9876", 3, "paint the fence") + if err != nil { + t.Fatalf("prepareTaskTree: %v", err) + } + if !strings.Contains(gitOut(t, repo, "worktree", "list"), tree.dir) { + t.Fatalf("git does not know the worktree %s", tree.dir) + } + + tree.releaseLanded() + + if list := gitOut(t, repo, "worktree", "list"); strings.Contains(list, tree.dir) { + t.Errorf("the worktree is still registered:\n%s", list) + } + if branchIsThere(repo, tree.branch) { + t.Errorf("the branch %s is still there", tree.branch) + } +} From f1f9f4c6d235499b80b9b9cf34a1d787a30c2c34 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 06:15:32 -0400 Subject: [PATCH 3/6] session: the checkout guard reads a linked worktree's own share of the clone Branches and worktree registrations belong to the clone's common directory, so from a linked worktree every sibling's task/* churn read as this run's damage. The guard now answers rev-parse --git-dir and --git-common-dir, skips the branch reading and narrows the worktree reading to the checkout itself when the two differ, and says so in the explanation it prints. The live-codeaf probe moves from /proc, which exists on Linux only, to the process table, and asks whether any other process's command line names a worktree of the same clone. Co-Authored-By: codeaf --- internal/session/hermetic_checkout_test.go | 240 +++++++++++++++------ 1 file changed, 173 insertions(+), 67 deletions(-) diff --git a/internal/session/hermetic_checkout_test.go b/internal/session/hermetic_checkout_test.go index 3388179be8..b3f10ca26e 100644 --- a/internal/session/hermetic_checkout_test.go +++ b/internal/session/hermetic_checkout_test.go @@ -4,7 +4,9 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "sort" + "strconv" "strings" "testing" ) @@ -66,6 +68,15 @@ import ( // do not care what anything is called and still catch the commit and the files, // which is the shape that actually cost somebody an afternoon. // +// AND A LINKED WORKTREE OF A SHARED CLONE READS LESS STILL. Branches belong to +// the clone's common directory, not to the working copy they were read from, so +// from a linked worktree every sibling's task/* churn — a branch cut for their +// own run, a branch deleted when their run landed — would look exactly like +// this run's damage, and the branch reading is skipped outright there. The +// worktree reading narrows instead of skipping: registrations under this root +// can still be this suite's, and a sibling's registration elsewhere in the +// clone cannot. +// // It cannot see a content change to a file that was already dirty before the // run — hashing the whole checkout each run costs more than it is worth — and // the incident's shape, new commits, new files, new branches and new worktrees, @@ -73,9 +84,12 @@ import ( // checkout is what the tree looked like at a moment: the commit it stood on, // the paths git considered dirty or unknown, the branches it had, and the -// worktrees registered against it. +// worktrees registered against it. When the tree is a linked worktree of a +// shared clone, branches is nil — the reading is skipped, not empty — and the +// worktrees are only the ones registered under the tree itself. type checkout struct { root string + linked bool head string dirty map[string]bool branches map[string]bool @@ -99,12 +113,38 @@ func watchTheCheckout() checkout { if watched.head, err = gitLine(root, "rev-parse", "HEAD"); err != nil { return checkout{} } + watched.linked = isLinkedWorktree(root) + if !watched.linked { + watched.branches = branchNames(root) + } watched.dirty = porcelain(root) - watched.branches = branchNames(root) - watched.worktrees = worktreePaths(root) + watched.worktrees = worktreePaths(root, watched.linked) return watched } +// isLinkedWorktree says whether the working copy at root is one worktree among +// several of one shared clone, which `rev-parse` answers by naming two +// different directories: the repository this copy carries its state in, and +// the common one every sibling of the same clone points at. A plain checkout +// answers the same directory twice. +func isLinkedWorktree(root string) bool { + gitDir, err := gitLine(root, "rev-parse", "--git-dir") + if err != nil || gitDir == "" { + return false + } + commonDir, err := gitLine(root, "rev-parse", "--git-common-dir") + if err != nil || commonDir == "" { + return false + } + if !filepath.IsAbs(gitDir) { // a plain checkout answers relative to root + gitDir = filepath.Join(root, gitDir) + } + if !filepath.IsAbs(commonDir) { + commonDir = filepath.Join(root, commonDir) + } + return filepath.Clean(gitDir) != filepath.Clean(commonDir) +} + // moved says how the checkout differs from what it was, and "" when it does // not. A watch that was never taken never reports. func (c checkout) moved() string { @@ -127,33 +167,40 @@ func (c checkout) moved() string { if moved := differing(c.dirty, porcelain(c.root)); len(moved) > 0 { said = append(said, "these paths are not what they were before the run:\n\t"+strings.Join(moved, "\n\t")) } - // A BRANCH IS A WRITE THAT MOVES NOTHING. `task/do-the-thing-` cut in - // the person's repository leaves HEAD exactly where it was and leaves the - // porcelain empty, and it is still the suite committing into a tree it does - // not own — the merge that would have moved HEAD is the only part that did - // not happen yet. - // - // AND A LIVE codeaf CREATES THE SAME BRANCHES. When another codeaf is using - // this checkout and TMPDIR is outside it, branch and worktree churn is - // ambient the way journal growth is (hermetic_test.go) — report the head - // and the porcelain, which a neighbour's ordinary work does not move, and - // leave the shared refs alone. The #578 TempDir-inside-checkout case still - // reports everything, because that is when the suite itself can mint them. - ignoreAmbientRefs := liveCodeafUsingCheckout(c.root) && !tempDirInsideCheckout(c.root) - if !ignoreAmbientRefs { - if moved := differing(c.branches, branchNames(c.root)); len(moved) > 0 { - said = append(said, "these branches are not what they were before the run:\n\t"+strings.Join(moved, "\n\t")) - } - // AND A WORKTREE IS A WRITE GIT IS TOLD TO IGNORE. The ground ladder puts a - // task's tree under `.codeaf/`, which .gitignore covers, so the porcelain - // above stays silent about a whole second checkout sitting in the person's - // tree. The registration is not ignorable: git keeps it, so it is asked for. - if moved := differing(c.worktrees, worktreePaths(c.root)); len(moved) > 0 { + // BRANCHES AND WORKTREES BELONG TO THE WHOLE SHARED CLONE the checkout was + // read from, and a LINKED worktree reads even less than the ambient case: + // every sibling of the same clone churns the same task/* refs this suite + // could not have written, so from a linked worktree the branch reading is + // skipped outright and the worktree reading keeps only the registrations + // under the checkout itself. The head and the porcelain above still watch + // everything, whatever the checkout is. + if c.linked { + if moved := differing(c.worktrees, worktreePaths(c.root, true)); len(moved) > 0 { said = append(said, "these worktrees are not what they were before the run:\n\t"+strings.Join(moved, "\n\t")) } - } else if moved := differing(c.branches, branchNames(c.root)); len(moved) > 0 || - len(differing(c.worktrees, worktreePaths(c.root))) > 0 { - fmt.Fprintf(os.Stderr, "checkout guard: another codeaf is using %s; ignoring task branch/worktree churn (head and porcelain still watched)\n", c.root) + } else { + // AND A LIVE codeaf CREATES THE SAME BRANCHES. When another codeaf is using + // this checkout and TMPDIR is outside it, branch and worktree churn is + // ambient the way journal growth is (hermetic_test.go) — report the head + // and the porcelain, which a neighbour's ordinary work does not move, and + // leave the shared refs alone. The #578 TempDir-inside-checkout case still + // reports everything, because that is when the suite itself can mint them. + ignoreAmbientRefs := liveCodeafUsingCheckout(c.root) && !tempDirInsideCheckout(c.root) + if !ignoreAmbientRefs { + if moved := differing(c.branches, branchNames(c.root)); len(moved) > 0 { + said = append(said, "these branches are not what they were before the run:\n\t"+strings.Join(moved, "\n\t")) + } + // AND A WORKTREE IS A WRITE GIT IS TOLD TO IGNORE. The ground ladder puts a + // task's tree under `.codeaf/`, which .gitignore covers, so the porcelain + // above stays silent about a whole second checkout sitting in the person's + // tree. The registration is not ignorable: git keeps it, so it is asked for. + if moved := differing(c.worktrees, worktreePaths(c.root, false)); len(moved) > 0 { + said = append(said, "these worktrees are not what they were before the run:\n\t"+strings.Join(moved, "\n\t")) + } + } else if moved := differing(c.branches, branchNames(c.root)); len(moved) > 0 || + len(differing(c.worktrees, worktreePaths(c.root, false))) > 0 { + fmt.Fprintf(os.Stderr, "checkout guard: another codeaf is using %s; ignoring task branch/worktree churn (head and porcelain still watched)\n", c.root) + } } if len(said) == 0 { return "" @@ -167,7 +214,7 @@ func (c checkout) moved() string { "`rev-parse --show-toplevel` answers with this checkout (task_run.go's repositoryRoot, " + "#578). Which one it was, this guard cannot tell you; if the run had GOTMPDIR or TMPDIR " + "pointing inside a checkout, suspect the second. Undo the above before pushing.\n" + - "(a second checkout running this same suite beside you cannot cause this; a test that names no directory can, and so can a temporary directory that is not outside your tree. A live codeaf using this checkout is ignored for branch/worktree churn when TMPDIR is outside the tree.)" + "(a second checkout running this same suite beside you cannot cause this; a test that names no directory can, and so can a temporary directory that is not outside your tree. A live codeaf using this checkout is ignored for branch/worktree churn when TMPDIR is outside the tree. When this checkout is a linked worktree of a shared clone, branch churn is not reported at all and the worktree reading covers only registrations under the checkout, because both belong to the clone the siblings share — a leak that only cut a task/ branch from inside a linked worktree slips past here, and the head and the porcelain above are what catch it.)" } // differing names every line that is in one listing and not the other, marked @@ -249,9 +296,12 @@ func harnessBranch(name string) bool { return strings.HasPrefix(name, harnessBranchPrefix) } -// worktreePaths is the set of worktree directories registered against root that -// THIS SUITE could have registered — including one it hid under an ignored -// folder, which is the whole reason this reading exists. +// worktreePaths is the set of worktree directories registered against root +// that THIS SUITE could have registered — including one it hid under an ignored +// folder, which is the whole reason this reading exists. For a linked worktree +// it is asked with underRoot, because the listing answers for the whole shared +// clone and only the registrations under the checkout itself can be the +// suite's. // // ONLY THE PATHS ARE KEPT. The plain listing carries each worktree's head // beside its path, so keeping the whole line would report a head move a second @@ -259,12 +309,22 @@ func harnessBranch(name string) bool { // question. The porcelain form is asked for so the path and the branch can be // read separately, and an unreadable tree is an empty set as everywhere else // here. -func worktreePaths(root string) map[string]bool { +func worktreePaths(root string, underRoot bool) map[string]bool { out, err := gitLines(root, "worktree", "list", "--porcelain") if err != nil { return map[string]bool{} } - return harnessWorktrees(root, out) + found := harnessWorktrees(root, out) + if !underRoot { + return found + } + under := strings.TrimSuffix(root, "/") + "/" + for path := range found { + if !strings.HasPrefix(path, under) { + delete(found, path) + } + } + return found } // harnessWorktrees reads `git worktree list --porcelain` — one record per @@ -440,58 +500,104 @@ func TestTheCheckoutGuardReportsOnlyWhatThisRunCouldHaveWritten(t *testing.T) { } } +// TestTheCheckoutGuardDoesNotReportASiblingWorktreesBranch is the shared-clone +// case, run for real: a linked worktree stands beside a sibling in one clone, +// the sibling cuts and takes away task/ branches and registers a worktree of +// its own while "the run" is watched, and none of it may read as this run's +// damage — branches belong to the clone's common directory, and the sibling's +// registrations stand outside the watched tree. The second half proves the +// churn is real: watched from the main checkout, where the refs are this +// checkout's own, the same branch deletion is reported. +func TestTheCheckoutGuardDoesNotReportASiblingWorktreesBranch(t *testing.T) { + shared := newTestRepo(t) + linked := filepath.Join(shared, "linked") + sibling := filepath.Join(shared, "sibling") + mustGit(t, shared, "worktree", "add", "-b", "task/this-run-0a1b2c", linked) + mustGit(t, shared, "worktree", "add", "-b", "task/sibling-run-9f8e7d", sibling) + // A branch the sibling will take away during the run, seen only through + // the clone's shared refs. + mustGit(t, shared, "branch", "task/sibling-gone-554433") + + t.Chdir(linked) + watched := watchTheCheckout() + if !watched.linked { + t.Fatal("the linked worktree was not read as linked, so this test is not standing where it means to") + } + // THE SAME REFS READ FROM THE MAIN CHECKOUT, before the churn, are this + // checkout's own — that watch is the contrast the narrowing has to hold + // against. + head, _ := gitLine(shared, "rev-parse", "HEAD") + watchedMain := checkout{root: shared, head: head, + dirty: porcelain(shared), branches: branchNames(shared), worktrees: worktreePaths(shared, false)} + + mustGit(t, sibling, "branch", "task/sibling-cuts-778899") + mustGit(t, sibling, "branch", "-D", "task/sibling-gone-554433") + mustGit(t, shared, "worktree", "add", "-b", "task/sibling-tree-112233", filepath.Join(shared, "sibling-tree")) + + if said := watched.moved(); said != "" { + t.Fatalf("a sibling worktree's churn was reported as this run's damage:\n%s", said) + } + + // AND THE SAME CHURN IS REPORTED FROM THE MAIN CHECKOUT — the narrowing + // above is tied to the linked worktree, not to the branch having been + // someone else's. + if said := watchedMain.moved(); !strings.Contains(said, "gone: task/sibling-gone-554433") { + t.Fatalf("from the main checkout the sibling's deleted branch was not reported:\n%s", said) + } +} + // liveCodeafUsingCheckout reports whether another codeaf process looks like it // is working in this repository right now — a chat, a tick, a hosted session — // whose task/ branches and `.codeaf/` worktrees would otherwise look exactly // like the suite's own leak. // -// IT IS BEST-EFFORT AND FAILS OPEN TO "NO". A machine without /proc, a binary +// THE QUESTION IS ASKED OF THE PROCESS TABLE, which exists wherever this +// program does: any other process whose command line names a worktree of the +// repository root belongs to is taken for a live codeaf. The worktrees come +// from `git worktree list --porcelain` — the checkout itself, and every tree +// registered against the same clone. A /proc walk was tried first and +// abandoned: it exists on Linux only, so on darwin the answer was always +// "no", and the ambient reading never engaged on the machine this suite is +// written on. +// +// IT IS BEST-EFFORT AND FAILS OPEN TO "NO". A machine without ps, a binary // renamed something else, or a sandbox that hides other processes simply keeps // the stricter reading; a false negative is a red that a person can re-run with // codeaf closed, and a false positive would hide a real suite write. func liveCodeafUsingCheckout(root string) bool { - under := strings.TrimSuffix(root, "/") + "/" - entries, err := os.ReadDir("/proc") + lines, err := gitLines(root, "worktree", "list", "--porcelain") if err != nil { return false } - self := os.Getpid() - for _, entry := range entries { - if !entry.IsDir() { - continue - } - name := entry.Name() - if len(name) == 0 || name[0] < '0' || name[0] > '9' { - continue - } - var pid int - for i := 0; i < len(name); i++ { - if name[i] < '0' || name[i] > '9' { - pid = -1 - break - } - pid = pid*10 + int(name[i]-'0') + places := map[string]bool{root: true} + for _, line := range lines { + if rest, ok := strings.CutPrefix(line, "worktree "); ok { + places[strings.TrimSpace(rest)] = true } - if pid <= 0 || pid == self { + } + out, err := exec.Command("ps", "-axo", "pid=,command=").Output() + if err != nil { + return false + } + self := os.Getpid() + for _, line := range strings.Split(string(out), "\n") { + pid, command, found := strings.Cut(strings.TrimSpace(line), " ") + if !found { continue } - cmdline, err := os.ReadFile("/proc/" + name + "/cmdline") - if err != nil { + // The suite's own test binary stands IN a worktree but does not name one + // on its command line, so it never reads as a neighbour here. + own, err := strconv.Atoi(strings.TrimSpace(pid)) + if err != nil || own <= 0 || own == self { continue } - joined := strings.ReplaceAll(string(cmdline), "\x00", " ") - if !strings.Contains(joined, "codeaf") { + if !strings.Contains(command, "codeaf") { continue } - // The suite's own test binary and helpers name codeaf too (`go test`, - // `codeaf-session-test-home`). A live chat is the binary itself, or a - // tick/host child, with the checkout in its cwd or its arguments. - cwd, err := os.Readlink("/proc/" + name + "/cwd") - if err == nil && (cwd == root || strings.HasPrefix(cwd, under)) { - return true - } - if strings.Contains(joined, root) { - return true + for place := range places { + if strings.Contains(command, place) { + return true + } } } return false From 9e0fd5a12946a12bd6bde58d424ff9cf277dc170 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 06:19:37 -0400 Subject: [PATCH 4/6] session: the no-directory status test stands in a repository whose tracked file has an unsaved edit Co-Authored-By: codeaf --- internal/session/taskgitdir_test.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/internal/session/taskgitdir_test.go b/internal/session/taskgitdir_test.go index 96607cae52..296421cbcf 100644 --- a/internal/session/taskgitdir_test.go +++ b/internal/session/taskgitdir_test.go @@ -23,6 +23,9 @@ import ( // [worktreeDirtIn]'s dirty fingerprint and [UnsavedEditsNote]'s status reading. // `git -C ""` is a no-op — git runs where the process is standing — so a direct // exec.Command with an empty directory slipped past the refusal in gitWith. +// The repository the process stands in holds a TRACKED file with an unsaved +// edit, because [UnsavedEditsNote] reads with --untracked-files=no and an +// untracked file alone would leave it silent whether or not git ran. func TestStatusReadsWithNoDirectoryNeverRuns(t *testing.T) { repo := t.TempDir() for _, args := range [][]string{{"init", "-q", "-b", "main"}, {"config", "user.name", "codeaf"}} { @@ -31,6 +34,19 @@ func TestStatusReadsWithNoDirectoryNeverRuns(t *testing.T) { } } writeFile(t, filepath.Join(repo, "note.md"), "work\n") + mustGit(t, repo, "add", "-A", "--", ".") + mustGit(t, repo, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", "the note") + writeFile(t, filepath.Join(repo, "note.md"), "more work\n") + + // Both readings see the edit when they are told where to look; that is what + // makes their silence below a refusal rather than an empty status. + if got := UnsavedEditsNote(repo); got != unsavedEditsWord { + t.Fatalf("UnsavedEditsNote(repo) = %q, so the edit is not visible to a status read", got) + } + if got := worktreeDirtIn(context.Background(), repo); got == "" { + t.Fatal("worktreeDirtIn(repo) answered \"\", so the edit is not visible to a status read") + } + t.Chdir(repo) if got := worktreeDirtIn(context.Background(), ""); got != "" { @@ -42,11 +58,6 @@ func TestStatusReadsWithNoDirectoryNeverRuns(t *testing.T) { if got := UnsavedEditsNote(""); got != "" { t.Errorf("UnsavedEditsNote with no directory returned %q, so a git command ran", got) } - if staged, problem := stagedPaths(repo); problem != "" { - t.Fatalf("read the index: %s", problem) - } else if len(staged) > 0 { - t.Errorf("a refused command staged %v in the repository the process was standing in", staged) - } } func TestAGitCommandWithNoDirectoryNeverRuns(t *testing.T) { From 951af3202175318d54e898762bae4f6a9bddf9aa Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 06:26:06 -0400 Subject: [PATCH 5/6] session, docs: the gitWith comment reads whole, and the change note for the directory refusal --- .../1099-no-git-command-without-a-directory.md | 14 ++++++++++++++ internal/session/task_run.go | 8 ++++---- 2 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 docs/changes/unreleased/1099-no-git-command-without-a-directory.md diff --git a/docs/changes/unreleased/1099-no-git-command-without-a-directory.md b/docs/changes/unreleased/1099-no-git-command-without-a-directory.md new file mode 100644 index 0000000000..d6bdf92d81 --- /dev/null +++ b/docs/changes/unreleased/1099-no-git-command-without-a-directory.md @@ -0,0 +1,14 @@ +--- +kind: fixed +title: No git command runs without a directory, and the checkout guard reads a linked worktree's own share +pr: 1099 +surface: [engine] +invalidates: + - "`worktreeDirtIn` and `UnsavedEditsNote` built `git -C ` themselves, and `git -C \"\"` is a no-op, so an empty directory ran the command where the process stood. Both go through `gitWith` now (`gitContext` is the same call under a caller's context), where the refusal of an empty directory and the pinned environment live once." + - "`taskTree.releaseLanded` removed a worktree and deleted its branch with no empty-root guard and without the root lock. It refuses a tree with no root or directory and takes the root lock, as `releaseKept` does." + - "The session package's checkout guard read branches and worktree registrations for the whole shared clone, so from a linked worktree a sibling's task/* churn was reported as the run's damage, and its live-codeaf probe walked `/proc`, which does not exist on darwin. From a linked worktree the branch reading is skipped and the worktree reading keeps only registrations under the checkout; the probe reads the process table." +--- + +A command with no directory runs wherever the process happens to be. Every +spelling of a git call in this package is a wrapper of one function that +refuses that, and the two that had gone round it are wrappers of it now. diff --git a/internal/session/task_run.go b/internal/session/task_run.go index 533f28b916..a9b75fcd6d 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -8960,10 +8960,10 @@ func git(dir string, args ...string) (string, error) { } // gitWith is [gitContext] with [context.Background] in place of a caller's own -// context, kept as its own name because nearly every call here is that. -// exactly one caller: the ground ladder stages a parent's tree into AN INDEX OF -// ITS OWN so that handing out a child never moves the parent's index -// ([sealGroundWork]). GIT_INDEX_FILE is the only way to say that to git, and a +// context, kept as its own name because nearly every call here is that. The +// environment it takes exists for exactly one caller: the ground ladder stages +// a parent's tree into AN INDEX OF ITS OWN so that handing out a child never +// moves the parent's index ([sealGroundWork]). GIT_INDEX_FILE is the only way to say that to git, and a // second copy of the pager and editor settings beside it would be the drift the // one-source-of-truth law forbids. func gitWith(dir string, environment []string, args ...string) (string, error) { From 32a1ca44de43a3d93d24e9312b390ed3d350e972 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 17 Sep 2026 06:47:46 -0400 Subject: [PATCH 6/6] session: releaseLanded leaves the root lock to the landing that holds it The lock is not re-entrant and every road to releaseLanded already holds it, so taking it again waited on itself. --- internal/session/groundladder.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/session/groundladder.go b/internal/session/groundladder.go index e60b8d8f79..c3cb24dc4f 100644 --- a/internal/session/groundladder.go +++ b/internal/session/groundladder.go @@ -1155,10 +1155,11 @@ func (t taskTree) releaseLanded() { if strings.TrimSpace(t.root) == "" || strings.TrimSpace(t.dir) == "" { return } - // THE REMOVAL AND THE DELETION ARE ONE SPAN OF GIT COMMANDS against the - // ground repository, so they take its lock the way every landing does - // ([taskTree.releaseKept] and [taskTree.carryBranchHome]). - defer lockGitRoot(t.place, t.root)() + // THE ROOT LOCK IS THE CALLER'S. Every road that reaches here is a landing + // that already holds it ([taskTree.carryBranchHome]'s span, task_run.go), + // the same way [taskTree.releaseKeptLocked] is reached, and the lock is not + // re-entrant: taking it again here waited on itself for the whole of a + // suite's timeout. if t.ownRepository() { _ = os.RemoveAll(t.dir) } else if _, err := git(t.root, "worktree", "remove", t.dir); err != nil {