perf(engine): keep furrow detection off the session build path

PERF.md

@@ -83,6 +83,16 @@ migration flag skips that check. `BenchmarkOpenSettledSessions` measures the
 whole public open/close operation. `TestSettledStoreTerminalStartup` separates
 fixture construction from the first interactive frame of the running terminal.
 
+Furrow detection stays off the hello path. The belt reads the remembered answer
+(`furrow.PeekTools`); the session asks furrow from a goroutine joined at close
+and appends the four workspace tools through `armFamily` if the folder is
+watched. Measured on Spark linux-arm64 with a cold engine per workspace and the
+owner profile, first frame fell from 3.87 s to about 0.5 s with the furrow
+that was embedded then; with furrow 0.3.3's fast status the unfixed tree is already
+at about 0.43 s, and this change keeps the build from depending on furrow's speed. The gate is
+`TestBuildingASessionDoesNotWaitForFurrow`, which holds `furrow status` open and
+asserts the build came back while that process was still alive; no clock decides it.
+
 Native peer acknowledgements reuse the validated routing view only for the same
 store and durably applied index. Every applied change, including revocation,
 invalidates that view. `TestR11R27UnchangedPeerAcknowledgementWork` exercises the

docs/changes/unreleased/1954-boot-without-furrow-detect.md

@@ -0,0 +1,27 @@
+---
+kind: fixed
+title: Show the first frame without waiting on furrow detection
+pr: 1954
+surface: [chat, engine, docs]
+invalidates:
+  - Building a session ran furrow to find out whether the folder is watched; it now starts that question in the background and the session never waits for it.
+  - The workspace snapshot, restore, fork and merge tools were decided when the belt was built; on a watched folder they now join the end of the belt when furrow answers.
+  - A session closing while another session on the same folder was still waiting for furrow used to wait for it too; it now leaves at once.
+---
+
+A cold launch used to block the engine hello behind `furrow --version` and
+`furrow status`, two to three seconds on a folder furrow was never pointed at
+(with the furrow that shipped before 0.3.3). Each session now asks furrow from a
+goroutine it owns and joins at close, so first-frame time no longer depends on
+how fast furrow is: on a cold engine and a real profile it was about 0.5 s
+instead of 3.9 s then, and it stays about 0.4 s whatever furrow does.
+
+An unwatched folder still gets none of the four tools. On a watched folder they
+are appended to the end of the tool list when furrow answers, like any other
+late capability, so the prompt cache is invalidated once at the tail and never
+moves anything ahead of it. The first request of a session waits up to 200 ms for
+a question that is already running, so a message sent the instant the session
+exists usually carries the tools and pays no invalidation. Tools never leave the
+list mid-conversation: a folder that stops being watched later keeps them, and
+they answer with furrow's own refusal. A furrow that fails or hangs is
+remembered as not attached for thirty seconds rather than asked again.

internal/furrow/ctx_test.go

