From 9b5f7c9c1e97e0c724976536d9634acc143aaf68 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Wed, 14 Oct 2026 18:37:41 +1100 Subject: [PATCH 1/2] fix(context_for_task): ask each file's sensitivity once per call The walk asks whether every node it reaches sits in a sensitive file: 641 to 2,023 times per pack over 109 to 242 files on arm R's real requests. The daemon's decision (connSession.changeSensitive) resolves the path's project on disk each time (historyRootFor -> workspacePool.Detect -> filepath.Glob for markers), so on the live daemon a pack overran its 2 s graph deadline even on a warm, fully resolved index and lost its affected-tests section and run-first line (PLAN-462 arm R v2: 3 of 4 Go packs). The in-process replays that cleared #651 wired no sensitive decision, so they could not see it. The answer is now remembered per path for the one call. Co-Authored-By: Claude Opus 5.5 Plumb-Session: warm-cobra --- CHANGELOG.md | 8 +++ internal/tools/context_collect.go | 20 ++++++- internal/tools/context_sensitive_memo_test.go | 52 +++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 internal/tools/context_sensitive_memo_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e55791e3..35e37050 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -300,6 +300,14 @@ ### Fixed +- **`context_for_task` asks each file's sensitivity once per call, so live packs + keep their affected tests.** The walk asks whether every node it reaches is in a + sensitive file, 600 to 2,000 times per pack over a hundred or two files, and + the daemon answers each by resolving the path's project on disk. A live pack + overran its 2 s graph deadline even on a warm index, losing the affected-tests + section and its run-first line in three of five real tasks. The answer is now + remembered per path for the call. + - **An edit to the global `config.toml` made while the daemon starts is no longer missed.** The daemon loads the file, then starts watching it; a write in between raised no event the watch could see, so the daemon kept the older diff --git a/internal/tools/context_collect.go b/internal/tools/context_collect.go index d33beade..9cad670e 100644 --- a/internal/tools/context_collect.go +++ b/internal/tools/context_collect.go @@ -6,6 +6,7 @@ import ( "path" "path/filepath" "strings" + "sync" "time" ) @@ -330,11 +331,28 @@ func (c *ContextCollector) walk(ctx, graphCtx context.Context, pack *contextPack // sensitiveFor adapts the injected decision to the index's root-relative paths. // It is nil when no decision is wired, which withholds nothing. +// +// The answer is remembered per path for the one call: a walk asks about every +// node it reaches, 600 to 2,000 times over a hundred or two files, and the +// daemon's decision resolves the path's project each time (workspace detection +// on disk), which made a live pack overrun its graph deadline. A path's +// sensitivity cannot change within the call that asks. func (c *ContextCollector) sensitiveFor(ctx context.Context, root string) func(string) bool { if c.sensitive == nil { return nil } - return func(rel string) bool { return c.sensitive(ctx, absUnder(root, rel), "") } + var mu sync.Mutex + seen := map[string]bool{} + return func(rel string) bool { + mu.Lock() + defer mu.Unlock() + if v, ok := seen[rel]; ok { + return v + } + v := c.sensitive(ctx, absUnder(root, rel), "") + seen[rel] = v + return v + } } // markStale marks the relationships that come from a file whose bytes changed diff --git a/internal/tools/context_sensitive_memo_test.go b/internal/tools/context_sensitive_memo_test.go new file mode 100644 index 00000000..6b93709d --- /dev/null +++ b/internal/tools/context_sensitive_memo_test.go @@ -0,0 +1,52 @@ +package tools + +import ( + "context" + "path/filepath" + "strings" + "sync" + "testing" +) + +// TestContextForTask_AsksEachPathItsSensitivityOnce: a walk reaches many nodes of +// the same files, and the daemon's decision resolves the path's project on disk +// each time it is asked, so one call asks it once per path. The withheld control +// shows the remembered answer is still the decision's own: the sensitive file's +// caller is withheld exactly as when every node asked. +func TestContextForTask_AsksEachPathItsSensitivityOnce(t *testing.T) { + s := newShop(t) + var mu sync.Mutex + asked := map[string]int{} + s.collector.WithSensitive(func(_ context.Context, p, _ string) bool { + mu.Lock() + asked[p]++ + mu.Unlock() + return strings.HasSuffix(filepath.ToSlash(p), "/cart/cart.go") + }) + pack := s.collect(t, map[string]any{"symbols": []string{"pricing/discount.go#Apply"}, "intent": contextIntentChange}) + + if len(asked) == 0 { + t.Fatal("control: the walk never asked about sensitivity, so the test proves nothing") + } + for p, n := range asked { + if n != 1 { + t.Errorf("%s was asked %d times in one call, want once", p, n) + } + } + total, ok := pack.related(goldID("cart/cart.go", "(*Cart).Total")) + if !ok || !total.Withheld { + t.Fatalf("control: the sensitive file's caller must still be withheld, got %+v (found %v)", total, ok) + } + + // A second call asks again: the memory lasts one call, never across calls. + before := len(asked) + _ = s.collect(t, map[string]any{"symbols": []string{"pricing/discount.go#Apply"}, "intent": contextIntentChange}) + for p, n := range asked { + if n != 2 { + t.Errorf("%s was asked %d times over two calls, want twice (once per call)", p, n) + } + } + if len(asked) != before { + t.Errorf("the second call reached %d paths, the first %d: the walk is not deterministic", len(asked), before) + } +} From 61dc7a3f5f2ec1db74a460b12b7a9344066f87d6 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Wed, 14 Oct 2026 18:51:05 +1100 Subject: [PATCH 2/2] test(context_for_task): pin that every node of a sensitive file stays withheld under the memo Review S1 (scarlet-badger): a memo returning the wrong remembered answer leaked only the second and later nodes of a file, which the control did not check. The test now requires every cart/cart.go node withheld (at least 2 reached) and none of its body text in the pack. The sensitiveFor comment says which asks bypass the memo (review NIT). Co-Authored-By: Claude Opus 5.5 Plumb-Session: warm-cobra --- internal/tools/context_collect.go | 4 ++- internal/tools/context_sensitive_memo_test.go | 28 +++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/internal/tools/context_collect.go b/internal/tools/context_collect.go index 9cad670e..b15eb5ad 100644 --- a/internal/tools/context_collect.go +++ b/internal/tools/context_collect.go @@ -336,7 +336,9 @@ func (c *ContextCollector) walk(ctx, graphCtx context.Context, pack *contextPack // node it reaches, 600 to 2,000 times over a hundred or two files, and the // daemon's decision resolves the path's project each time (workspace detection // on disk), which made a live pack overrun its graph deadline. A path's -// sensitivity cannot change within the call that asks. +// sensitivity cannot change within the call that asks. Only the walk's asks go +// through this memo; the constraint, document and hint paths ask c.sensitive +// directly, a few times each. func (c *ContextCollector) sensitiveFor(ctx context.Context, root string) func(string) bool { if c.sensitive == nil { return nil diff --git a/internal/tools/context_sensitive_memo_test.go b/internal/tools/context_sensitive_memo_test.go index 6b93709d..7103b6a4 100644 --- a/internal/tools/context_sensitive_memo_test.go +++ b/internal/tools/context_sensitive_memo_test.go @@ -11,8 +11,9 @@ import ( // TestContextForTask_AsksEachPathItsSensitivityOnce: a walk reaches many nodes of // the same files, and the daemon's decision resolves the path's project on disk // each time it is asked, so one call asks it once per path. The withheld control -// shows the remembered answer is still the decision's own: the sensitive file's -// caller is withheld exactly as when every node asked. +// shows the remembered answer is still the decision's own: every node of the +// sensitive file is withheld, the repeated asks included, and none of its text +// reaches the pack. func TestContextForTask_AsksEachPathItsSensitivityOnce(t *testing.T) { s := newShop(t) var mu sync.Mutex @@ -33,9 +34,26 @@ func TestContextForTask_AsksEachPathItsSensitivityOnce(t *testing.T) { t.Errorf("%s was asked %d times in one call, want once", p, n) } } - total, ok := pack.related(goldID("cart/cart.go", "(*Cart).Total")) - if !ok || !total.Withheld { - t.Fatalf("control: the sensitive file's caller must still be withheld, got %+v (found %v)", total, ok) + // Every node from the sensitive file is withheld, not only the first one asked + // about: a memo that remembered the wrong answer would leak the later ones. + var fromCart int + for _, r := range pack.Related { + if r.Node.Path != "cart/cart.go" { + continue + } + fromCart++ + if !r.Withheld { + t.Errorf("%s from the sensitive cart/cart.go is not withheld: %+v", nodeSelector(r.Node), r) + } + } + if fromCart < 2 { + t.Fatalf("control: the walk reached %d node(s) of cart/cart.go, want at least 2, so a repeated answer is tested", fromCart) + } + out := renderContextPack(pack) + for _, body := range []string{"kept := c.items[:0]", "sub += it.Cents * it.Qty", "c.items[i].Qty += it.Qty"} { + if strings.Contains(out, body) { + t.Errorf("content of the sensitive cart/cart.go reached the pack: %q", body) + } } // A second call asks again: the memory lasts one call, never across calls.