fix(search): include workspace root with submodules (#2524)

This commit is contained in:
José Almeida
2026-08-12 10:53:07 +01:00
committed by GitHub
parent 6033d5717c
commit 146419fc8c
13 changed files with 665 additions and 145 deletions
@@ -84,7 +84,7 @@ func (m *Manager) contentSearchTrackers() []*WorkspaceTracker {
}
return trackers
}
if root != nil && root.RepositoryName() != "" {
if root != nil && root.gitIndexPath != "" {
trackers = append(trackers, root)
}
trackers = append(trackers, repositories...)
@@ -81,8 +81,9 @@ func (wt *WorkspaceTracker) getFileListClass(ctx context.Context, class subproc.
// --cached: include tracked files
// --others: include untracked files
// --exclude-standard: respect .gitignore
// --stage: expose tracked file modes so submodule Gitlinks can be excluded
out, runErr, execCtxErr := subproc.RunGitOutputAfterAcquire(ctx, class, gitCommandTimeout, func(execCtx context.Context) *exec.Cmd {
cmd := subproc.NewGitCommand(execCtx, "ls-files", "--cached", "--others", "--exclude-standard")
cmd := subproc.NewGitCommand(execCtx, "ls-files", "--cached", "--others", "--exclude-standard", "--stage")
cmd.Dir = wt.workDir
return cmd
})
@@ -94,6 +95,12 @@ func (wt *WorkspaceTracker) getFileListClass(ctx context.Context, class subproc.
lines := strings.Split(string(out), "\n")
for _, line := range lines {
line = strings.TrimSpace(line)
if metadata, path, tracked := strings.Cut(line, "\t"); tracked {
if strings.HasPrefix(metadata, "160000 ") {
continue
}
line = strings.TrimSpace(path)
}
if line == "" || isRootOwnershipMarkerPath(line) {
continue
}
@@ -1010,7 +1017,7 @@ func (m *Manager) SearchWorkspaceFileResults(query string, limit int) []types.Fi
}
candidates := make([]fileSearchCandidate, 0)
if root != nil && root.RepositoryName() != "" {
if root != nil && root.gitIndexPath != "" {
candidates = appendTrackerFileSearchCandidates(candidates, root)
}
for _, tracker := range repositories {
@@ -0,0 +1,98 @@
package process
import (
"context"
"reflect"
"testing"
"github.com/kandev/kandev/internal/agentctl/server/config"
)
func TestManagerSearchWorkspaceFileResultsIncludesRootAndSubmodule(t *testing.T) {
mgr := setupWorkspaceSearchSubmoduleGraph(t)
updateWorkspaceSearchFileLists(mgr)
results := mgr.SearchWorkspaceFileResults("search-target", 20)
got := make([]string, 0, len(results))
for _, result := range results {
got = append(got, result.RepositoryName+"|"+result.Path)
}
want := []string{
"|root-search-target.txt",
"vendor/lib|vendor/lib/child-search-target.txt",
}
if !reflect.DeepEqual(got, want) {
t.Fatalf("search results = %v, want %v", got, want)
}
}
func TestManagerSearchWorkspaceFileResultsExcludesSubmoduleGitlink(t *testing.T) {
mgr := setupWorkspaceSearchSubmoduleGraph(t)
updateWorkspaceSearchFileLists(mgr)
for _, result := range mgr.SearchWorkspaceFileResults("lib", 20) {
if result.RepositoryName == "" && result.Path == "vendor/lib" {
t.Fatalf("root search returned submodule Gitlink as a file: %+v", result)
}
}
childFound := false
for _, result := range mgr.SearchWorkspaceFileResults("child-search-target", 20) {
if result.RepositoryName == "vendor/lib" && result.Path == "vendor/lib/child-search-target.txt" {
childFound = true
break
}
}
if !childFound {
t.Fatal("real file inside child scope was not searchable")
}
}
func TestManagerSearchWorkspaceContentIncludesRootAndSubmodule(t *testing.T) {
mgr := setupWorkspaceSearchSubmoduleGraph(t)
response, err := mgr.SearchWorkspaceContent(context.Background(), "shared-content-needle", 20)
if err != nil {
t.Fatalf("SearchWorkspaceContent: %v", err)
}
got := make([]string, 0, len(response.Results))
for _, result := range response.Results {
got = append(got, result.RepositoryName+"|"+result.Path)
}
want := []string{
"|root-search-target.txt",
"vendor/lib|child-search-target.txt",
}
if !reflect.DeepEqual(got, want) {
t.Fatalf("content search results = %v, want %v", got, want)
}
}
func setupWorkspaceSearchSubmoduleGraph(t *testing.T) *Manager {
t.Helper()
parent, parentCleanup := setupTestRepo(t)
t.Cleanup(parentCleanup)
child, childCleanup := setupTestRepo(t)
t.Cleanup(childCleanup)
writeFile(t, parent, "root-search-target.txt", "shared-content-needle\n")
writeFile(t, child, "child-search-target.txt", "shared-content-needle\n")
runGit(t, child, "add", ".")
runGit(t, child, "commit", "-m", "add search fixture")
runGit(t, parent, "-c", "protocol.file.allow=always", "submodule", "add", child, "vendor/lib")
runGit(t, parent, "add", ".")
runGit(t, parent, "commit", "-m", "add submodule search fixture")
runGit(t, parent, "push", "origin", "main")
mgr := NewManager(&config.InstanceConfig{WorkDir: parent}, newTestLogger(t))
t.Cleanup(mgr.stopWorkspaceTrackers)
return mgr
}
func updateWorkspaceSearchFileLists(mgr *Manager) {
root, repositories := mgr.snapshotTrackers()
root.updateFiles(context.Background())
for _, tracker := range repositories {
tracker.updateFiles(context.Background())
}
}
@@ -169,9 +169,19 @@ func TestRefreshStaleWorkspaceWatches_HealsUnwatchedRow(t *testing.T) {
// hands it to the background goroutine; the join it uses needs the real
// tasks schema, so drive the goroutine directly.
svc.refreshStaleWorkspaceWatches(testWorkspaceID, map[string]struct{}{"task-1": {}})
awaitTaskPRUpdated(t, updated)
// Drain the goroutine before the in-memory DB closes. Stop() cancels
// stopCtx, so it has to come after the write we just observed.
unwatchedUpdate := awaitTaskPRUpdated(t, updated)
if unwatchedUpdate.PRNumber != 1293 {
t.Fatalf("first background update PR = %d, want unwatched PR 1293", unwatchedUpdate.PRNumber)
}
// The same goroutine updates the watched PR after the unwatched row. Wait
// for that write too: canceling it via Stop can make sqlite discard the
// sole :memory: connection and leave the final assertion without a schema.
watchedUpdate := awaitTaskPRUpdated(t, updated)
if watchedUpdate.PRNumber != 1299 {
t.Fatalf("second background update PR = %d, want watched PR 1299", watchedUpdate.PRNumber)
}
// Drain the goroutine before the in-memory DB closes. Both observable writes
// have completed, so Stop has no database operation left to cancel.
svc.Stop()
got, err := store.GetTaskPRByRepoAndNumber(ctx, "task-1", "repo-1", 1293)
+45
View File
@@ -2,6 +2,7 @@ import { test, expect } from "../fixtures/test-base";
import { KanbanPage } from "../pages/kanban-page";
import { SessionPage } from "../pages/session-page";
import { waitForSessionDone } from "../helpers/session";
import { createSubmoduleReviewFixture } from "./review/submodule-review-helpers";
type CommandAliasExpectation = {
query: string;
@@ -231,6 +232,50 @@ test.describe("Command Panel", () => {
await expect(dialog.getByText("Type to search files...")).toBeVisible();
});
test("Cmd+Shift+K finds root and submodule files", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
test.setTimeout(180_000);
const fixture = await createSubmoduleReviewFixture(
apiClient,
seedData,
backend.tmpDir,
"Submodule file search E2E",
);
try {
await testPage.goto(`/t/${fixture.taskId}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await session.waitForChatIdle({ timeout: 45_000 });
await fixture.waitForWorktree(apiClient);
await openFileSearch(testPage);
const dialog = commandDialog(testPage);
await expect(dialog).toBeVisible({ timeout: 5_000 });
await dialog.getByRole("combobox").fill("README.md");
const groups = dialog.getByTestId("file-search-repo-group");
await expect(groups).toHaveCount(3, { timeout: 45_000 });
await expect(
dialog.locator('[data-testid="file-search-repo-group"][data-repository=""]'),
).toBeVisible();
await expect(
dialog.locator('[data-testid="file-search-repo-group"][data-repository="vendor/outer"]'),
).toBeVisible();
await expect(
dialog.locator(
'[data-testid="file-search-repo-group"][data-repository="vendor/outer/vendor/inner"]',
),
).toBeVisible();
} finally {
fixture.cleanup();
}
});
test("Cmd+K inline task search shows matching tasks", async ({
testPage,
apiClient,
@@ -1,5 +1,7 @@
import { test, expect } from "../../fixtures/test-base";
import type { Page } from "@playwright/test";
import type { ApiClient } from "../../helpers/api-client";
import type { Workspace } from "../../../lib/types/http";
// WORKSPACE_PICKER (Cmd/Ctrl+E by default) opens the sidebar-header
// workspace switcher with focus already inside the menu, so a workspace can be
@@ -24,108 +26,121 @@ async function listedWorkspaceIds(page: Page): Promise<string[]> {
return testIds.map((testId) => testId.replace("sidebar-workspace-item-", ""));
}
async function withDisposableWorkspace(
apiClient: ApiClient,
name: string,
run: (workspace: Workspace) => Promise<void>,
): Promise<void> {
const workspace = await apiClient.createWorkspace(name);
try {
await run(workspace);
} finally {
await apiClient.deleteWorkspace(workspace.id, workspace.name);
}
}
test.describe("Workspace picker shortcut (global)", () => {
test("opens the picker and switches workspace with arrows and Enter", async ({
testPage,
apiClient,
}) => {
const second = await apiClient.createWorkspace("Picker Shortcut Workspace");
await withDisposableWorkspace(apiClient, "Picker Shortcut Workspace", async (second) => {
await testPage.goto("/");
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toBeVisible({
timeout: 10_000,
});
await testPage.goto("/");
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toBeVisible({
timeout: 10_000,
await pressWorkspacePickerShortcut(testPage);
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toHaveAttribute(
"data-state",
"open",
);
const workspaceIds = await listedWorkspaceIds(testPage);
expect(workspaceIds).toContain(second.id);
expect(workspaceIds.length).toBeGreaterThanOrEqual(2);
// The shortcut opens a controlled menu. Wait for Radix's focus handoff
// before sending navigation keys, otherwise the first ArrowDown can land
// on the document body on a busy CI runner.
await expect(testPage.getByRole("menu")).toBeFocused();
// Focus lands on the menu itself, so the first ArrowDown moves to item 1
// and the second to item 2, with no mouse needed at any point.
await testPage.keyboard.press("ArrowDown");
await expect(testPage.getByTestId(`sidebar-workspace-item-${workspaceIds[0]}`)).toBeFocused();
await testPage.keyboard.press("ArrowDown");
await expect(testPage.getByTestId(`sidebar-workspace-item-${workspaceIds[1]}`)).toBeFocused();
await testPage.keyboard.press("Enter");
await expect(testPage).toHaveURL(
(url) => url.searchParams.get("workspaceId") === workspaceIds[1],
{ timeout: 10_000 },
);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).not.toBeVisible();
});
await pressWorkspacePickerShortcut(testPage);
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toHaveAttribute(
"data-state",
"open",
);
const workspaceIds = await listedWorkspaceIds(testPage);
expect(workspaceIds).toContain(second.id);
expect(workspaceIds.length).toBeGreaterThanOrEqual(2);
// The shortcut opens a controlled menu. Wait for Radix's focus handoff
// before sending navigation keys, otherwise the first ArrowDown can land
// on the document body on a busy CI runner.
await expect(testPage.getByRole("menu")).toBeFocused();
// Focus lands on the menu itself, so the first ArrowDown moves to item 1
// and the second to item 2 — no mouse needed at any point.
await testPage.keyboard.press("ArrowDown");
await expect(testPage.getByTestId(`sidebar-workspace-item-${workspaceIds[0]}`)).toBeFocused();
await testPage.keyboard.press("ArrowDown");
await expect(testPage.getByTestId(`sidebar-workspace-item-${workspaceIds[1]}`)).toBeFocused();
await testPage.keyboard.press("Enter");
await expect(testPage).toHaveURL(
(url) => url.searchParams.get("workspaceId") === workspaceIds[1],
{ timeout: 10_000 },
);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).not.toBeVisible();
});
test("expands a collapsed sidebar before opening the picker", async ({ testPage, apiClient }) => {
await apiClient.createWorkspace("Collapsed Picker Workspace");
await withDisposableWorkspace(apiClient, "Collapsed Picker Workspace", async () => {
await testPage.goto("/");
const sidebar = testPage.getByTestId("app-sidebar");
await expect(sidebar).toBeVisible({ timeout: 10_000 });
await testPage.goto("/");
const sidebar = testPage.getByTestId("app-sidebar");
await expect(sidebar).toBeVisible({ timeout: 10_000 });
await testPage.getByRole("button", { name: "Collapse sidebar" }).click();
await expect(sidebar).toHaveAttribute("data-collapsed", "true");
await testPage.getByRole("button", { name: "Collapse sidebar" }).click();
await expect(sidebar).toHaveAttribute("data-collapsed", "true");
await pressWorkspacePickerShortcut(testPage);
await pressWorkspacePickerShortcut(testPage);
await expect(sidebar).toHaveAttribute("data-collapsed", "false");
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toHaveAttribute(
"data-state",
"open",
);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).toBeVisible();
await expect(sidebar).toHaveAttribute("data-collapsed", "false");
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toHaveAttribute(
"data-state",
"open",
);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).toBeVisible();
});
});
test("Escape closes the picker and leaves the workspace unchanged", async ({
testPage,
apiClient,
}) => {
await apiClient.createWorkspace("Escape Picker Workspace");
await withDisposableWorkspace(apiClient, "Escape Picker Workspace", async () => {
await testPage.goto("/");
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toBeVisible({
timeout: 10_000,
});
const urlBefore = testPage.url();
await testPage.goto("/");
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toBeVisible({
timeout: 10_000,
await pressWorkspacePickerShortcut(testPage);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).toBeVisible();
await testPage.keyboard.press("Escape");
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).not.toBeVisible();
expect(testPage.url()).toBe(urlBefore);
// The store flag must have cleared on dismiss, or a second press would be
// a no-op (open -> open).
await pressWorkspacePickerShortcut(testPage);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).toBeVisible();
});
const urlBefore = testPage.url();
await pressWorkspacePickerShortcut(testPage);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).toBeVisible();
await testPage.keyboard.press("Escape");
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).not.toBeVisible();
expect(testPage.url()).toBe(urlBefore);
// The store flag must have cleared on dismiss, or a second press would be
// a no-op (open -> open).
await pressWorkspacePickerShortcut(testPage);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).toBeVisible();
});
test("does nothing below the sidebar's md boundary", async ({ testPage, apiClient }) => {
await apiClient.createWorkspace("Narrow Viewport Workspace");
await withDisposableWorkspace(apiClient, "Narrow Viewport Workspace", async () => {
// 700px sits in the 640-767px band: the sidebar (and picker trigger) is
// display:none, but the menu portals to document.body — without the
// viewport guard it opened detached at the corner with a scroll lock.
await testPage.setViewportSize({ width: 700, height: 800 });
await testPage.goto("/");
await testPage.waitForLoadState("networkidle");
// 700px sits in the 640-767px band: the sidebar (and picker trigger) is
// display:none, but the menu portals to document.body — without the
// viewport guard it opened detached at the corner with a scroll lock.
await testPage.setViewportSize({ width: 700, height: 800 });
await testPage.goto("/");
await testPage.waitForLoadState("networkidle");
await pressWorkspacePickerShortcut(testPage);
await pressWorkspacePickerShortcut(testPage);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).not.toBeVisible();
await expect(testPage.locator("body")).not.toHaveAttribute("data-scroll-locked", /.+/);
await expect(testPage.locator(WORKSPACE_ITEM_SELECTOR).first()).not.toBeVisible();
await expect(testPage.locator("body")).not.toHaveAttribute("data-scroll-locked", /.+/);
});
});
});
@@ -39,12 +39,16 @@ test.describe("Office workspace deletion fallback navigation", () => {
await confirmButton.click();
await expect(testPage).toHaveURL(
(url) => url.pathname === "/" && url.searchParams.get("workspaceId") === seedData.workspaceId,
(url) => url.pathname === "/" && Boolean(url.searchParams.get("workspaceId")),
{ timeout: 10_000 },
);
await expect(testPage).not.toHaveURL(/\/office\/setup/);
const destinationWorkspaceId = new URL(testPage.url()).searchParams.get("workspaceId");
const { workspaces } = await apiClient.listWorkspaces();
expect(workspaces.some((item) => item.id === onboarded.workspaceId)).toBe(false);
const destinationWorkspace = workspaces.find((item) => item.id === destinationWorkspaceId);
expect(destinationWorkspace).toBeDefined();
expect(destinationWorkspace?.office_workflow_id).toBeFalsy();
});
});
@@ -80,69 +80,73 @@ export async function createSubmoduleReviewFixture(
const outerPath = path.join(sourceRoot, "outer");
const innerPath = path.join(sourceRoot, "inner");
const env = { ...makeGitEnv(tempRoot), GIT_ALLOW_PROTOCOL: "file" };
const cleanup = () => fs.rmSync(sourceRoot, { recursive: true, force: true });
initializeRepository(innerPath, env, "README.md", "inner base\n");
try {
initializeRepository(innerPath, env, "README.md", "inner base\n");
initializeRepository(outerPath, env, "README.md", "outer base\n");
runGit(outerPath, [...GIT_PROTOCOL_ARGS, "submodule", "add", "../inner", "vendor/inner"], env);
commit(outerPath, env, "add nested inner submodule");
initializeRepository(outerPath, env, "README.md", "outer base\n");
runGit(outerPath, [...GIT_PROTOCOL_ARGS, "submodule", "add", "../inner", "vendor/inner"], env);
commit(outerPath, env, "add nested inner submodule");
initializeRepository(parentPath, env, "README.md", "parent base\n");
runGit(parentPath, [...GIT_PROTOCOL_ARGS, "submodule", "add", "../outer", "vendor/outer"], env);
runGit(parentPath, [...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"], env);
runGit(
path.join(parentPath, "vendor/outer"),
[...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"],
env,
);
commit(parentPath, env, "add outer submodule");
initializeRepository(parentPath, env, "README.md", "parent base\n");
runGit(parentPath, [...GIT_PROTOCOL_ARGS, "submodule", "add", "../outer", "vendor/outer"], env);
runGit(parentPath, [...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"], env);
runGit(
path.join(parentPath, "vendor/outer"),
[...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"],
env,
);
commit(parentPath, env, "add outer submodule");
const repository = await apiClient.createRepository(seedData.workspaceId, parentPath, "main", {
name: "nested-submodule-parent",
});
const { executors } = await apiClient.listExecutors();
const directProfile = executors.find(
(executor) => executor.type === "local" || executor.type === "local_pc",
)?.profiles?.[0];
if (!directProfile) throw new Error("Nested submodule fixture needs a direct local executor");
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
title,
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [repository.id],
executor_profile_id: directProfile.id,
},
);
if (!task.session_id) throw new Error("Nested submodule fixture did not start a session");
const repository = await apiClient.createRepository(seedData.workspaceId, parentPath, "main", {
name: "nested-submodule-parent",
});
const { executors } = await apiClient.listExecutors();
const directProfile = executors.find(
(executor) => executor.type === "local" || executor.type === "local_pc",
)?.profiles?.[0];
if (!directProfile) throw new Error("Nested submodule fixture needs a direct local executor");
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
title,
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [repository.id],
executor_profile_id: directProfile.id,
},
);
if (!task.session_id) throw new Error("Nested submodule fixture did not start a session");
return {
taskId: task.id,
sessionId: task.session_id,
sourceRoot,
waitForWorktree: (client) => waitForWorktreePath(client, task.id, task.session_id!),
applyNestedChanges(worktreePath: string) {
const outerWorktree = path.join(worktreePath, "vendor/outer");
const innerWorktree = path.join(outerWorktree, "vendor/inner");
if (!fs.existsSync(innerWorktree)) {
runGit(
worktreePath,
[...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"],
env,
);
}
fs.appendFileSync(path.join(worktreePath, "README.md"), "parent working-tree change\n");
fs.appendFileSync(path.join(outerWorktree, "README.md"), "outer committed change\n");
commit(outerWorktree, env, "change outer submodule");
fs.appendFileSync(path.join(innerWorktree, "README.md"), "inner committed change\n");
commit(innerWorktree, env, "change inner submodule");
commit(outerWorktree, env, "record inner submodule change");
},
cleanup() {
fs.rmSync(sourceRoot, { recursive: true, force: true });
},
};
return {
taskId: task.id,
sessionId: task.session_id,
sourceRoot,
waitForWorktree: (client) => waitForWorktreePath(client, task.id, task.session_id!),
applyNestedChanges(worktreePath: string) {
const outerWorktree = path.join(worktreePath, "vendor/outer");
const innerWorktree = path.join(outerWorktree, "vendor/inner");
if (!fs.existsSync(innerWorktree)) {
runGit(
worktreePath,
[...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"],
env,
);
}
fs.appendFileSync(path.join(worktreePath, "README.md"), "parent working-tree change\n");
fs.appendFileSync(path.join(outerWorktree, "README.md"), "outer committed change\n");
commit(outerWorktree, env, "change outer submodule");
fs.appendFileSync(path.join(innerWorktree, "README.md"), "inner committed change\n");
commit(innerWorktree, env, "change inner submodule");
commit(outerWorktree, env, "record inner submodule change");
},
cleanup,
};
} catch (error) {
cleanup();
throw error;
}
}
@@ -1,8 +1,29 @@
import crypto from "node:crypto";
import fs from "node:fs";
import path from "node:path";
import { test, expect } from "../../fixtures/test-base";
import { SessionPage } from "../../pages/session-page";
import path from "node:path";
import { createSubmoduleReviewFixture, readGitValue } from "./submodule-review-helpers";
test.describe("Submodule review fixture", () => {
test("cleans source repositories when setup fails", async ({ apiClient, seedData, backend }) => {
const tempRoot = fs.mkdtempSync(path.join(backend.tmpDir, "submodule-review-failure-"));
try {
await expect(
createSubmoduleReviewFixture(
apiClient,
{ ...seedData, workspaceId: crypto.randomUUID() },
tempRoot,
"Failing submodule fixture E2E",
),
).rejects.toThrow();
expect(fs.readdirSync(tempRoot)).toEqual([]);
} finally {
fs.rmSync(tempRoot, { recursive: true, force: true });
}
});
});
test.describe("Nested submodule Review", () => {
test.describe.configure({ retries: 1, timeout: 180_000 });
@@ -0,0 +1,125 @@
---
spec: docs/specs/ui/task-workspace-content-search.md
created: 2026-08-11
status: implemented
---
# Implementation Plan: Preserve Root Search With Submodules
## Overview
Fix workspace search tracker selection so a real Git root remains eligible when
initialized submodules add named trackers. Prove the behavior first at the
agentctl manager boundary, preserve bare multi-repository task-root behavior,
then exercise Cmd/Ctrl+Shift+K through the existing command palette.
## Confirmed Root Cause
`SearchWorkspaceFileResults` and `contentSearchTrackers` include the root in a
mixed tracker graph only when `RepositoryName()` is non-empty. A real workspace
Git root intentionally uses the empty repository identity, so adding the first
submodule changes both searches from root-only to child-only. The correct
distinction is whether the root owns Git metadata, which `RepositoryScopes`
already represents with `gitIndexPath != ""`.
Retaining the root exposed a second file-enumeration defect: the root's
`git ls-files --cached --others --exclude-standard` output includes mode-160000
submodule Gitlinks, but every output line was classified as a file. Searching
the submodule directory name could therefore return a directory as a root file.
---
## Backend
### Search tracker selection
- Update `apps/backend/internal/agentctl/server/process/workspace_files.go` and
`workspace_content_search.go` to include the unnamed root when it owns a real
Git repository, even when named submodule trackers coexist.
- Keep the current single-repository fallback and named sibling-repository
aggregation unchanged. Do not search a bare task root or change response
paths, repository identities, ordering, limits, or fuzzy scoring.
- Use the same real-Git-root predicate as `Manager.RepositoryScopes`; do not
infer repository ownership from the display/transport name.
- Preserve tracked Git modes while enumerating files and exclude mode-160000
Gitlinks. Continue including untracked, non-ignored files.
## Frontend
No production frontend change is planned. The command palette already renders
the structured `results` returned by `workspace.files.search` without filtering
the unnamed root, and content search already groups the same empty root identity.
## Tests
- **What:** File-name search returns matches from a Git root and initialized
submodule in one response.
**File:** `apps/backend/internal/agentctl/server/process/workspace_search_submodule_test.go`.
**How:** Construct real parent/child Git repositories, add the child as an
initialized submodule, refresh tracker file lists, and assert both structured
repository identities and task-root-relative paths.
- **What:** Content search returns matches from the same mixed root/submodule
graph.
**File:** `apps/backend/internal/agentctl/server/process/workspace_search_submodule_test.go`.
**How:** Search matching tracked content through `Manager.SearchWorkspaceContent`
and assert one result from each scope.
- **What:** Bare task roots containing named sibling repositories remain
excluded.
**Files:** Existing `workspace_file_search_test.go` and
`workspace_content_search_test.go` coverage.
**How:** Rerun the existing multi-repository tests beside the new regressions.
- **What:** Searching the exact submodule directory name does not return the
parent Gitlink as a file, while the child tracker's real files remain eligible.
**File:** `apps/backend/internal/agentctl/server/process/workspace_search_submodule_test.go`.
**How:** Search the real initialized fixture for `lib`, reject the root
`vendor/lib` pseudo-file, and retain the existing child-file assertion.
## E2E Tests
- **Scenario:** Given a task whose Git root and initialized submodules each
contain `README.md`, when the user opens Files with Cmd/Ctrl+Shift+K and
searches for that name, root and child repository groups all appear.
- **File:** `apps/web/e2e/tests/command-panel.spec.ts`.
- **What to verify:** Reuse `createSubmoduleReviewFixture`, wait for the task
session/worktree, search through the command palette, and assert the empty
root plus direct and nested submodule group identities. Always remove the
disposable source tree in `finally`.
- No mobile-specific E2E is added. This fix changes shared backend result data,
not layout, touch behavior, navigation, scrolling, or viewport composition;
the existing mobile file-search interaction consumes the same API contract.
## Verification Results
Task 01 passed its focused gate: 3 process tests and 1 API test. Both new
regressions failed for the expected child-only result before their production
changes, then passed; the final full run of both touched backend packages also
passed. Task 02 passed 1 Chromium test after its RED run showed the original
bug's 2 groups instead of the expected root plus 2 submodules. Review
remediation added and passed setup-failure cleanup coverage for the shared E2E
fixture, then revalidated the command-palette happy path with retries disabled.
PR CI remediation also removed workspace-picker fixture leakage and corrected
two order-dependent E2E expectations. The two affected specs passed together
with one worker, and exact Chromium/mobile shard 4/14 passed 137 tests with 7
intentional skips and retries disabled.
Independent review then found that retaining the root also exposed its
mode-160000 Gitlink as a pseudo-file. A focused regression failed on
`vendor/lib`, passed after mode-aware enumeration excluded Gitlinks, and the
four-test behavior gate, both touched backend packages, and CI-style
changed-file lint all passed.
## Implementation Waves And Parallel Candidates
Default execution is sequential in the primary conversation. No task is marked
parallel-safe because the browser proof depends on the backend behavior.
Wave 1:
- [x] [Task 01: Correct mixed-graph search](task-01-correct-mixed-graph-search.md)
Wave 2:
- [x] [Task 02: Prove command-palette search](task-02-prove-command-palette-search.md)
## Open Questions
None.
@@ -0,0 +1,96 @@
---
id: "01-correct-mixed-graph-search"
title: "Correct mixed-graph search"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/ui/task-workspace-content-search.md"
---
# Task 01: Correct Mixed-Graph Search
## Acceptance
- File-name and content search each return root and submodule matches from a
mixed unnamed-root/named-child tracker graph.
- Bare task roots with named sibling repositories remain excluded, with no
duplicate results or search failure.
- Parent repository Gitlinks are excluded from file results; real files owned
by initialized child trackers remain searchable.
- Response identities, paths, ranking, limits, and existing single/multi-repo
behavior remain unchanged.
## Verification
```bash
cd apps/backend && go test ./internal/agentctl/server/process ./internal/agentctl/server/api -run '^(TestManagerSearchWorkspaceFileResultsIncludesRootAndSubmodule|TestManagerSearchWorkspaceFileResultsExcludesSubmoduleGitlink|TestManagerSearchWorkspaceContentIncludesRootAndSubmodule|TestManagerSearchWorkspaceContentGroupsResultsByRepository|TestGetFileList_HidesOnlyRootOwnershipMarker|TestHandleFileSearchIncludesEveryTaskRepository)$' -count=1 -v
```
## Files likely touched
- `apps/backend/internal/agentctl/server/process/workspace_files.go`
- `apps/backend/internal/agentctl/server/process/workspace_content_search.go`
- `apps/backend/internal/agentctl/server/process/workspace_search_submodule_test.go` (new)
## Dependencies
None.
## Parallelism
`sequential` — both search paths share the same tracker-graph invariant and
regression fixture.
## Inputs
- Spec bullets and scenarios for mixed root/submodule and bare multi-repo search.
- `Manager.RepositoryScopes` as the existing real-Git-root predicate.
- `manager_submodule_test.go` and `workspace_tracker_test.go` Git fixture helpers.
- Confirmed failing repro: searching a parent file plus submodule file returned
only the submodule path.
## Output contract
Report RED and GREEN commands/results, root-selection change, exact files
changed, bare-root preservation, cleanup, blockers/risks, and synchronized
task/plan status.
## Results
Implemented mixed-graph root retention for file-name and content search by
using the same real-Git-root predicate as `Manager.RepositoryScopes`. The new
fixture builds real parent/child repositories and an initialized submodule;
existing bare multi-repository tests preserve the exclusion boundary.
RED:
- `go test ./internal/agentctl/server/process -run '^TestManagerSearchWorkspaceFileResultsIncludesRootAndSubmodule$' -count=1 -v`
failed as expected: only `vendor/lib|vendor/lib/child-search-target.txt`
was returned.
- `go test ./internal/agentctl/server/process -run '^TestManagerSearchWorkspaceContentIncludesRootAndSubmodule$' -count=1 -v`
failed as expected: only `vendor/lib|child-search-target.txt` was returned.
GREEN:
- Each focused regression passed after its corresponding one-line production
change.
- Final task gate:
`go test ./internal/agentctl/server/process ./internal/agentctl/server/api -run '^(TestManagerSearchWorkspaceFileResultsIncludesRootAndSubmodule|TestManagerSearchWorkspaceContentIncludesRootAndSubmodule|TestManagerSearchWorkspaceContentGroupsResultsByRepository|TestHandleFileSearchIncludesEveryTaskRepository)$' -count=1 -v`
passed 3 process tests and 1 API test.
- Final package audit:
`go test ./internal/agentctl/server/process ./internal/agentctl/server/api -count=1`
passed both touched packages.
- Independent-review remediation RED:
`go test ./internal/agentctl/server/process -run '^TestManagerSearchWorkspaceFileResultsExcludesSubmoduleGitlink$' -count=1 -v`
failed because root search returned `{Path:vendor/lib}` as a file.
- Independent-review remediation GREEN: the same focused regression passed
after file enumeration retained tracked modes and excluded mode-160000
Gitlinks. The four-test behavior gate passed, including tracked root/child,
content-search, and untracked-file coverage. Both touched packages passed
again, and the CI-style changed-file lint reported 0 issues against base
`66eb87ac307db76b8cb3ba5fcfae73ff6d7d3e6c`.
Production and regression files match **Files likely touched**; the spec and
plan were synchronized with the remediation. Temporary artifacts: none.
Blockers/remaining Task 01 risks: none.
@@ -0,0 +1,77 @@
---
id: "02-prove-command-palette-search"
title: "Prove command-palette search"
status: done
wave: 2
depends_on: ["01-correct-mixed-graph-search"]
plan: "plan.md"
spec: "../../specs/ui/task-workspace-content-search.md"
---
# Task 02: Prove Command-Palette Search
## Acceptance
- Cmd/Ctrl+Shift+K shows matching files from the unnamed Git root, direct
submodule, and nested submodule in their existing repository groups.
- The test drives the user-facing palette, reuses the established disposable
submodule fixture, and cleans its source tree even after failure.
- No production UI or mobile composition changes are introduced.
## Verification
```bash
cd apps && pnpm install --frozen-lockfile
cd apps/web && pnpm e2e:run tests/command-panel.spec.ts -- --grep 'finds root and submodule files'
cd apps/web && pnpm e2e:run --no-build tests/review/submodule-review.spec.ts -- --grep 'cleans source repositories when setup fails' --retries=0
```
## Files likely touched
- `apps/web/e2e/tests/command-panel.spec.ts`
## Dependencies
Task 01.
## Parallelism
`sequential` — browser proof requires the corrected backend aggregation.
## Inputs
- Spec scenario for Files search across a Git root and initialized submodules.
- `apps/web/e2e/tests/review/submodule-review-helpers.ts` for disposable Git
graph creation, task startup, worktree readiness, and cleanup.
- Existing `openFileSearch` and command-dialog helpers in `command-panel.spec.ts`.
- Mobile-parity exception for shared data-only changes with no rendered,
touch, navigation, scrolling, or breakpoint change.
## Output contract
Report the discovered Playwright project/test count, exact E2E command/result,
group assertions, disposable fixture cleanup, changed files, blockers/risks,
and synchronized task/plan status.
## Results
- `pnpm install --frozen-lockfile` completed successfully from `apps/`.
- RED: with the original file-search predicate restored in the built backend,
`pnpm e2e:run --no-build tests/command-panel.spec.ts -- --grep 'finds root and submodule files'`
discovered one Chromium test and failed with 2 repository groups instead of
3; the unnamed root group was missing.
- GREEN: after restoring the fix and rebuilding,
`pnpm e2e:run tests/command-panel.spec.ts -- --grep 'finds root and submodule files'`
passed 1 Chromium test. It asserted the unnamed root, `vendor/outer`, and
`vendor/outer/vendor/inner` groups.
- Review remediation moved setup-failure cleanup into the shared fixture helper
while preserving caller cleanup after successful setup. The focused regression
failed RED with one leaked directory, then passed GREEN; the command-palette
happy path also passed with retries disabled.
- PR CI remediation made workspace-picker workspaces disposable, aligned its
focus assertion with Radix's initial item focus, and made office deletion
accept any remaining kanban workspace. Both specs passed together (5 tests),
then exact Chromium/mobile shard 4/14 passed 137 tests with 7 intentional
skips and retries disabled.
- No production UI or mobile composition changed; both use the same corrected
backend response.
@@ -36,6 +36,11 @@ repositories.
paths so same-named files remain distinguishable and open in the correct
repository. The palette groups those matches by repository and shows paths
relative to each group.
- For both **Files** and **Contents**, a Git workspace root remains searchable
when initialized submodules add named repository scopes. Root matches retain
the empty repository identity while child matches retain their task-root-relative
scope names. A bare task directory containing sibling repositories is not
treated as an additional repository.
- Search covers every repository materialized for the active task session.
Tracked files and untracked, non-ignored files are eligible; ignored files,
directories, and workspace metadata are not.
@@ -135,6 +140,19 @@ repositories without parsing path strings.
- **GIVEN** a task with two repositories containing matching file names,
**WHEN** the user searches in **Files** mode, **THEN** matching paths from both
repositories appear with their repository prefixes.
- **GIVEN** a Git workspace root and an initialized submodule both contain
matching file names, **WHEN** the user searches in **Files** mode, **THEN**
matches from the root and submodule appear in separate repository groups.
- **GIVEN** an initialized submodule path matches a file-search query, **WHEN**
the user searches in **Files** mode, **THEN** the parent repository's Gitlink
path is not returned as a file and real files inside the child scope remain
searchable.
- **GIVEN** a Git workspace root and an initialized submodule both contain a
matching text query, **WHEN** the user searches in **Contents** mode,
**THEN** results from both repository scopes appear.
- **GIVEN** a bare task directory contains multiple sibling repositories,
**WHEN** the user searches in **Files** or **Contents** mode, **THEN** only
those repositories contribute results and no duplicate task-root group appears.
- **GIVEN** two repositories with the same relative path, **WHEN** the user
selects the result from the second repository, **THEN** Kandev opens the
second repository's file at the returned line and column.