From 032b693eb24cb13cc0cbafe2d70995abf3567560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Sun, 16 Aug 2026 20:03:41 +0100 Subject: [PATCH] test: cover incremental content-search publishing and git-failure propagation (#2713) Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 5 (1M context) --- .../workspace_content_search_failure_test.go | 86 +++++++++++++++++++ .../use-workspace-content-search.test.ts | 43 ++++++++++ 2 files changed, 129 insertions(+) create mode 100644 apps/backend/internal/agentctl/server/process/workspace_content_search_failure_test.go diff --git a/apps/backend/internal/agentctl/server/process/workspace_content_search_failure_test.go b/apps/backend/internal/agentctl/server/process/workspace_content_search_failure_test.go new file mode 100644 index 0000000000..aff07c739a --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/workspace_content_search_failure_test.go @@ -0,0 +1,86 @@ +//go:build !windows + +package process + +import ( + "context" + "os" + "testing" + "time" + + "github.com/kandev/kandev/internal/common/subproc" +) + +// Content search resolves its file list from a live `git ls-files` behind the +// shared Git admission semaphore, so under contention that command can be +// starved at admission or killed at its execution deadline. Both failures must +// reach the caller as errors. +// +// A swallowed failure here would be indistinguishable from a genuinely empty +// workspace: the palette would report "No content matches found" for a file +// that exists on disk, and any test asserting on that match would fail with no +// diagnosable cause. These pin the propagation at the tracker, which is where +// the error is born and where it could be dropped; Manager.SearchWorkspaceContent +// returns the first tracker error unchanged. + +// TestSearchContentReportsGitAdmissionStarvation covers a caller that gives up +// waiting for a Git slot that never frees. +func TestSearchContentReportsGitAdmissionStarvation(t *testing.T) { + tracker := newContentSearchFailureTracker(t) + + restoreCap := subproc.Git().SetCapForTest(1) + t.Cleanup(restoreCap) + hold, err := subproc.AcquireGit(context.Background(), subproc.GitInteractive) + if err != nil { + t.Fatalf("hold git slot: %v", err) + } + t.Cleanup(hold) + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + t.Cleanup(cancel) + results, err := tracker.SearchContent(ctx, "needle", 10) + if err == nil { + t.Fatalf("SearchContent returned nil error with %d results while Git admission "+ + "was starved; a starved search must not be reported as an empty workspace", + len(results)) + } +} + +// TestSearchContentReportsGitCommandTimeout covers a Git process that wedges +// past gitCommandTimeout and is killed. +func TestSearchContentReportsGitCommandTimeout(t *testing.T) { + tracker := newContentSearchFailureTracker(t) + + previousTimeout := gitCommandTimeout + gitCommandTimeout = 200 * time.Millisecond + t.Cleanup(func() { gitCommandTimeout = previousTimeout }) + shimDir := installSleepGitShim(t) + t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + results, err := tracker.SearchContent(context.Background(), "needle", 10) + if err == nil { + t.Fatalf("SearchContent returned nil error with %d results after the file-list "+ + "command was killed at its deadline; a failed search must not be reported "+ + "as an empty workspace", len(results)) + } +} + +// newContentSearchFailureTracker builds a tracker over a repository holding one +// match and asserts the match is findable while Git is healthy, so a later +// empty result cannot be blamed on the fixture. +func newContentSearchFailureTracker(t *testing.T) *WorkspaceTracker { + t.Helper() + repoDir, cleanup := setupTestRepo(t) + t.Cleanup(cleanup) + writeFile(t, repoDir, "match.txt", "workspace content search needle\n") + + tracker := NewWorkspaceTrackerForRepo(repoDir, "repo", newTestLogger(t)) + baseline, err := tracker.SearchContent(context.Background(), "needle", 10) + if err != nil { + t.Fatalf("baseline SearchContent returned error: %v", err) + } + if len(baseline) != 1 { + t.Fatalf("baseline result count = %d, want 1", len(baseline)) + } + return tracker +} diff --git a/apps/web/hooks/domains/session/use-workspace-content-search.test.ts b/apps/web/hooks/domains/session/use-workspace-content-search.test.ts index bb464dabef..0e74452af3 100644 --- a/apps/web/hooks/domains/session/use-workspace-content-search.test.ts +++ b/apps/web/hooks/domains/session/use-workspace-content-search.test.ts @@ -251,3 +251,46 @@ describe("useWorkspaceContentSearch stale responses", () => { expect(result.current.isSearching).toBe(false); }); }); + +describe("useWorkspaceContentSearch incremental publishing", () => { + // The retry loop exists so a repository that registers with the workspace + // process manager after task launch still contributes matches. Publishing + // the merged set only once the whole budget was spent put a fixed + // 250ms + 7 * 500ms = 3.75s floor under every search, which is what made + // the "cached preview content match" e2e spec fail its 5s assertion under + // CI load. This pins the per-attempt half of that contract: a repository + // whose matches only appear on a later attempt is published when that + // attempt lands, not when the budget runs out. + it("publishes a late repository's matches as soon as its attempt lands", async () => { + const primary = { + repository_name: "primary", + path: "src/primary.ts", + line: 1, + column: 1, + preview: "primary needle", + match_ranges: [], + }; + const extra = { + repository_name: "extra", + path: "src/extra.ts", + line: 2, + column: 1, + preview: "extra needle", + match_ranges: [], + }; + mockSearchWorkspaceContent + .mockResolvedValueOnce({ results: [primary] }) + .mockResolvedValue({ results: [primary, extra] }); + const { result } = renderHook(() => + useWorkspaceContentSearch({ enabled: true, query: "needle", sessionId: "session-1" }), + ); + + await act(async () => vi.advanceTimersByTimeAsync(250)); + expect(result.current.results).toEqual([primary]); + + // One retry delay later the second repository has registered. + await act(async () => vi.advanceTimersByTimeAsync(500)); + expect(result.current.results).toEqual([primary, extra]); + expect(result.current.isSearching).toBe(true); + }); +});