diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts index 154939e824..9117f7cf91 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts @@ -24,6 +24,7 @@ import { } from "@/lib/types/http"; import type { KanbanState } from "@/lib/state/slices"; import { findTaskInSnapshots } from "@/lib/kanban/find-task"; +import { repositorySlug } from "@/lib/repository-slug"; import { resolvePreferredSessionId } from "../task-select-helpers"; // Map workflow snapshot to kanban state on workspace switch. @@ -162,7 +163,7 @@ export function useSheetData(workspaceId: string | null) { const repositories = workspaceId ? (repositoriesByWorkspace[workspaceId] ?? []) : []; const ctx: SheetItemCtx = { repositoryPathsById: new Map( - repositories.map((repo: Repository) => [repo.id, repo.local_path]), + repositories.map((repo: Repository) => [repo.id, repositorySlug(repo)]), ), workflowNameById: new Map(workflows.map((w) => [w.id, w.name])), stepTitleById: new Map(allSteps.map((s) => [s.id, s.title])), diff --git a/apps/web/components/task/sidebar-filter/use-filter-value-options.test.ts b/apps/web/components/task/sidebar-filter/use-filter-value-options.test.ts index 97f93b68a2..1b0735f803 100644 --- a/apps/web/components/task/sidebar-filter/use-filter-value-options.test.ts +++ b/apps/web/components/task/sidebar-filter/use-filter-value-options.test.ts @@ -1,6 +1,24 @@ import { describe, it, expect } from "vitest"; import type { WorkflowSnapshotData } from "@/lib/state/slices/kanban/types"; -import { workflowStepOptions } from "./use-filter-value-options"; +import type { Repository } from "@/lib/types/http"; +import { repositorySlug } from "@/lib/repository-slug"; +import { repositoryOptions, workflowStepOptions } from "./use-filter-value-options"; + +function repo(overrides: Partial): Repository { + return { + id: "r1", + workspace_id: "w1", + name: "", + source_type: "local", + local_path: "", + provider: "", + provider_repo_id: "", + provider_owner: "", + provider_name: "", + default_branch: "main", + ...overrides, + } as Repository; +} function snapshot( id: string, @@ -20,6 +38,25 @@ function snapshot( }; } +describe("repositoryOptions (#1213)", () => { + // The option value becomes the saved filter clause value. It MUST equal what + // the task board sets as each task's repositoryPath (repositorySlug) — for a + // local repo that is the repo name, NOT the full local_path. Using local_path + // here was the bug: the clause matched no task and the board went empty. + it("uses repositorySlug as the option value (local repo → name, not local_path)", () => { + const local = repo({ name: "kandev", local_path: "/home/carlos/Projects/kandev" }); + const provider = repo({ provider_owner: "kdlbs", provider_name: "kandev-web" }); + const options = repositoryOptions({ w1: [local, provider] }); + expect(options).toEqual([ + { value: repositorySlug(local), label: repositorySlug(local) }, + { value: repositorySlug(provider), label: repositorySlug(provider) }, + ]); + expect(options[0].value).toBe("kandev"); + expect(options[0].value).not.toBe(local.local_path); + expect(options[1].value).toBe("kdlbs/kandev-web"); + }); +}); + describe("workflowStepOptions", () => { it("tags each step with its workflow name as the group", () => { const snapshots = { diff --git a/apps/web/components/task/sidebar-filter/use-filter-value-options.ts b/apps/web/components/task/sidebar-filter/use-filter-value-options.ts index 7ac6522566..2cceddf631 100644 --- a/apps/web/components/task/sidebar-filter/use-filter-value-options.ts +++ b/apps/web/components/task/sidebar-filter/use-filter-value-options.ts @@ -5,6 +5,7 @@ import { useAppStore } from "@/components/state-provider"; import type { AppState } from "@/lib/state/store"; import type { FilterDimension } from "@/lib/state/slices/ui/sidebar-view-types"; import { getExecutorLabel } from "@/lib/executor-icons"; +import { repositorySlug } from "@/lib/repository-slug"; type Option = { value: string; label: string; color?: string; group?: string }; type Snapshots = AppState["kanbanMulti"]["snapshots"]; @@ -45,11 +46,10 @@ function executorTypeOptions(snapshots: Snapshots): Option[] { return [...seen].sort().map((v) => ({ value: v, label: getExecutorLabel(v) })); } -function repositoryOptions(repositoriesByWorkspace: ReposByWorkspace): Option[] { +export function repositoryOptions(repositoriesByWorkspace: ReposByWorkspace): Option[] { const repos = Object.values(repositoriesByWorkspace).flat(); return repos.map((r) => { - const slug = - r.provider_owner && r.provider_name ? `${r.provider_owner}/${r.provider_name}` : r.local_path; + const slug = repositorySlug(r); return { value: slug, label: slug }; }); } diff --git a/apps/web/components/task/task-session-sidebar.tsx b/apps/web/components/task/task-session-sidebar.tsx index bb128d55cd..fe5416f510 100644 --- a/apps/web/components/task/task-session-sidebar.tsx +++ b/apps/web/components/task/task-session-sidebar.tsx @@ -24,6 +24,7 @@ import { useSidebarTaskPrefs } from "@/hooks/domains/sidebar/use-sidebar-task-pr import { useTaskActions, useArchiveAndSwitchTask } from "@/hooks/use-task-actions"; import { useTaskRemoval } from "@/hooks/use-task-removal"; import { findTaskInSnapshots } from "@/lib/kanban/find-task"; +import { repositorySlug } from "@/lib/repository-slug"; import { buildSwitchToSession, selectTaskWithLayout } from "./task-select-helpers"; import { getSessionInfoForTask } from "@/lib/utils/session-info"; import { getWebSocketClient } from "@/lib/ws/connection"; @@ -239,12 +240,7 @@ function useSidebarData(workspaceId: string | null) { const tasksWithRepositories = useMemo(() => { const repositories = workspaceId ? (repositoriesByWorkspace[workspaceId] ?? []) : []; const repositorySlugById = new Map( - repositories.map((repo: Repository) => [ - repo.id, - repo.provider_owner && repo.provider_name - ? `${repo.provider_owner}/${repo.provider_name}` - : repo.name || repo.local_path?.split("/").filter(Boolean).pop() || repo.local_path, - ]), + repositories.map((repo: Repository) => [repo.id, repositorySlug(repo)]), ); const titleById = new Map(allTasks.map((t) => [t.id, t.title])); const workflowNameById = new Map(workflows.map((w) => [w.id, w.name])); diff --git a/apps/web/e2e/pages/sidebar-filter-popover.ts b/apps/web/e2e/pages/sidebar-filter-popover.ts index 6aac42a374..640e3c6c35 100644 --- a/apps/web/e2e/pages/sidebar-filter-popover.ts +++ b/apps/web/e2e/pages/sidebar-filter-popover.ts @@ -114,6 +114,15 @@ export class SidebarFilterPopoverPage { await this.clauseRow(index).getByTestId("filter-value-input").fill(value); } + /** Pick a single-select enum clause value (e.g. repository) by its option label. */ + async setClauseEnumValue(index: number, optionLabel: string): Promise { + const trigger = this.clauseRow(index).getByTestId("filter-value-select"); + await trigger.click(); + // No .first(): exact:true + unique slugs mean a single match, so Playwright's + // strict mode surfaces a duplicate-option regression instead of masking it. + await this.page.getByRole("option", { name: optionLabel, exact: true }).click(); + } + async removeClause(index: number): Promise { await this.clauseRow(index).getByTestId("filter-clause-remove").click(); } diff --git a/apps/web/e2e/tests/task/sidebar-filter.spec.ts b/apps/web/e2e/tests/task/sidebar-filter.spec.ts index 5b40b68db6..da88ffa9b5 100644 --- a/apps/web/e2e/tests/task/sidebar-filter.spec.ts +++ b/apps/web/e2e/tests/task/sidebar-filter.spec.ts @@ -12,7 +12,11 @@ * - Draft semantics + discard * - Last-view deletion guard */ +import path from "node:path"; +import fs from "node:fs"; +import { execSync } from "node:child_process"; import { test, expect } from "../../fixtures/test-base"; +import { makeGitEnv } from "../../helpers/git-helper"; import { SessionPage } from "../../pages/session-page"; import { SidebarFilterPopoverPage } from "../../pages/sidebar-filter-popover"; @@ -304,6 +308,67 @@ test.describe("Sidebar filter — saved views CRUD", () => { }); }); +test.describe("Sidebar filter — repository dimension (#1213)", () => { + test("filtering by a repository keeps only that repo's tasks (not an empty board)", async ({ + testPage, + apiClient, + backend, + seedData, + }) => { + // The default seeded repo is a LOCAL repo (no GitHub provider), named + // "E2E Repo". Regression for #1213: a local repo's filter option value used + // to be its full local_path while each task carried the repo *name*, so a + // saved repository clause matched nothing and the board went empty. + + // A second, distinct local repo so we can prove the clause keeps matches and + // drops non-matches. + const repoBName = "Second E2E Repo"; + const repoBDir = path.join(backend.tmpDir, "repos", "e2e-repo-filter-b"); + fs.mkdirSync(repoBDir, { recursive: true }); + const gitEnv = makeGitEnv(backend.tmpDir); + execSync("git init -b main", { cwd: repoBDir, env: gitEnv }); + execSync('git commit --allow-empty -m "init"', { cwd: repoBDir, env: gitEnv }); + const repoB = await apiClient.createRepository(seedData.workspaceId, repoBDir, "main", { + name: repoBName, + }); + + await apiClient.createTask(seedData.workspaceId, "Task in repo A", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + await apiClient.createTask(seedData.workspaceId, "Task in repo B", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [repoB.id], + }); + + const navTask = await apiClient.createTask(seedData.workspaceId, "Repo Filter Nav", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await testPage.goto(`/t/${navTask.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.sidebar).toBeVisible({ timeout: 10_000 }); + const filters = new SidebarFilterPopoverPage(testPage); + await expect(filters.bar).toBeVisible(); + + // Both tasks visible before filtering. + await expect(session.sidebar.getByText("Task in repo A")).toBeVisible(); + await expect(session.sidebar.getByText("Task in repo B")).toBeVisible(); + + await filters.addFilterRow(); + await filters.setClauseDimension(0, "Repository"); + await filters.setClauseEnumValue(0, "E2E Repo"); + await filters.close(); + + // Repo A's task survives (this was empty before the fix); repo B's is hidden. + await expect(session.sidebar.getByText("Task in repo A")).toBeVisible(); + await expect(session.sidebar.getByText("Task in repo B")).toHaveCount(0); + }); +}); + test.describe("Sidebar filter — draft semantics", () => { test("dirty indicator appears after edits, clears on discard", async ({ testPage, diff --git a/apps/web/lib/repository-slug.test.ts b/apps/web/lib/repository-slug.test.ts new file mode 100644 index 0000000000..6ddae67fb7 --- /dev/null +++ b/apps/web/lib/repository-slug.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect } from "vitest"; +import type { Repository } from "@/lib/types/http"; +import { repositorySlug } from "./repository-slug"; + +function repo(overrides: Partial): Repository { + return { + id: "r1", + workspace_id: "w1", + name: "", + source_type: "local", + local_path: "", + provider: "", + provider_repo_id: "", + provider_owner: "", + provider_name: "", + default_branch: "main", + ...overrides, + } as Repository; +} + +describe("repositorySlug", () => { + it("uses owner/name for provider-backed repos", () => { + expect(repositorySlug(repo({ provider_owner: "kdlbs", provider_name: "kandev" }))).toBe( + "kdlbs/kandev", + ); + }); + + it("falls back to the repo name for local repos (matches board grouping identity)", () => { + expect( + repositorySlug(repo({ name: "kandev", local_path: "/home/carlos/Projects/kandev" })), + ).toBe("kandev"); + }); + + it("falls back to the last path segment when name is empty", () => { + expect(repositorySlug(repo({ name: "", local_path: "/home/carlos/Projects/kandev" }))).toBe( + "kandev", + ); + }); + + it("falls back to the full local_path when no path segment is available", () => { + // "/" collapses to an empty array after split/filter, so the final + // `|| repo.local_path` branch is the one actually exercised here. + expect(repositorySlug(repo({ name: "", local_path: "/" }))).toBe("/"); + }); +}); diff --git a/apps/web/lib/repository-slug.ts b/apps/web/lib/repository-slug.ts new file mode 100644 index 0000000000..44814d2ce1 --- /dev/null +++ b/apps/web/lib/repository-slug.ts @@ -0,0 +1,14 @@ +import type { Repository } from "@/lib/types/http"; + +/** + * Canonical identity for a repository — the task board's per-task + * `repositoryPath`, the sidebar filter's option value, and the grouping key. + * All sites MUST derive it identically, or a saved repo filter compares + * mismatched strings and silently matches nothing (#1213). + */ +export function repositorySlug(repo: Repository): string { + if (repo.provider_owner && repo.provider_name) { + return `${repo.provider_owner}/${repo.provider_name}`; + } + return repo.name || repo.local_path.split("/").filter(Boolean).pop() || repo.local_path; +} diff --git a/apps/web/lib/sidebar/apply-view.test.ts b/apps/web/lib/sidebar/apply-view.test.ts index 41e66bb674..6beb5ee3e0 100644 --- a/apps/web/lib/sidebar/apply-view.test.ts +++ b/apps/web/lib/sidebar/apply-view.test.ts @@ -3,6 +3,8 @@ import type { TaskSwitcherItem } from "@/components/task/task-switcher"; import { applyFilters, applyGroup, applySort, applyView, mergeGroupOrder } from "./apply-view"; import type { FilterClause, SidebarView } from "@/lib/state/slices/ui/sidebar-view-types"; import { DEFAULT_VIEW } from "@/lib/state/slices/ui/sidebar-view-builtins"; +import type { Repository } from "@/lib/types/http"; +import { repositorySlug } from "@/lib/repository-slug"; function task(overrides: Partial): TaskSwitcherItem { return { @@ -147,6 +149,38 @@ describe("applyFilters — per-dimension", () => { }); }); +describe("applyFilters — repository (#1213)", () => { + // The saved clause value is the filter option value, and the board sets each + // task's repositoryPath — BOTH via repositorySlug. For a local repo the slug + // is the repo name (not the full local_path). The bug was that the option + // value used local_path while the board used the name, so the clause matched + // nothing and the board went empty. + it("filters by a local repository (option value matches the board task field)", () => { + const localRepo = { + id: "r1", + workspace_id: "w1", + name: "kandev", + source_type: "local", + local_path: "/home/carlos/Projects/kandev", + provider: "", + provider_repo_id: "", + provider_owner: "", + provider_name: "", + default_branch: "main", + } as Repository; + const optionValue = repositorySlug(localRepo); + // The option value is the repo name, never the full filesystem path. + expect(optionValue).toBe("kandev"); + expect(optionValue).not.toBe(localRepo.local_path); + const tasks = [ + task({ id: "a", repositoryPath: repositorySlug(localRepo) }), + task({ id: "b", repositoryPath: "org/other" }), + ]; + const out = applyFilters(tasks, [C({ dimension: "repository", op: "is", value: optionValue })]); + expect(out.map((t) => t.id)).toEqual(["a"]); + }); +}); + describe("applyFilters — titleMatch + combos", () => { it("filters by titleMatch matches (case-insensitive substring)", () => { const tasks = [