@@ -0,0 +1,101 @@
+//go:build unix
+
+package furrow
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+	"time"
+)
+
+// installParkedFurrow is a furrow whose `status` announces itself on the ready
+// pipe and then parks on the gate pipe, so a test can hold one question open for
+// as long as it likes and prove a second asker did not wait for it.
+func installParkedFurrow(t *testing.T) (pid func() int, release func()) {
+	t.Helper()
+	carryNothing(t)
+	dir := t.TempDir()
+	ready, gate, pidfile := filepath.Join(dir, "ready"), filepath.Join(dir, "gate"), filepath.Join(dir, "pid")
+	for _, fifo := range []string{ready, gate} {
+		if err := syscall.Mkfifo(fifo, 0o600); err != nil {
+			t.Fatalf("mkfifo: %v", err)
+		}
+	}
+	script := filepath.Join(dir, "furrow")
+	body := `#!/bin/sh
+if [ "$1" = "--version" ]; then echo "furrow 0.1.0"; exit 0; fi
+echo $$ > "` + pidfile + `"
+echo started > "` + ready + `"
+cat "` + gate + `" > /dev/null
+echo '{"head":"aaaabbbbcccc0001","watcher_running":true}'
+`
+	if err := os.WriteFile(script, []byte(body), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", dir+string(os.PathListSeparator)+"/usr/bin"+string(os.PathListSeparator)+"/bin")
+	t.Setenv(BinaryEnvVar, "")
+	Forget()
+	t.Cleanup(Forget)
+	t.Cleanup(func() {
+		if f, err := os.OpenFile(gate, os.O_WRONLY|syscall.O_NONBLOCK, 0); err == nil {
+			_ = f.Close()
+		}
+	})
+	pid = func() int {
+		done := make(chan struct{})
+		go func() { _, _ = os.ReadFile(ready); close(done) }()
+		select {
+		case <-done:
+		case <-time.After(time.Minute):
+			t.Fatal("furrow's status call never started")
+		}
+		raw, _ := os.ReadFile(pidfile)
+		var n int
+		_, _ = fmt.Sscan(string(raw), &n)
+		return n
+	}
+	release = func() {
+		done := make(chan struct{})
+		go func() { _ = os.WriteFile(gate, nil, 0o600); close(done) }()
+		select {
+		case <-done:
+		case <-time.After(time.Minute):
+			t.Fatal("furrow was not waiting at the gate")
+		}
+	}
+	return pid, release
+}
+
+// A caller whose context is cancelled stops waiting behind another caller's
+// question. The other caller's furrow is untouched and still answers.
+func TestAWaiterCanAbandonAnotherAskersQuestion(t *testing.T) {
+	pid, release := installParkedFurrow(t)
+	root := t.TempDir()
+	leader := make(chan Presence, 1)
+	go func() { leader <- Detect(context.Background(), root) }()
+	leaderPid := pid()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	waiter := make(chan Presence, 1)
+	go func() { waiter <- Detect(ctx, root) }()
+	cancel()
+	select {
+	case got := <-waiter:
+		if got.Available() {
+			t.Fatalf("an abandoned wait answered %+v", got)
+		}
+	case <-time.After(time.Minute):
+		t.Fatal("a cancelled caller kept waiting behind another caller's furrow")
+	}
+	if syscall.Kill(leaderPid, 0) != nil {
+		t.Fatal("abandoning the wait cut the other caller's furrow")
+	}
+	release()
+	if got := <-leader; !got.Available() {
+		t.Fatalf("the question that was waited behind answered %+v; want available", got)
+	}
+}

internal/furrow/furrow.go

@@ -65,7 +65,11 @@ const (
 // mount must cost a session a bounded pause rather than a hang; a detection
 // that times out reads as "not attached", which is the answer that leaves the
 // belt honest.
-const detectTimeout = 5 * time.Second
+//
+// It is a variable for the reason [embedded] is: a test has to be able to say
+// "a furrow that hangs" without waiting out the real bound. Nothing in the
+// product reassigns it.
+var detectTimeout = 5 * time.Second
 
 // readTimeout bounds the read-only questions — the timeline, the fork list, a
 // restore preview. They touch the store and nothing else, so they are quick,
@@ -165,6 +169,14 @@ func lockAttach(root string) func() {
 	return lock.Unlock
 }
 
+// detecting holds the questions that are being asked right now, one per root.
+// A caller that finds its root here waits for that question instead of asking
+// its own, and can leave the wait when its own context ends ([Detect]).
+var (
+	detectingMu sync.Mutex
+	detecting   = map[string]chan struct{}{}
+)
+
 // Forget drops the cached answer for every workspace. It exists for tests,
 // which change what is on PATH between cases and would otherwise read a
 // neighbour's answer, and for a caller that has just watched the person attach
@@ -191,20 +203,67 @@ func Forget() {
 // CAN act on are carried in the fields instead.
 func Detect(ctx context.Context, root string) Presence {
 	root = absoluteRoot(root)
+	for {
+		if presence, ok := fresh(root); ok {
+			return presence
+		}
+		// ONE ASKER PER FOLDER AT A TIME, AND A WAITER CAN LEAVE. Every session on
+		// a workspace starts its own background question, so a window that opens
+		// several chats at once would otherwise run the same two execs once per
+		// chat. The later callers wait for the first one's answer, but on their
+		// OWN context: a session closing, or a ground-ladder rung whose deadline
+		// passed, stops waiting at once and leaves the first caller's furrow
+		// running for whoever still wants it. (A plain mutex here made a Close
+		// wait out another session's five-second bound.)
+		detectingMu.Lock()
+		if running, ok := detecting[root]; ok {
+			detectingMu.Unlock()
+			select {
+			case <-running:
+				// The asker finished. Its answer is in the cache, or it was
+				// cut short and wrote nothing, in which case this caller asks.
+				continue
+			case <-ctx.Done():
+				return Presence{}
+			}
+		}
+		running := make(chan struct{})
+		detecting[root] = running
+		detectingMu.Unlock()
+		defer func() {
+			detectingMu.Lock()
+			delete(detecting, root)
+			detectingMu.Unlock()
+			close(running)
+		}()
+
+		presence := detect(ctx, root)
+
+		// AN ANSWER A CANCELLATION WROTE IS NOT FURROW'S. A caller that was cut
+		// mid-question (a session closing while its background refresh was
+		// still in `furrow status`) got "not attached" because the exec was
+		// killed, not because the folder is unwatched, and leaving that in the
+		// cache would tell every other session on the folder the same untruth.
+		// A furrow that FAILED or TIMED OUT on its own is different: that is an
+		// answer, it is cached like any other, and it is what keeps a hanging
+		// furrow to two launches per [detectTTL] however many belts are built.
+		if ctx.Err() == nil {
+			presenceMu.Lock()
+			presenceCache[root] = cachedPresence{presence: presence, asked: time.Now()}
+			presenceMu.Unlock()
+		}
+		return presence
+	}
+}
 
+// fresh is the cached answer for a root when it is still inside [detectTTL].
+func fresh(root string) (Presence, bool) {
 	presenceMu.Lock()
+	defer presenceMu.Unlock()
 	if cached, ok := presenceCache[root]; ok && time.Since(cached.asked) < detectTTL {
-		presenceMu.Unlock()
-		return cached.presence
+		return cached.presence, true
 	}
-	presenceMu.Unlock()
-
-	presence := detect(ctx, root)
-
-	presenceMu.Lock()
-	presenceCache[root] = cachedPresence{presence: presence, asked: time.Now()}
-	presenceMu.Unlock()
-	return presence
+	return Presence{}, false
 }
 
 func detect(ctx context.Context, root string) Presence {
@@ -329,7 +388,14 @@ func Open(ctx context.Context, root string) *Workspace {
 	if !presence.Available() {
 		return nil
 	}
-	return &Workspace{root: absoluteRoot(root), binary: presence.Path}
+	return presence.workspace(root)
+}
+
+// workspace is the one place a Workspace is made from an answer, shared by
+// [Open] and [PeekTools] so the two cannot disagree about what holding one
+// claims.
+func (p Presence) workspace(root string) *Workspace {
+	return &Workspace{root: absoluteRoot(root), binary: p.Path}
 }
 
 // wholeWorkspaceTimeout bounds the two calls whose cost is the person's project

internal/furrow/peek.go

@@ -0,0 +1,38 @@
+package furrow
+
+import (
+	"time"
+
+	"github.com/Agent-Field/codeaf/internal/exec/bare"
+)
+
+// peek is the last answer about one folder, read from memory and NEVER from a
+// subprocess. It is the other half of [Detect], which is the door that asks.
+//
+// known is false when nobody has asked about this folder yet. fresh is false
+// when the answer is older than [detectTTL]; the stale answer is still returned
+// because it is the best one there is.
+func peek(root string) (presence Presence, fresh, known bool) {
+	root = absoluteRoot(root)
+	presenceMu.Lock()
+	defer presenceMu.Unlock()
+	cached, ok := presenceCache[root]
+	if !ok {
+		return Presence{}, false, false
+	}
+	return cached.presence, time.Since(cached.asked) < detectTTL, true
+}
+
+// PeekTools is [Tools] for a caller that may not wait: the four verbs when the
+// remembered answer says this folder is attached, and nothing when it says it is
+// not or when there is no answer yet. The absence law holds either way — a verb
+// is only ever handed over on the strength of an answer furrow gave. fresh says
+// whether that answer is young enough to rely on; a caller that wants a newer one
+// asks [Detect] from somewhere it is allowed to wait.
+func PeekTools(root string) (tools []bare.Tool, fresh bool) {
+	presence, fresh, known := peek(root)
+	if !known || !presence.Available() {
+		return nil, fresh
+	}
+	return presence.workspace(root).Tools(), fresh
+}