fix: preserve client-side navigation for task links (#3844)
This commit is contained in:
+2
-2
@@ -111,8 +111,8 @@ surface.
|
||||
`next-themes` directly. The routing/image/dynamic adapters now provide
|
||||
browser-native behavior for the Vite SPA while legacy Next entrypoints are
|
||||
phased out.
|
||||
- Components: <200 lines, extract to domain components, composition over props.
|
||||
- Hooks: domain-organized in `hooks/domains/`, encapsulate subscription + selection.
|
||||
- Task links: `lib/links.ts::linkToTask` is the only `/t/:taskId` builder; pass raw IDs, use `TaskLink` or `AppLink`, and use `linkToTask` for router pushes. Keep compatibility `/tasks/:id`, Office/API paths, and route-recognition prefixes separate.
|
||||
- Components stay under 200 lines; extract domain components. Hooks belong in `hooks/domains/` and encapsulate subscription plus selection.
|
||||
- **Code-host dashboards:** GitHub, GitLab, and plugin code-host pages must use
|
||||
the provider-neutral primitives in `components/integrations/` for
|
||||
change-request lists, rows, toolbars, scope controls, task preset menus, and
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
"use client";
|
||||
|
||||
import dynamic from "@/lib/routing/client-dynamic";
|
||||
import Link from "@/components/routing/app-link";
|
||||
import TaskLink from "@/components/routing/task-link";
|
||||
import { IconInfoCircle } from "@tabler/icons-react";
|
||||
import { Label } from "@kandev/ui/label";
|
||||
import { Switch } from "@kandev/ui/switch";
|
||||
@@ -47,13 +47,13 @@ export function TaskAdvancedMode({ task, onToggleSimple }: TaskAdvancedModeProps
|
||||
</Label>
|
||||
<Switch id="advanced-toggle" checked onCheckedChange={() => onToggleSimple()} />
|
||||
</div>
|
||||
<Link
|
||||
href={`/t/${task.id}`}
|
||||
<TaskLink
|
||||
taskId={task.id}
|
||||
className="text-xs text-muted-foreground underline-offset-2 hover:underline cursor-pointer whitespace-nowrap"
|
||||
data-testid="task-cross-link"
|
||||
>
|
||||
{t("office:openInAdvancedView")}
|
||||
</Link>
|
||||
</TaskLink>
|
||||
</>
|
||||
),
|
||||
});
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
*/
|
||||
|
||||
import Link from "@/components/routing/app-link";
|
||||
import TaskLink from "@/components/routing/task-link";
|
||||
import { TaskPageContent } from "@/components/task/task-page-content";
|
||||
import { TaskBody, resolveTaskBodyMode } from "@/components/task/TaskBody";
|
||||
import { TaskHeader } from "@/components/task/TaskHeader";
|
||||
@@ -153,11 +154,21 @@ function SimpleTaskHeaderRow({ task }: { task: Task | null }) {
|
||||
|
||||
function CrossLinkRow({ taskId, target }: { taskId: string; target: "office" | "kanban" }) {
|
||||
const { t } = useTranslation();
|
||||
const href = target === "office" ? `/office/tasks/${taskId}` : `/t/${taskId}`;
|
||||
const label = target === "office" ? t("tasks:openInOfficeView") : t("tasks:openInAdvancedView");
|
||||
if (target === "kanban") {
|
||||
return (
|
||||
<TaskLink
|
||||
taskId={taskId}
|
||||
className="text-xs text-muted-foreground underline-offset-2 hover:underline cursor-pointer"
|
||||
data-testid="task-cross-link"
|
||||
>
|
||||
{label}
|
||||
</TaskLink>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<Link
|
||||
href={href}
|
||||
href={`/office/tasks/${taskId}`}
|
||||
className="text-xs text-muted-foreground underline-offset-2 hover:underline cursor-pointer"
|
||||
data-testid="task-cross-link"
|
||||
>
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { redirect } from "@/lib/routing/server-navigation";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
|
||||
/**
|
||||
* Compatibility alias for `/tasks/:id`.
|
||||
@@ -25,6 +26,5 @@ export default async function TasksDetailAliasPage({
|
||||
query.set(key, value);
|
||||
}
|
||||
}
|
||||
const queryString = query.toString();
|
||||
redirect(queryString ? `/t/${id}?${queryString}` : `/t/${id}`);
|
||||
redirect(linkToTask(id, { searchParams: query }));
|
||||
}
|
||||
|
||||
@@ -116,7 +116,7 @@ describe("RunsSection run log", () => {
|
||||
|
||||
fireEvent.click(screen.getByTestId("run-row-run-x"));
|
||||
|
||||
expect(mockPush).toHaveBeenCalledWith("/tasks/task-hidden");
|
||||
expect(mockPush).toHaveBeenCalledWith("/t/task-hidden");
|
||||
});
|
||||
|
||||
it("does not link a run that never produced a task", () => {
|
||||
|
||||
@@ -21,6 +21,7 @@ import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@
|
||||
import { IconChevronDown, IconChevronUp, IconRefresh, IconTrash } from "@tabler/icons-react";
|
||||
import { useAutomationRuns } from "@/hooks/domains/settings/use-automation-runs";
|
||||
import { buildRunOutcomeReasonSuffix } from "@/lib/automation-run-reason";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import type { AutomationRun, RunStatus } from "@/lib/types/automation";
|
||||
import { formatRelativeTime } from "@/lib/utils";
|
||||
|
||||
@@ -326,7 +327,7 @@ export function RunsSection({ automationId, workspaceId }: RunsSectionProps) {
|
||||
run={run}
|
||||
deleting={deleting}
|
||||
onDelete={deleteRun}
|
||||
onNavigate={(id) => router.push(`/tasks/${id}`)}
|
||||
onNavigate={(id) => router.push(linkToTask(id))}
|
||||
/>
|
||||
))
|
||||
)}
|
||||
|
||||
@@ -5,6 +5,7 @@ import { Input } from "@kandev/ui/input";
|
||||
import { Label } from "@kandev/ui/label";
|
||||
import type { AutomationTrigger } from "@/lib/types/automation";
|
||||
import { copyToClipboard } from "@/lib/utils/copy-to-clipboard";
|
||||
import TaskLink from "@/components/routing/task-link";
|
||||
|
||||
export function PluginWebhookControls({
|
||||
trigger,
|
||||
@@ -81,9 +82,9 @@ export function PluginWebhookControls({
|
||||
{new Date(receipt.created_at * 1000).toLocaleString()}:{" "}
|
||||
{t(`automations:pluginWebhookState_${receipt.state}`)}{" "}
|
||||
{receipt.task_id ? (
|
||||
<a className="underline" href={`/tasks/${receipt.task_id}`}>
|
||||
<TaskLink taskId={receipt.task_id} className="underline">
|
||||
{receipt.run_id}
|
||||
</a>
|
||||
</TaskLink>
|
||||
) : (
|
||||
receipt.run_id
|
||||
)}
|
||||
|
||||
@@ -147,7 +147,7 @@ it("updates the cache and store after associating the created task", async () =>
|
||||
|
||||
await waitFor(() => expect(mocks.cache).toHaveBeenCalledWith(workspaceId, "task-1", linked));
|
||||
expect(mocks.setTaskPullRequest).toHaveBeenCalledWith("task-1", linked);
|
||||
expect(mocks.push).toHaveBeenCalledWith("/tasks/task-1");
|
||||
expect(mocks.push).toHaveBeenCalledWith("/t/task-1");
|
||||
});
|
||||
|
||||
it("reports a pull request association failure", async () => {
|
||||
@@ -158,7 +158,7 @@ it("reports a pull request association failure", async () => {
|
||||
|
||||
await waitFor(() => expect(mocks.toastError).toHaveBeenCalledWith("Azure association failed"));
|
||||
expect(mocks.close).toHaveBeenCalled();
|
||||
expect(mocks.push).toHaveBeenCalledWith("/tasks/task-1");
|
||||
expect(mocks.push).toHaveBeenCalledWith("/t/task-1");
|
||||
});
|
||||
|
||||
it("associates and caches a created task for a work item", async () => {
|
||||
|
||||
@@ -7,6 +7,7 @@ import { useAppStore } from "@/components/state-provider";
|
||||
import { cacheAzureDevOpsTaskPullRequest } from "@/hooks/domains/azure-devops/use-azure-devops-task-pull-requests";
|
||||
import { cacheAzureDevOpsTaskWorkItem } from "@/hooks/domains/azure-devops/use-azure-devops-task-work-items";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import {
|
||||
associateAzureDevOpsPullRequest,
|
||||
associateAzureDevOpsWorkItem,
|
||||
@@ -186,7 +187,7 @@ export function AzureDevOpsTaskLauncher({
|
||||
) => {
|
||||
await linkCreatedTask(task);
|
||||
onClose();
|
||||
if (meta?.autoFocus !== false) router.push(`/tasks/${task.id}`);
|
||||
if (meta?.autoFocus !== false) router.push(linkToTask(task.id));
|
||||
};
|
||||
|
||||
if (!workspaceId || !payload || !launch?.workflow || !launch.workflowSteps[0]) return null;
|
||||
|
||||
@@ -292,7 +292,7 @@ describe("QuickTaskLauncher issue linking", () => {
|
||||
expect(mocks.upsertTaskIssue).toHaveBeenCalledWith(WORKSPACE_ID, link);
|
||||
});
|
||||
expect(mocks.push).toHaveBeenCalledTimes(autoFocus ? 1 : 0);
|
||||
if (autoFocus) expect(mocks.push).toHaveBeenCalledWith("/tasks/task-1");
|
||||
if (autoFocus) expect(mocks.push).toHaveBeenCalledWith("/t/task-1");
|
||||
},
|
||||
);
|
||||
|
||||
@@ -302,7 +302,7 @@ describe("QuickTaskLauncher issue linking", () => {
|
||||
|
||||
mocks.dialogProps?.onSuccess?.({ id: "task-1" } as Task);
|
||||
|
||||
expect(mocks.push).toHaveBeenCalledWith("/tasks/task-1");
|
||||
expect(mocks.push).toHaveBeenCalledWith("/t/task-1");
|
||||
await waitFor(() => expect(mocks.linkTaskIssue).toHaveBeenCalledTimes(1));
|
||||
expect(mocks.upsertTaskIssue).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useMemo } from "react";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import type { Icon } from "@tabler/icons-react";
|
||||
import { TaskCreateDialog } from "@/components/task-create-dialog";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
@@ -236,7 +237,7 @@ export function QuickTaskLauncher({
|
||||
});
|
||||
}
|
||||
onClose();
|
||||
if (meta?.autoFocus !== false) router.push(`/tasks/${task.id}`);
|
||||
if (meta?.autoFocus !== false) router.push(linkToTask(task.id));
|
||||
};
|
||||
|
||||
if (!workspaceId || !defaultWorkflow || !defaultStep || !dialog) return null;
|
||||
|
||||
@@ -17,6 +17,7 @@ import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint";
|
||||
import { toast } from "@/lib/toast/sonner";
|
||||
import { formatRelativeTime } from "@/lib/i18n/formats";
|
||||
import { copyToClipboard } from "@/lib/utils/copy-to-clipboard";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import {
|
||||
inboxHistoryClarificationQuestions,
|
||||
inboxHistoryPermissionContent,
|
||||
@@ -32,8 +33,7 @@ const TOUCH_ICON_BUTTON =
|
||||
"max-md:min-h-11 max-md:min-w-11 [@media(pointer:coarse)]:min-h-11 [@media(pointer:coarse)]:min-w-11";
|
||||
|
||||
function taskHrefForBundle(bundle: InboxHistoryBundle): string {
|
||||
if (!bundle.session_id) return `/t/${bundle.task_id}`;
|
||||
return `/t/${bundle.task_id}?sessionId=${encodeURIComponent(bundle.session_id)}`;
|
||||
return linkToTask(bundle.task_id, { sessionId: bundle.session_id ?? undefined });
|
||||
}
|
||||
|
||||
function InboxHistoryQuestionDetail({ bundle }: { bundle: InboxHistoryBundle }) {
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useMemo } from "react";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import { TaskCreateDialog } from "@/components/task-create-dialog";
|
||||
import type { Task, Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import type { JiraTicket } from "@/lib/types/jira";
|
||||
@@ -62,7 +63,7 @@ export function QuickTaskLauncher({
|
||||
};
|
||||
const handleSuccess = (task: Task, _mode?: "create" | "edit", meta?: { autoFocus?: boolean }) => {
|
||||
onClose();
|
||||
if (meta?.autoFocus !== false) router.push(`/tasks/${task.id}`);
|
||||
if (meta?.autoFocus !== false) router.push(linkToTask(task.id));
|
||||
};
|
||||
|
||||
if (!workspaceId || !defaultWorkflow || !defaultStep || !dialog) return null;
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useMemo } from "react";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import { TaskCreateDialog } from "@/components/task-create-dialog";
|
||||
import type { Task, Workflow, WorkflowStep } from "@/lib/types/http";
|
||||
import type { LinearIssue } from "@/lib/types/linear";
|
||||
@@ -84,7 +85,7 @@ export function LinearQuickTaskLauncher({
|
||||
};
|
||||
const handleSuccess = (task: Task, _mode?: "create" | "edit", meta?: { autoFocus?: boolean }) => {
|
||||
onClose();
|
||||
if (meta?.autoFocus !== false) router.push(`/tasks/${task.id}`);
|
||||
if (meta?.autoFocus !== false) router.push(linkToTask(task.id));
|
||||
};
|
||||
|
||||
if (!workspaceId || !defaultWorkflow || !defaultStep || !dialog) return null;
|
||||
|
||||
@@ -7,6 +7,7 @@ import { formatRelativeTime } from "@/lib/i18n/formats";
|
||||
import { getTaskStateIcon } from "@/lib/ui/state-icons";
|
||||
import { FAILED_TASK_STATUS } from "@/lib/threads/thread-session-status";
|
||||
import { parseStrictRfc3339Timestamp } from "@/lib/utils/strict-timestamp";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import {
|
||||
failedInboxOriginMarkerKey,
|
||||
hasResolvableFailedInboxReason,
|
||||
@@ -16,7 +17,7 @@ import type { FailedInboxRow as FailedInboxRowData } from "@/lib/types/failed-in
|
||||
// The row addresses a task, never a session (design-01#Session-resolution):
|
||||
// the open-task control needs only the task id.
|
||||
function taskHrefForRow(row: FailedInboxRowData): string {
|
||||
return `/t/${row.task_id}`;
|
||||
return linkToTask(row.task_id);
|
||||
}
|
||||
|
||||
export function FailedInboxRow({ row }: { row: FailedInboxRowData }) {
|
||||
|
||||
@@ -41,6 +41,7 @@ import {
|
||||
rowSecondaryText,
|
||||
} from "@/lib/needs-you-inbox/row-presentation";
|
||||
import { resolveThreadSessionStatus } from "@/lib/threads/thread-session-status";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import type { TaskSessionState } from "@/lib/types/http";
|
||||
import type {
|
||||
LateClarificationSnapshot,
|
||||
@@ -167,8 +168,7 @@ function RowActionsMenu({
|
||||
}
|
||||
|
||||
function taskHrefForBundle(bundle: ClarificationInboxBundle): string {
|
||||
if (!bundle.session_id) return `/t/${bundle.task_id}`;
|
||||
return `/t/${bundle.task_id}?sessionId=${encodeURIComponent(bundle.session_id)}`;
|
||||
return linkToTask(bundle.task_id, { sessionId: bundle.session_id ?? undefined });
|
||||
}
|
||||
|
||||
// eslint-disable-next-line max-lines-per-function -- the row keeps its responsive actions and inline panel together.
|
||||
|
||||
@@ -4,14 +4,18 @@ import {
|
||||
clearNavigationBlockerForTests,
|
||||
setNavigationBlocker,
|
||||
} from "@/lib/routing/navigation-guard";
|
||||
import { LOCATION_CHANGE_EVENT } from "@/lib/routing/navigation-event";
|
||||
import Link from "./app-link";
|
||||
|
||||
const BLOCKED_PATH = "/settings/general/appearance";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
clearNavigationBlockerForTests();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
// eslint-disable-next-line max-lines-per-function -- the suite keeps one AppLink contract together.
|
||||
describe("AppLink", () => {
|
||||
it("navigates same-origin links through browser history", () => {
|
||||
window.history.replaceState({}, "", "/");
|
||||
@@ -37,13 +41,48 @@ describe("AppLink", () => {
|
||||
expect(onClick).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("calls onNavigated after the location event for a committed navigation", () => {
|
||||
const events: string[] = [];
|
||||
const onNavigated = vi.fn(() => events.push("callback"));
|
||||
window.addEventListener(LOCATION_CHANGE_EVENT, () => events.push("location"), { once: true });
|
||||
render(
|
||||
<Link href="/tasks" onNavigated={onNavigated}>
|
||||
Tasks
|
||||
</Link>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Tasks"));
|
||||
|
||||
expect(events).toEqual(["location", "callback"]);
|
||||
expect(onNavigated).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("does not intercept modified clicks", () => {
|
||||
const pushState = vi.spyOn(window.history, "pushState");
|
||||
render(<Link href="/tasks">Tasks</Link>);
|
||||
const onNavigated = vi.fn();
|
||||
render(
|
||||
<Link href="/tasks" onNavigated={onNavigated}>
|
||||
Tasks
|
||||
</Link>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Tasks"), { metaKey: true });
|
||||
|
||||
expect(pushState).not.toHaveBeenCalled();
|
||||
expect(onNavigated).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not call onNavigated when a caller prevents the click", () => {
|
||||
const onNavigated = vi.fn();
|
||||
render(
|
||||
<Link href="/tasks" onNavigated={onNavigated} onClick={(event) => event.preventDefault()}>
|
||||
Tasks
|
||||
</Link>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Tasks"));
|
||||
|
||||
expect(onNavigated).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not intercept hash-only links", () => {
|
||||
@@ -70,7 +109,7 @@ describe("AppLink", () => {
|
||||
});
|
||||
|
||||
it("waits for a navigation blocker before changing routes", () => {
|
||||
window.history.replaceState({}, "", "/settings/general/appearance");
|
||||
window.history.replaceState({}, "", BLOCKED_PATH);
|
||||
let proceed: () => void = () => undefined;
|
||||
const onLocationChange = vi.fn();
|
||||
window.addEventListener("kandev:navigation", onLocationChange, { once: true });
|
||||
@@ -81,9 +120,49 @@ describe("AppLink", () => {
|
||||
|
||||
fireEvent.click(screen.getByText("Tasks"));
|
||||
|
||||
expect(window.location.pathname).toBe("/settings/general/appearance");
|
||||
expect(window.location.pathname).toBe(BLOCKED_PATH);
|
||||
proceed();
|
||||
expect(window.location.pathname).toBe("/tasks");
|
||||
expect(onLocationChange).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("runs onNavigated only when a navigation blocker proceeds", () => {
|
||||
window.history.replaceState({}, "", BLOCKED_PATH);
|
||||
let proceed: () => void = () => undefined;
|
||||
const onNavigated = vi.fn();
|
||||
setNavigationBlocker((intent) => {
|
||||
proceed = intent.proceed;
|
||||
});
|
||||
render(
|
||||
<Link href="/tasks" onNavigated={onNavigated}>
|
||||
Tasks
|
||||
</Link>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Tasks"));
|
||||
|
||||
expect(onNavigated).not.toHaveBeenCalled();
|
||||
proceed();
|
||||
expect(onNavigated).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("does not call onNavigated when a navigation blocker cancels", () => {
|
||||
window.history.replaceState({}, "", BLOCKED_PATH);
|
||||
let cancel: () => void = () => undefined;
|
||||
const onNavigated = vi.fn();
|
||||
setNavigationBlocker((intent) => {
|
||||
cancel = intent.cancel;
|
||||
});
|
||||
render(
|
||||
<Link href="/tasks" onNavigated={onNavigated}>
|
||||
Tasks
|
||||
</Link>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Tasks"));
|
||||
cancel();
|
||||
|
||||
expect(window.location.pathname).toBe(BLOCKED_PATH);
|
||||
expect(onNavigated).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,10 +11,11 @@ type AppLinkHref = string | URL;
|
||||
export type AppLinkProps = Omit<AnchorHTMLAttributes<HTMLAnchorElement>, "href"> & {
|
||||
href: AppLinkHref;
|
||||
children?: ReactNode;
|
||||
onNavigated?: () => void;
|
||||
};
|
||||
|
||||
const Link = forwardRef<HTMLAnchorElement, AppLinkProps>(function Link(
|
||||
{ href, onClick, ...props },
|
||||
{ href, onClick, onNavigated, ...props },
|
||||
ref,
|
||||
) {
|
||||
const resolvedHref = href.toString();
|
||||
@@ -27,6 +28,7 @@ const Link = forwardRef<HTMLAnchorElement, AppLinkProps>(function Link(
|
||||
pushNavigationState({}, "", resolvedHref, () => {
|
||||
window.scrollTo({ top: 0, left: 0 });
|
||||
window.dispatchEvent(new Event(LOCATION_CHANGE_EVENT));
|
||||
onNavigated?.();
|
||||
});
|
||||
};
|
||||
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import TaskLink from "./task-link";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("TaskLink", () => {
|
||||
it("builds a canonical task href from raw task and query context", () => {
|
||||
const searchParams = new URLSearchParams([["view", "focused"]]);
|
||||
render(
|
||||
<TaskLink taskId="task/123" layout="plan" sessionId="session 123" searchParams={searchParams}>
|
||||
Open task
|
||||
</TaskLink>,
|
||||
);
|
||||
|
||||
expect(screen.getByRole("link", { name: "Open task" }).getAttribute("href")).toBe(
|
||||
"/t/task%2F123?view=focused&layout=plan&sessionId=session+123",
|
||||
);
|
||||
});
|
||||
|
||||
it("forwards navigation callbacks and anchor refs through AppLink", () => {
|
||||
const onNavigated = vi.fn();
|
||||
const ref = { current: null as HTMLAnchorElement | null };
|
||||
render(
|
||||
<TaskLink ref={ref} taskId="task-123" onNavigated={onNavigated}>
|
||||
Open task
|
||||
</TaskLink>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("link", { name: "Open task" }));
|
||||
|
||||
expect(ref.current).toBe(screen.getByRole("link", { name: "Open task" }));
|
||||
expect(onNavigated).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,21 @@
|
||||
"use client";
|
||||
|
||||
import { forwardRef } from "react";
|
||||
import Link, { type AppLinkProps } from "./app-link";
|
||||
import { linkToTask, type TaskLinkOptions } from "@/lib/links";
|
||||
|
||||
export type TaskLinkProps = Omit<AppLinkProps, "href"> &
|
||||
TaskLinkOptions & {
|
||||
taskId: string;
|
||||
};
|
||||
|
||||
const TaskLink = forwardRef<HTMLAnchorElement, TaskLinkProps>(function TaskLink(
|
||||
{ taskId, layout, sessionId, searchParams, ...props },
|
||||
ref,
|
||||
) {
|
||||
return (
|
||||
<Link {...props} ref={ref} href={linkToTask(taskId, { layout, sessionId, searchParams })} />
|
||||
);
|
||||
});
|
||||
|
||||
export default TaskLink;
|
||||
@@ -171,7 +171,7 @@ describe("RunsListPage", () => {
|
||||
|
||||
fireEvent.click(screen.getByTestId("run-entry-run-1"));
|
||||
|
||||
expect(mocks.push).toHaveBeenCalledWith("/tasks/task-1");
|
||||
expect(mocks.push).toHaveBeenCalledWith("/t/task-1");
|
||||
});
|
||||
|
||||
it("states the scheduler constraint rather than leaving it to be discovered", async () => {
|
||||
|
||||
@@ -8,6 +8,7 @@ import Link from "@/components/routing/app-link";
|
||||
import { PageTopbar } from "@/components/page-topbar";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import { cn } from "@/lib/utils";
|
||||
import type { WorkspaceAutomationRun } from "@/lib/types/automation";
|
||||
import { buildAgenda, STATE_DOT_CLASS } from "./automation-rows";
|
||||
@@ -231,7 +232,7 @@ export function RunsListPage({ workspaceId }: { workspaceId?: string }) {
|
||||
<RecentRuns
|
||||
runs={runsState.runs}
|
||||
loading={loading}
|
||||
onOpen={(taskId) => router.push(`/tasks/${taskId}`)}
|
||||
onOpen={(taskId) => router.push(linkToTask(taskId))}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
|
||||
@@ -114,7 +114,7 @@ describe("RunsPageClient feed", () => {
|
||||
|
||||
fireEvent.click(screen.getByTestId("run-entry-run-x"));
|
||||
|
||||
expect(mocks.push).toHaveBeenCalledWith("/tasks/task-hidden");
|
||||
expect(mocks.push).toHaveBeenCalledWith("/t/task-hidden");
|
||||
});
|
||||
|
||||
it("does not navigate for a run that never produced a task", async () => {
|
||||
|
||||
@@ -8,6 +8,7 @@ import { Button } from "@kandev/ui/button";
|
||||
import Link from "@/components/routing/app-link";
|
||||
import { PageTopbar } from "@/components/page-topbar";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import type { WorkspaceAutomationRun } from "@/lib/types/automation";
|
||||
import { RunFeedItem } from "./run-feed-item";
|
||||
import { ANY_AUTOMATION, RunFilters, isDefaultFilters } from "./run-filters";
|
||||
@@ -191,7 +192,7 @@ export function RunsPageClient({ workspaceId }: RunsPageClientProps) {
|
||||
loading={loading}
|
||||
error={error}
|
||||
onClearFilters={clearFilters}
|
||||
onOpen={(taskId) => router.push(`/tasks/${taskId}`)}
|
||||
onOpen={(taskId) => router.push(linkToTask(taskId))}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -4,6 +4,7 @@ import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import {
|
||||
canvasHref,
|
||||
getCanvas,
|
||||
@@ -372,10 +373,7 @@ async function editCanvasFromHost(options: CanvasHostEditOptions): Promise<void>
|
||||
try {
|
||||
const response = await startCanvasEdit(canvas.id);
|
||||
if (response.task_id) {
|
||||
const query = response.session_id
|
||||
? `?sessionId=${encodeURIComponent(response.session_id)}`
|
||||
: "";
|
||||
router.push(`/t/${encodeURIComponent(response.task_id)}${query}`);
|
||||
router.push(linkToTask(response.task_id, { sessionId: response.session_id ?? undefined }));
|
||||
}
|
||||
} catch (reason: unknown) {
|
||||
onError(reason);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { TFunction } from "i18next";
|
||||
import { formatDateTime } from "@/lib/i18n/formats";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import type { KubernetesSession } from "@/lib/types/http-kubernetes";
|
||||
|
||||
const POD_STATUS_LABEL_KEYS: Record<string, string> = {
|
||||
@@ -86,7 +87,7 @@ export function translateTaskLink(session: KubernetesSession, t: TFunction): str
|
||||
}
|
||||
|
||||
export function taskHref(taskId: string): string {
|
||||
return `/t/${encodeURIComponent(taskId)}`;
|
||||
return linkToTask(taskId);
|
||||
}
|
||||
|
||||
export function shortId(value: string): string {
|
||||
|
||||
@@ -22,7 +22,7 @@ import {
|
||||
import { Spinner } from "@kandev/ui/spinner";
|
||||
import { IconExternalLink } from "@tabler/icons-react";
|
||||
import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import TaskLink from "@/components/routing/task-link";
|
||||
import type {
|
||||
DiagnosticBundleCapabilities,
|
||||
DiagnosticBundleSource,
|
||||
@@ -477,8 +477,8 @@ function SessionRow({
|
||||
aria-label={session.session_id}
|
||||
/>
|
||||
<div className="min-w-0 flex-1">
|
||||
<a
|
||||
href={linkToTask(session.task_id)}
|
||||
<TaskLink
|
||||
taskId={session.task_id}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="flex w-fit max-w-full items-center gap-1 truncate text-sm font-medium text-foreground underline-offset-4 hover:underline focus-visible:rounded-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
|
||||
@@ -487,7 +487,7 @@ function SessionRow({
|
||||
>
|
||||
<span className="truncate">{taskTitle}</span>
|
||||
<IconExternalLink className="size-3 shrink-0" />
|
||||
</a>
|
||||
</TaskLink>
|
||||
<label htmlFor={checkboxID} className="mt-1 block min-h-11 cursor-pointer">
|
||||
<span className="block truncate text-sm font-medium">{title}</span>
|
||||
<span className="block truncate text-xs text-muted-foreground">{details}</span>
|
||||
|
||||
@@ -25,6 +25,7 @@ import { Button } from "@kandev/ui/button";
|
||||
import Link from "@/components/routing/app-link";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import {
|
||||
archiveCanvas,
|
||||
canvasHref,
|
||||
@@ -101,10 +102,7 @@ function useWorkspaceCanvasState(workspaceId: string) {
|
||||
try {
|
||||
const response = await startCanvasEdit(canvas.id);
|
||||
if (response.task_id) {
|
||||
const query = response.session_id
|
||||
? `?sessionId=${encodeURIComponent(response.session_id)}`
|
||||
: "";
|
||||
router.push(`/t/${encodeURIComponent(response.task_id)}${query}`);
|
||||
router.push(linkToTask(response.task_id, { sessionId: response.session_id ?? undefined }));
|
||||
}
|
||||
} catch (reason: unknown) {
|
||||
setError(canvasErrorMessage(reason, t, "canvases:actionFailed"));
|
||||
|
||||
@@ -152,6 +152,7 @@ function MobileChatPanelContent({
|
||||
<TaskChatPanel
|
||||
sessionId={effectiveSessionId}
|
||||
taskId={effectiveSessionId ? activeTaskId : null}
|
||||
statusTaskId={activeTaskId}
|
||||
onOpenFile={onOpenFile}
|
||||
pendingScrollTarget={scrollTarget}
|
||||
isVisible={isVisible}
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { useEffect, useState, useCallback, useRef, useMemo } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import Link from "@/components/routing/app-link";
|
||||
import TaskLink from "@/components/routing/task-link";
|
||||
import {
|
||||
IconCopy,
|
||||
IconPlayerPause,
|
||||
@@ -141,13 +142,13 @@ function useSimplePaneTopbar(task: Task, onToggleAdvanced?: () => void) {
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
<Link
|
||||
href={`/t/${task.id}`}
|
||||
<TaskLink
|
||||
taskId={task.id}
|
||||
className="text-xs text-muted-foreground underline-offset-2 hover:underline cursor-pointer whitespace-nowrap"
|
||||
data-testid="task-cross-link"
|
||||
>
|
||||
{t("task:openInAdvancedView")}
|
||||
</Link>
|
||||
</TaskLink>
|
||||
</>
|
||||
),
|
||||
});
|
||||
|
||||
@@ -1,22 +1,34 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const getTaskDependenciesMock = vi.hoisted(() => vi.fn());
|
||||
const touchDrawerMock = vi.hoisted(() => ({ value: false }));
|
||||
vi.mock("@/lib/api/domains/task-dependencies-api", () => ({
|
||||
getTaskDependencies: getTaskDependenciesMock,
|
||||
}));
|
||||
import { cleanup, render, screen, waitFor } from "@testing-library/react";
|
||||
vi.mock("@/hooks/use-compact-task-chrome", () => ({
|
||||
useTouchDrawer: () => touchDrawerMock.value,
|
||||
}));
|
||||
import { cleanup, createEvent, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import {
|
||||
clearNavigationBlockerForTests,
|
||||
setNavigationBlocker,
|
||||
} from "@/lib/routing/navigation-guard";
|
||||
import { TaskDependencyChip } from "./task-dependency-chip";
|
||||
import type { KanbanState } from "@/lib/state/slices/kanban/types";
|
||||
|
||||
beforeEach(() => {
|
||||
getTaskDependenciesMock.mockReset();
|
||||
touchDrawerMock.value = false;
|
||||
// Default: the server also reports no edges, so the fetch fallback cannot be
|
||||
// what makes a "renders nothing" assertion pass.
|
||||
getTaskDependenciesMock.mockResolvedValue({ id: "unknown", depends_on: [], blocks: [] });
|
||||
});
|
||||
|
||||
afterEach(cleanup);
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
clearNavigationBlockerForTests();
|
||||
});
|
||||
|
||||
type KanbanTask = KanbanState["tasks"][number];
|
||||
|
||||
@@ -38,6 +50,7 @@ function renderChip(tasks: KanbanTask[], taskId: string) {
|
||||
);
|
||||
}
|
||||
|
||||
// eslint-disable-next-line max-lines-per-function -- the suite covers both disclosure surfaces.
|
||||
describe("TaskDependencyChip", () => {
|
||||
it("renders nothing when the task has no edges in either direction", () => {
|
||||
renderChip([task({ id: "a" })], "a");
|
||||
@@ -73,6 +86,52 @@ describe("TaskDependencyChip", () => {
|
||||
expect(chip.textContent).toMatch(/1/);
|
||||
});
|
||||
|
||||
it("uses canonical guarded links for both dependency directions", () => {
|
||||
renderChip(
|
||||
[
|
||||
task({
|
||||
id: "b",
|
||||
dependsOn: [{ id: "a", title: "A", status: "pending" }],
|
||||
blocks: [{ id: "c", title: "C" }],
|
||||
}),
|
||||
],
|
||||
"b",
|
||||
);
|
||||
const chip = screen.getByTestId(CHIP_TESTID);
|
||||
fireEvent.click(chip);
|
||||
const entries = screen.getAllByTestId("task-dependency-entry");
|
||||
|
||||
expect(entries.map((entry) => entry.getAttribute("href"))).toEqual(["/t/a", "/t/c"]);
|
||||
|
||||
const click = createEvent.click(entries[0]);
|
||||
fireEvent(entries[0], click);
|
||||
expect(click.defaultPrevented).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the disclosure open until guarded navigation commits", async () => {
|
||||
let proceed: () => void = () => undefined;
|
||||
setNavigationBlocker((intent) => {
|
||||
proceed = intent.proceed;
|
||||
});
|
||||
renderChip([task({ id: "b", dependsOn: [{ id: "a", title: "A", status: "pending" }] })], "b");
|
||||
fireEvent.click(screen.getByTestId(CHIP_TESTID));
|
||||
fireEvent.click(screen.getByTestId("task-dependency-entry"));
|
||||
|
||||
expect(screen.getByTestId("task-dependency-chip-popover")).toBeTruthy();
|
||||
proceed();
|
||||
await waitFor(() => expect(screen.queryByTestId("task-dependency-chip-popover")).toBeNull());
|
||||
});
|
||||
|
||||
it("keeps dependency rows touch-sized in the drawer", () => {
|
||||
touchDrawerMock.value = true;
|
||||
renderChip([task({ id: "b", dependsOn: [{ id: "a", title: "A", status: "pending" }] })], "b");
|
||||
fireEvent.click(screen.getByTestId(CHIP_TESTID));
|
||||
|
||||
expect(screen.getByTestId("task-dependency-entry").className).toContain(
|
||||
"[@media(pointer:coarse)]:min-h-11",
|
||||
);
|
||||
});
|
||||
|
||||
it("renders for a resolved dependency so the chain is still visible after it unblocks", () => {
|
||||
renderChip(
|
||||
[
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
|
||||
import { cn } from "@kandev/ui/lib/utils";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import TaskLink from "@/components/routing/task-link";
|
||||
import { useTouchDrawer } from "@/hooks/use-compact-task-chrome";
|
||||
import { getTaskDependencies } from "@/lib/api/domains/task-dependencies-api";
|
||||
import type { TaskDependencyRef } from "@/lib/state/slices/kanban/types";
|
||||
@@ -153,15 +154,23 @@ function statusIcon(status?: string) {
|
||||
function DependencyRow({
|
||||
ref: entry,
|
||||
showStatus,
|
||||
touchTarget,
|
||||
onNavigated,
|
||||
}: {
|
||||
ref: TaskDependencyRef;
|
||||
showStatus: boolean;
|
||||
touchTarget: boolean;
|
||||
onNavigated: () => void;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<a
|
||||
href={`/tasks/${entry.id}`}
|
||||
className="flex items-center gap-2 rounded px-2 py-1.5 text-xs hover:bg-accent min-w-0"
|
||||
<TaskLink
|
||||
taskId={entry.id}
|
||||
onNavigated={onNavigated}
|
||||
className={cn(
|
||||
"flex min-w-0 cursor-pointer items-center gap-2 rounded px-2 py-1.5 text-xs hover:bg-accent",
|
||||
touchTarget && "[@media(pointer:coarse)]:min-h-11",
|
||||
)}
|
||||
data-testid="task-dependency-entry"
|
||||
>
|
||||
{showStatus ? (
|
||||
@@ -175,11 +184,19 @@ function DependencyRow({
|
||||
{t(`task:state.${entry.state}`, { defaultValue: entry.state })}
|
||||
</span>
|
||||
)}
|
||||
</a>
|
||||
</TaskLink>
|
||||
);
|
||||
}
|
||||
|
||||
function DependencyLists({ data }: { data: DependencyChipData }) {
|
||||
function DependencyLists({
|
||||
data,
|
||||
touchTarget,
|
||||
onNavigated,
|
||||
}: {
|
||||
data: DependencyChipData;
|
||||
touchTarget: boolean;
|
||||
onNavigated: () => void;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<div className="flex flex-col gap-3">
|
||||
@@ -189,7 +206,13 @@ function DependencyLists({ data }: { data: DependencyChipData }) {
|
||||
{t("task:dependencyBlockedBy")}
|
||||
</h4>
|
||||
{data.dependsOn.map((entry) => (
|
||||
<DependencyRow key={entry.id} ref={entry} showStatus />
|
||||
<DependencyRow
|
||||
key={entry.id}
|
||||
ref={entry}
|
||||
showStatus
|
||||
touchTarget={touchTarget}
|
||||
onNavigated={onNavigated}
|
||||
/>
|
||||
))}
|
||||
</section>
|
||||
)}
|
||||
@@ -199,7 +222,13 @@ function DependencyLists({ data }: { data: DependencyChipData }) {
|
||||
{t("task:dependencyBlocks")}
|
||||
</h4>
|
||||
{data.blocks.map((entry) => (
|
||||
<DependencyRow key={entry.id} ref={entry} showStatus={false} />
|
||||
<DependencyRow
|
||||
key={entry.id}
|
||||
ref={entry}
|
||||
showStatus={false}
|
||||
touchTarget={touchTarget}
|
||||
onNavigated={onNavigated}
|
||||
/>
|
||||
))}
|
||||
</section>
|
||||
)}
|
||||
@@ -293,7 +322,7 @@ export function TaskDependencyChip({ taskId }: { taskId: string | null }) {
|
||||
</DrawerClose>
|
||||
</DrawerHeader>
|
||||
<div className="overflow-y-auto p-2">
|
||||
<DependencyLists data={data} />
|
||||
<DependencyLists data={data} touchTarget onNavigated={() => setOpen(false)} />
|
||||
</div>
|
||||
</DrawerContent>
|
||||
</Drawer>
|
||||
@@ -313,7 +342,7 @@ export function TaskDependencyChip({ taskId }: { taskId: string | null }) {
|
||||
</button>
|
||||
</PopoverTrigger>
|
||||
<PopoverContent align="start" className="w-72 p-2" data-testid="task-dependency-chip-popover">
|
||||
<DependencyLists data={data} />
|
||||
<DependencyLists data={data} touchTarget={false} onNavigated={() => setOpen(false)} />
|
||||
</PopoverContent>
|
||||
</Popover>
|
||||
);
|
||||
|
||||
@@ -302,7 +302,7 @@ test("connects and browses Azure work items, PRs, and feedback", async ({
|
||||
);
|
||||
await taskDialog.getByTestId("submit-start-agent-chevron").click();
|
||||
await testPage.getByTestId("submit-create-without-agent").click();
|
||||
await expect(testPage).toHaveURL(/\/tasks\//);
|
||||
await expect(testPage).toHaveURL(/\/(?:t|tasks)\//);
|
||||
await testPage.goto("/azure-devops");
|
||||
await testPage.getByTestId("azure-devops-work-items-mode").click();
|
||||
await testPage.getByTestId("azure-devops-search-button").click();
|
||||
|
||||
@@ -271,7 +271,7 @@ test("mobile board opens a focused column editor without horizontal overflow", a
|
||||
"Implement: Handle token rotation",
|
||||
);
|
||||
await testPage.getByRole("button", { name: "Create only" }).click();
|
||||
await expect(testPage).toHaveURL(/\/tasks\//);
|
||||
await expect(testPage).toHaveURL(/\/(?:t|tasks)\//);
|
||||
await testPage.goto("/azure-devops");
|
||||
await testPage.getByTestId("azure-devops-work-items-mode").click();
|
||||
await testPage.getByTestId("azure-devops-mobile-filter-button").click();
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
// Filename starts with "mobile-" so this runs on the mobile-chrome project.
|
||||
import { expect, test } from "../../fixtures/test-base";
|
||||
import {
|
||||
assertLocatorWithinViewportX,
|
||||
assertNoDocumentHorizontalOverflow,
|
||||
} from "../../helpers/layout-assertions";
|
||||
|
||||
test("mobile dependency links preserve the task workbench and touch layout", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const predecessor = await apiClient.createTask(
|
||||
seedData.workspaceId,
|
||||
"Mobile navigation predecessor",
|
||||
{
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
const target = await apiClient.createTask(seedData.workspaceId, "Mobile navigation target", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
blocked_by: [predecessor.id],
|
||||
});
|
||||
const dependent = await apiClient.createTask(
|
||||
seedData.workspaceId,
|
||||
"Mobile navigation dependent",
|
||||
{
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
blocked_by: [target.id],
|
||||
},
|
||||
);
|
||||
|
||||
await testPage.goto(`/t/${target.id}`);
|
||||
const taskTitle = testPage.getByTestId("mobile-task-picker-trigger");
|
||||
await expect(taskTitle).toContainText(target.title);
|
||||
|
||||
const documentRequests: string[] = [];
|
||||
const onRequest = (request: import("@playwright/test").Request) => {
|
||||
if (
|
||||
request.isNavigationRequest() &&
|
||||
request.frame() === testPage.mainFrame() &&
|
||||
request.resourceType() === "document"
|
||||
) {
|
||||
documentRequests.push(request.url());
|
||||
}
|
||||
};
|
||||
testPage.on("request", onRequest);
|
||||
|
||||
try {
|
||||
await testPage.evaluate(() => {
|
||||
(window as Window & { __taskLinkDocumentSentinel?: string }).__taskLinkDocumentSentinel =
|
||||
"mobile-dependency-links-stay-in-spa";
|
||||
});
|
||||
|
||||
const chip = testPage.getByTestId("task-dependency-chip");
|
||||
await chip.tap();
|
||||
const drawer = testPage.getByTestId("task-dependency-chip-drawer");
|
||||
await expect(drawer).toBeVisible();
|
||||
await expect(drawer.locator("[class*='overflow-y-auto']")).toHaveCount(1);
|
||||
await assertLocatorWithinViewportX(drawer, "dependency drawer");
|
||||
await assertNoDocumentHorizontalOverflow(testPage, "dependency drawer");
|
||||
|
||||
const entries = drawer.getByTestId("task-dependency-entry");
|
||||
await expect(entries).toHaveCount(2);
|
||||
for (const entry of await entries.all()) {
|
||||
const box = await entry.boundingBox();
|
||||
expect(box, "dependency entry must have geometry").not.toBeNull();
|
||||
expect(box?.height ?? 0, "dependency entry touch target").toBeGreaterThanOrEqual(44);
|
||||
await assertLocatorWithinViewportX(entry, "dependency entry");
|
||||
}
|
||||
|
||||
const blockedBy = entries.filter({ hasText: predecessor.title });
|
||||
await expect(blockedBy).toHaveAttribute("href", `/t/${predecessor.id}`);
|
||||
await blockedBy.tap();
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${predecessor.id}$`));
|
||||
await expect(testPage.getByTestId("mobile-task-picker-trigger")).toContainText(
|
||||
predecessor.title,
|
||||
);
|
||||
await expect(testPage.getByTestId("task-dependency-chip-drawer")).toHaveCount(0);
|
||||
expect(documentRequests).toHaveLength(0);
|
||||
expect(
|
||||
await testPage.evaluate(
|
||||
() =>
|
||||
(window as Window & { __taskLinkDocumentSentinel?: string }).__taskLinkDocumentSentinel,
|
||||
),
|
||||
).toBe("mobile-dependency-links-stay-in-spa");
|
||||
|
||||
await testPage.goBack();
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${target.id}$`));
|
||||
await expect(testPage.getByTestId("mobile-task-picker-trigger")).toContainText(target.title);
|
||||
expect(documentRequests).toHaveLength(0);
|
||||
|
||||
await chip.tap();
|
||||
const reopenedDrawer = testPage.getByTestId("task-dependency-chip-drawer");
|
||||
await expect(reopenedDrawer).toBeVisible();
|
||||
const blocks = reopenedDrawer
|
||||
.getByTestId("task-dependency-entry")
|
||||
.filter({ hasText: dependent.title });
|
||||
await expect(blocks).toHaveAttribute("href", `/t/${dependent.id}`);
|
||||
await blocks.tap();
|
||||
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${dependent.id}$`));
|
||||
await expect(testPage.getByTestId("mobile-task-picker-trigger")).toContainText(dependent.title);
|
||||
await expect(testPage.getByTestId("task-dependency-chip-drawer")).toHaveCount(0);
|
||||
expect(documentRequests).toHaveLength(0);
|
||||
expect(
|
||||
await testPage.evaluate(
|
||||
() =>
|
||||
(window as Window & { __taskLinkDocumentSentinel?: string }).__taskLinkDocumentSentinel,
|
||||
),
|
||||
).toBe("mobile-dependency-links-stay-in-spa");
|
||||
await assertNoDocumentHorizontalOverflow(testPage, "mobile dependency navigation");
|
||||
} finally {
|
||||
testPage.off("request", onRequest);
|
||||
}
|
||||
});
|
||||
@@ -264,4 +264,95 @@ test.describe("Task dependencies", () => {
|
||||
await testPage.goto(`/t/${unrelated.id}`);
|
||||
await expect(testPage.getByTestId("task-dependency-chip")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("dependency links preserve the desktop task workbench", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const predecessor = await apiClient.createTask(seedData.workspaceId, "Navigation predecessor", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
});
|
||||
const target = await apiClient.createTask(seedData.workspaceId, "Navigation target", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
blocked_by: [predecessor.id],
|
||||
});
|
||||
const dependent = await apiClient.createTask(seedData.workspaceId, "Navigation dependent", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
blocked_by: [target.id],
|
||||
});
|
||||
|
||||
await testPage.goto(`/t/${target.id}`);
|
||||
const title = testPage.locator('[data-testid="task-topbar-title"]:visible');
|
||||
await expect(title).toHaveText(target.title);
|
||||
|
||||
const documentRequests: string[] = [];
|
||||
const onRequest = (request: import("@playwright/test").Request) => {
|
||||
if (
|
||||
request.isNavigationRequest() &&
|
||||
request.frame() === testPage.mainFrame() &&
|
||||
request.resourceType() === "document"
|
||||
) {
|
||||
documentRequests.push(request.url());
|
||||
}
|
||||
};
|
||||
testPage.on("request", onRequest);
|
||||
|
||||
try {
|
||||
await testPage.evaluate(() => {
|
||||
(window as Window & { __taskLinkDocumentSentinel?: string }).__taskLinkDocumentSentinel =
|
||||
"dependency-links-stay-in-spa";
|
||||
});
|
||||
|
||||
const chip = testPage.getByTestId("task-dependency-chip");
|
||||
await chip.click();
|
||||
const blockedBy = testPage
|
||||
.getByTestId("task-dependency-entry")
|
||||
.filter({ hasText: predecessor.title });
|
||||
await expect(blockedBy).toHaveAttribute("href", `/t/${predecessor.id}`);
|
||||
await blockedBy.click();
|
||||
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${predecessor.id}$`));
|
||||
await expect(title).toHaveText(predecessor.title);
|
||||
await expect(testPage.getByTestId("task-dependency-chip-popover")).toHaveCount(0);
|
||||
expect(documentRequests).toHaveLength(0);
|
||||
expect(
|
||||
await testPage.evaluate(
|
||||
() =>
|
||||
(window as Window & { __taskLinkDocumentSentinel?: string }).__taskLinkDocumentSentinel,
|
||||
),
|
||||
).toBe("dependency-links-stay-in-spa");
|
||||
|
||||
await testPage.goBack();
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${target.id}$`));
|
||||
await expect(title).toHaveText(target.title);
|
||||
expect(documentRequests).toHaveLength(0);
|
||||
|
||||
await chip.click();
|
||||
const blocks = testPage
|
||||
.getByTestId("task-dependency-entry")
|
||||
.filter({ hasText: dependent.title });
|
||||
await expect(blocks).toHaveAttribute("href", `/t/${dependent.id}`);
|
||||
await blocks.click();
|
||||
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${dependent.id}$`));
|
||||
await expect(title).toHaveText(dependent.title);
|
||||
await expect(testPage.getByTestId("task-dependency-chip-popover")).toHaveCount(0);
|
||||
expect(documentRequests).toHaveLength(0);
|
||||
expect(
|
||||
await testPage.evaluate(
|
||||
() =>
|
||||
(window as Window & { __taskLinkDocumentSentinel?: string }).__taskLinkDocumentSentinel,
|
||||
),
|
||||
).toBe("dependency-links-stay-in-spa");
|
||||
} finally {
|
||||
testPage.off("request", onRequest);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,312 @@
|
||||
/**
|
||||
* Keep first-party task workbench navigation on the shared URL and link
|
||||
* authorities. This rule deliberately proves only the common static forms:
|
||||
* literal/template/concatenated task routes, simple local aliases, imported
|
||||
* linkToTask aliases, native task anchors, and window.location writes.
|
||||
*/
|
||||
|
||||
const TASK_ROUTE_PREFIX = /^\/(?:t|tasks)\//;
|
||||
const TASK_ROUTE_WITH_ID = /^\/(?:t|tasks)\/.+/;
|
||||
const LINKS_MODULE = /(?:^|\/)lib\/links(?:\.[cm]?[jt]s)?$/;
|
||||
|
||||
function unwrap(node) {
|
||||
if (!node) return null;
|
||||
if (
|
||||
node.type === "TSAsExpression" ||
|
||||
node.type === "TSTypeAssertion" ||
|
||||
node.type === "TSNonNullExpression" ||
|
||||
node.type === "ChainExpression"
|
||||
) {
|
||||
return unwrap(node.expression);
|
||||
}
|
||||
return node;
|
||||
}
|
||||
|
||||
function lookupVariable(scope, name) {
|
||||
for (let current = scope; current; current = current.upper) {
|
||||
const found = current.variables.find((variable) => variable.name === name);
|
||||
if (found) return found;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function variableForIdentifier(context, node) {
|
||||
if (node.type !== "Identifier") return null;
|
||||
return lookupVariable(context.sourceCode.getScope(node), node.name);
|
||||
}
|
||||
|
||||
function isLinksImport(def) {
|
||||
return def.type === "ImportBinding" && LINKS_MODULE.test(String(def.parent?.source?.value));
|
||||
}
|
||||
|
||||
function isLinkToTaskImport(def) {
|
||||
if (!isLinksImport(def)) return false;
|
||||
return def.node?.type === "ImportSpecifier" && def.node.imported?.name === "linkToTask";
|
||||
}
|
||||
|
||||
function isLinksNamespaceImport(def) {
|
||||
return isLinksImport(def) && def.node?.type === "ImportNamespaceSpecifier";
|
||||
}
|
||||
|
||||
function resolvesToLinkToTask(context, node, seen = new Set()) {
|
||||
const current = unwrap(node);
|
||||
if (!current) return false;
|
||||
|
||||
if (current.type === "MemberExpression") {
|
||||
const property = current.computed ? current.property : current.property;
|
||||
const propertyName = property.type === "Identifier" ? property.name : property.value;
|
||||
if (propertyName !== "linkToTask") return false;
|
||||
const namespace = current.object;
|
||||
if (namespace.type !== "Identifier") return false;
|
||||
const variable = variableForIdentifier(context, namespace);
|
||||
return Boolean(variable?.defs.some(isLinksNamespaceImport));
|
||||
}
|
||||
|
||||
if (current.type !== "Identifier") return false;
|
||||
const variable = variableForIdentifier(context, current);
|
||||
if (!variable || seen.has(variable)) return false;
|
||||
seen.add(variable);
|
||||
if (variable.defs.some(isLinkToTaskImport)) return true;
|
||||
|
||||
return variable.defs.some((def) => {
|
||||
if (def.type !== "Variable" || !def.node?.init) return false;
|
||||
return resolvesToLinkToTask(context, def.node.init, seen);
|
||||
});
|
||||
}
|
||||
|
||||
function staticStringFromVariable(context, node, seen) {
|
||||
const variable = variableForIdentifier(context, node);
|
||||
if (!variable || seen.has(variable)) return null;
|
||||
for (const def of variable.defs) {
|
||||
if (def.type !== "Variable" || !def.node?.init) continue;
|
||||
const branchSeen = new Set(seen);
|
||||
branchSeen.add(variable);
|
||||
const value = isStaticString(context, def.node.init, branchSeen);
|
||||
if (value !== null) return value;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function isStaticString(context, node, seen = new Set()) {
|
||||
const current = unwrap(node);
|
||||
if (!current) return null;
|
||||
if (current.type === "Literal" && typeof current.value === "string") return current.value;
|
||||
if (current.type === "TemplateLiteral" && current.expressions.length === 0) {
|
||||
return current.quasis[0]?.value.cooked ?? "";
|
||||
}
|
||||
if (current.type === "BinaryExpression" && current.operator === "+") {
|
||||
const left = isStaticString(context, current.left, new Set(seen));
|
||||
const right = isStaticString(context, current.right, new Set(seen));
|
||||
return left !== null && right !== null ? left + right : null;
|
||||
}
|
||||
return current.type === "Identifier" ? staticStringFromVariable(context, current, seen) : null;
|
||||
}
|
||||
|
||||
function routeFromTemplate(node) {
|
||||
const first = node.quasis[0]?.value.cooked ?? "";
|
||||
if (!TASK_ROUTE_PREFIX.test(first)) return null;
|
||||
if (TASK_ROUTE_WITH_ID.test(first)) return "raw";
|
||||
return node.expressions.length > 0 ? "raw" : null;
|
||||
}
|
||||
|
||||
function routeFromBinary(context, node, seen) {
|
||||
const staticValue = isStaticString(context, node, new Set(seen));
|
||||
if (staticValue !== null) return TASK_ROUTE_WITH_ID.test(staticValue) ? "raw" : null;
|
||||
const left = isStaticString(context, node.left, new Set(seen));
|
||||
if (!left || !TASK_ROUTE_PREFIX.test(left)) return null;
|
||||
if (TASK_ROUTE_WITH_ID.test(left)) return "raw";
|
||||
// A dynamic right-hand side supplies the task segment. A static query or
|
||||
// hash after the bare prefix still has no task id and is not a detail route.
|
||||
const right = isStaticString(context, node.right, new Set(seen));
|
||||
return right === null ? "raw" : null;
|
||||
}
|
||||
|
||||
function combineRouteKinds(left, right) {
|
||||
if (left === "raw" || right === "raw") return "raw";
|
||||
if (left === "builder" || right === "builder") return "builder";
|
||||
return null;
|
||||
}
|
||||
|
||||
function routeKindFromVariable(context, node, seen) {
|
||||
const variable = variableForIdentifier(context, node);
|
||||
if (!variable || seen.has(variable)) return null;
|
||||
if (variable.defs.some(isLinkToTaskImport)) return "builder";
|
||||
for (const def of variable.defs) {
|
||||
if (def.type !== "Variable" || !def.node?.init) continue;
|
||||
const branchSeen = new Set(seen);
|
||||
branchSeen.add(variable);
|
||||
const value = routeKind(context, def.node.init, branchSeen);
|
||||
if (value) return value;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function routeKindFromComposite(context, node, seen) {
|
||||
const left = routeKind(context, node.left ?? node.consequent, new Set(seen));
|
||||
const right = routeKind(context, node.right ?? node.alternate, new Set(seen));
|
||||
return combineRouteKinds(left, right);
|
||||
}
|
||||
|
||||
function routeKind(context, node, seen = new Set()) {
|
||||
const current = unwrap(node);
|
||||
if (!current) return null;
|
||||
switch (current.type) {
|
||||
case "Literal":
|
||||
return typeof current.value === "string" && TASK_ROUTE_WITH_ID.test(current.value)
|
||||
? "raw"
|
||||
: null;
|
||||
case "TemplateLiteral":
|
||||
return routeFromTemplate(current);
|
||||
case "BinaryExpression":
|
||||
return current.operator === "+" ? routeFromBinary(context, current, seen) : null;
|
||||
case "ConditionalExpression":
|
||||
case "LogicalExpression":
|
||||
return routeKindFromComposite(context, current, seen);
|
||||
case "SequenceExpression":
|
||||
return routeKind(context, current.expressions.at(-1), seen);
|
||||
case "CallExpression":
|
||||
return resolvesToLinkToTask(context, current.callee) ? "builder" : null;
|
||||
case "Identifier":
|
||||
return routeKindFromVariable(context, current, seen);
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function isNativeAnchorHref(node) {
|
||||
const opening = node.parent;
|
||||
return (
|
||||
opening?.type === "JSXOpeningElement" &&
|
||||
opening.name?.type === "JSXIdentifier" &&
|
||||
opening.name.name === "a"
|
||||
);
|
||||
}
|
||||
|
||||
function hrefExpression(node) {
|
||||
if (!node) return null;
|
||||
if (node.type === "JSXExpressionContainer") return node.expression;
|
||||
return node;
|
||||
}
|
||||
|
||||
function isLocationObject(node) {
|
||||
const current = unwrap(node);
|
||||
if (!current) return false;
|
||||
if (current.type === "Identifier") return current.name === "location";
|
||||
return (
|
||||
current.type === "MemberExpression" &&
|
||||
!current.computed &&
|
||||
current.property.type === "Identifier" &&
|
||||
current.property.name === "location" &&
|
||||
current.object.type === "Identifier" &&
|
||||
(current.object.name === "window" || current.object.name === "globalThis")
|
||||
);
|
||||
}
|
||||
|
||||
function isLocationHref(node) {
|
||||
const current = unwrap(node);
|
||||
return (
|
||||
current?.type === "MemberExpression" &&
|
||||
((current.computed &&
|
||||
current.property.type === "Literal" &&
|
||||
current.property.value === "href") ||
|
||||
(!current.computed &&
|
||||
current.property.type === "Identifier" &&
|
||||
current.property.name === "href")) &&
|
||||
isLocationObject(current.object)
|
||||
);
|
||||
}
|
||||
|
||||
function isLocationMethod(node) {
|
||||
const current = unwrap(node);
|
||||
if (current?.type !== "MemberExpression" || !isLocationObject(current.object)) return false;
|
||||
const property = current.computed ? current.property : current.property;
|
||||
const name = property.type === "Identifier" ? property.name : property.value;
|
||||
return name === "assign" || name === "replace";
|
||||
}
|
||||
|
||||
function isRouteContainer(parent, node) {
|
||||
if (parent.type === "BinaryExpression" && parent.operator === "+") return true;
|
||||
if (parent.type === "ConditionalExpression") {
|
||||
return parent.consequent === node || parent.alternate === node;
|
||||
}
|
||||
if (parent.type === "LogicalExpression") return parent.right === node;
|
||||
if (parent.type === "TSAsExpression") return parent.expression === node;
|
||||
if (parent.type === "TSTypeAssertion") return parent.expression === node;
|
||||
if (parent.type === "TSNonNullExpression") return parent.expression === node;
|
||||
return parent.type === "ChainExpression" && parent.expression === node;
|
||||
}
|
||||
|
||||
function isNestedRoutePart(node, context) {
|
||||
const parent = node.parent;
|
||||
return Boolean(parent && isRouteContainer(parent, node) && routeKind(context, parent) === "raw");
|
||||
}
|
||||
|
||||
/** @type {import("eslint").Rule.RuleModule} */
|
||||
export const noTaskLinkBypass = {
|
||||
meta: {
|
||||
type: "problem",
|
||||
docs: {
|
||||
description: "Require shared task URL and link primitives for task workbench navigation.",
|
||||
},
|
||||
schema: [],
|
||||
messages: {
|
||||
taskLinkBypass:
|
||||
"Use linkToTask() for task workbench URLs and TaskLink/AppLink for task anchors; do not hand-build or natively navigate task-detail links.",
|
||||
},
|
||||
},
|
||||
create(context) {
|
||||
const reported = new WeakSet();
|
||||
const report = (node) => {
|
||||
if (!node || reported.has(node)) return;
|
||||
reported.add(node);
|
||||
context.report({ node, messageId: "taskLinkBypass" });
|
||||
};
|
||||
|
||||
return {
|
||||
JSXAttribute(node) {
|
||||
if (node.name?.type !== "JSXIdentifier" || node.name.name !== "href") return;
|
||||
const expression = hrefExpression(node.value);
|
||||
const kind = routeKind(context, expression);
|
||||
if (!kind) return;
|
||||
if (kind === "builder" && !isNativeAnchorHref(node)) return;
|
||||
// Local raw aliases are reported at their defining expression. This
|
||||
// avoids a second diagnostic on the consuming JSX attribute.
|
||||
if (kind === "raw" && expression?.type === "Identifier") return;
|
||||
report(expression ?? node);
|
||||
},
|
||||
AssignmentExpression(node) {
|
||||
if (isLocationHref(node.left) && routeKind(context, node.right)) report(node.right);
|
||||
},
|
||||
CallExpression(node) {
|
||||
if (isLocationMethod(node.callee) && routeKind(context, node.arguments[0])) {
|
||||
report(node.arguments[0]);
|
||||
}
|
||||
},
|
||||
Literal(node) {
|
||||
if (typeof node.value !== "string" || isNestedRoutePart(node, context)) return;
|
||||
if (node.parent?.type === "JSXAttribute") return;
|
||||
if (routeKind(context, node) === "raw") report(node);
|
||||
},
|
||||
TemplateLiteral(node) {
|
||||
if (isNestedRoutePart(node, context)) return;
|
||||
if (routeKind(context, node) === "raw") report(node);
|
||||
},
|
||||
BinaryExpression(node) {
|
||||
if (node.operator !== "+" || isNestedRoutePart(node, context)) return;
|
||||
if (routeKind(context, node) === "raw") report(node);
|
||||
},
|
||||
ConditionalExpression(node) {
|
||||
if (routeKind(context, node) === "raw") report(node);
|
||||
},
|
||||
LogicalExpression(node) {
|
||||
if (routeKind(context, node) === "raw") report(node);
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
export const taskLinksPlugin = {
|
||||
rules: { "no-task-link-bypass": noTaskLinkBypass },
|
||||
};
|
||||
|
||||
export const TASK_LINK_RULE_ID = "task-links/no-task-link-bypass";
|
||||
@@ -0,0 +1,116 @@
|
||||
import { RuleTester } from "eslint";
|
||||
import tseslint from "typescript-eslint";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { noTaskLinkBypass } from "./no-task-link-bypass.mjs";
|
||||
|
||||
RuleTester.describe = describe;
|
||||
RuleTester.it = it;
|
||||
|
||||
const ruleTester = new RuleTester({
|
||||
languageOptions: {
|
||||
parser: tseslint.parser as never,
|
||||
parserOptions: { ecmaVersion: 2022, sourceType: "module", ecmaFeatures: { jsx: true } },
|
||||
},
|
||||
});
|
||||
|
||||
ruleTester.run("no-task-link-bypass", noTaskLinkBypass, {
|
||||
valid: [
|
||||
{ code: 'const prefix = "/t/"; pathname.startsWith(prefix);' },
|
||||
{ code: 'for (const prefix of ["/t/", "/tasks/"]) pathname.startsWith(prefix);' },
|
||||
{ code: "const apiPath = `/api/tasks/${taskId}`;" },
|
||||
{ code: "const officePath = `/office/tasks/${taskId}`;" },
|
||||
{ code: "router.push(linkToTask(taskId));" },
|
||||
{
|
||||
code: `import { linkToTask } from "@/lib/links";
|
||||
import Link from "@/components/routing/app-link";
|
||||
<Link href={linkToTask(taskId)} />;`,
|
||||
},
|
||||
{
|
||||
code: `import { linkToTask } from "@/lib/links";
|
||||
import AppLink from "@/components/routing/app-link";
|
||||
<AppLink href={linkToTask(taskId)} />;`,
|
||||
},
|
||||
{ code: "<TaskLink taskId={taskId} />;" },
|
||||
{ code: 'const tasksPath = "/tasks";' },
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
code: 'const href = "/t/task-1";',
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: "const href = `/tasks/${taskId}`;",
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: 'const href = "/t/" + taskId;',
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: 'const href = condition ? `/t/${taskId}` : "/tasks/fallback";',
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: "const href = condition && `/t/${taskId}`;",
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: '<a href="/t/task-1">Task</a>;',
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: '<Link href="/t/task-1">Task</Link>;',
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: '<AppLink href={"/tasks/task-1"}>Task</AppLink>;',
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: `import { linkToTask as taskHref } from "@/lib/links";
|
||||
<a href={taskHref(taskId)}>Task</a>;`,
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: `import { linkToTask } from "@/lib/links";
|
||||
const href = linkToTask(taskId);
|
||||
<a href={href}>Task</a>;`,
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: `import * as Links from "@/lib/links";
|
||||
<a href={Links.linkToTask(taskId)}>Task</a>;`,
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: "window.location.assign(`/tasks/${taskId}`);",
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: "window.location.href = `/t/${taskId}`;",
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: `function openTask(taskId: string) {
|
||||
const prefix = "/tasks/";
|
||||
const href = prefix + taskId;
|
||||
return <a href={href}>Task</a>;
|
||||
}`,
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
{
|
||||
code: `function openTask(taskId: string) {
|
||||
const prefix = "/tasks/";
|
||||
window.location.assign(prefix + taskId);
|
||||
}`,
|
||||
errors: [{ messageId: "taskLinkBypass" }],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
describe("no-task-link-bypass rule", () => {
|
||||
it("keeps the diagnostic focused on first-party task workbench routes", () => {
|
||||
expect(noTaskLinkBypass.meta?.messages?.taskLinkBypass).toContain("TaskLink");
|
||||
});
|
||||
});
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
e2eSleepPlugin,
|
||||
SLEEP_EXEMPT_FILES,
|
||||
} from "./eslint-rules/no-unsanctioned-sleep.mjs";
|
||||
import { taskLinksPlugin } from "./eslint-rules/no-task-link-bypass.mjs";
|
||||
|
||||
const eslintConfig = defineConfig([
|
||||
{
|
||||
@@ -86,6 +87,24 @@ const eslintConfig = defineConfig([
|
||||
plugins: { i18next },
|
||||
rules: { "i18next/no-literal-string": ["error", noLiteralStringOptions] },
|
||||
},
|
||||
// First-party task workbench URLs and anchors have one URL authority and one
|
||||
// click transport. Tests and the authority module itself are excluded so the
|
||||
// rule checks callers rather than its own implementation and fixtures.
|
||||
{
|
||||
files: ["**/*.ts", "**/*.tsx"],
|
||||
ignores: [
|
||||
"**/*.test.ts",
|
||||
"**/*.test.tsx",
|
||||
"**/*.test-helpers.ts",
|
||||
"**/*.test-helpers.tsx",
|
||||
"**/*.test-utils.ts",
|
||||
"**/*.test-utils.tsx",
|
||||
"e2e/**",
|
||||
"lib/links.ts",
|
||||
],
|
||||
plugins: { "task-links": taskLinksPlugin },
|
||||
rules: { "task-links/no-task-link-bypass": "error" },
|
||||
},
|
||||
// E2E tests (Playwright): disable React hooks rules since Playwright's `use()` and
|
||||
// `test.extend()` patterns are falsely flagged, and relax test-specific limits.
|
||||
{
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { EntityReference } from "@/lib/types/entity-reference";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
|
||||
const IDENTITY_NAME = /^[a-z0-9][a-z0-9._:-]{0,127}$/;
|
||||
const CONTROL_CHARACTER = /[\u0000-\u001f\u007f-\u009f]/u;
|
||||
@@ -57,7 +58,7 @@ function hasSafeURL(reference: EntityReference): boolean {
|
||||
return false;
|
||||
}
|
||||
if (reference.provider === "kandev" && reference.kind === "task") {
|
||||
return reference.url === `/t/${encodeURIComponent(reference.id)}`;
|
||||
return reference.url === linkToTask(reference.id);
|
||||
}
|
||||
try {
|
||||
const parsed = new URL(reference.url);
|
||||
|
||||
@@ -19,6 +19,23 @@ describe("task links", () => {
|
||||
expect(linkToTask("task-123", "plan")).toBe("/t/task-123?layout=plan");
|
||||
});
|
||||
|
||||
it("encodes the raw task id once and merges task query context", () => {
|
||||
const searchParams = new URLSearchParams([
|
||||
["layout", "old"],
|
||||
["tag", "first"],
|
||||
["tag", "second"],
|
||||
]);
|
||||
|
||||
expect(
|
||||
linkToTask("task/123%raw", {
|
||||
searchParams,
|
||||
layout: "plan",
|
||||
sessionId: "session 123",
|
||||
}),
|
||||
).toBe("/t/task%2F123%25raw?layout=plan&tag=first&tag=second&sessionId=session+123");
|
||||
expect(searchParams.toString()).toBe("layout=old&tag=first&tag=second");
|
||||
});
|
||||
|
||||
it("keeps /tasks for the task list route", () => {
|
||||
expect(linkToTasks()).toBe("/tasks");
|
||||
expect(linkToTasks("workspace-123")).toBe("/tasks?workspace=workspace-123");
|
||||
|
||||
+19
-3
@@ -1,6 +1,22 @@
|
||||
export function linkToTask(taskId: string, layout?: string): string {
|
||||
const base = `/t/${taskId}`;
|
||||
return layout ? `${base}?layout=${encodeURIComponent(layout)}` : base;
|
||||
export type TaskLinkOptions = {
|
||||
layout?: string;
|
||||
sessionId?: string;
|
||||
searchParams?: URLSearchParams;
|
||||
};
|
||||
|
||||
export function linkToTask(taskId: string, layout?: string): string;
|
||||
export function linkToTask(taskId: string, options?: TaskLinkOptions): string;
|
||||
export function linkToTask(taskId: string, layoutOrOptions?: string | TaskLinkOptions): string {
|
||||
const base = `/t/${encodeURIComponent(taskId)}`;
|
||||
const options =
|
||||
typeof layoutOrOptions === "string" ? { layout: layoutOrOptions } : (layoutOrOptions ?? {});
|
||||
const searchParams = new URLSearchParams(options.searchParams);
|
||||
|
||||
if (options.layout !== undefined) searchParams.set("layout", options.layout);
|
||||
if (options.sessionId !== undefined) searchParams.set("sessionId", options.sessionId);
|
||||
|
||||
const query = searchParams.toString();
|
||||
return query ? `${base}?${query}` : base;
|
||||
}
|
||||
|
||||
export function linkToTaskOverview({
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
import path from "node:path";
|
||||
|
||||
import { ESLint } from "eslint";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
const WEB_DIR = path.resolve(import.meta.dirname, "../..");
|
||||
const eslint = new ESLint({ cwd: WEB_DIR });
|
||||
|
||||
describe("task link eslint wiring", () => {
|
||||
it("guards a new production component in an arbitrary directory", async () => {
|
||||
const [result] = await eslint.lintText(
|
||||
`export function NewTaskLink({ taskId }: { taskId: string }) {
|
||||
return <a href={\`/t/\${taskId}\`} />;
|
||||
}
|
||||
`,
|
||||
{ filePath: path.join(WEB_DIR, "components/new-surface/task-link-fixture.tsx") },
|
||||
);
|
||||
|
||||
expect(result.messages).toEqual([
|
||||
expect.objectContaining({
|
||||
ruleId: "task-links/no-task-link-bypass",
|
||||
severity: 2,
|
||||
}),
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,122 @@
|
||||
# Task-link migration inventory
|
||||
|
||||
Source snapshot: 2026-09-21. Re-run the rule over the whole production tree;
|
||||
this literal search is evidence, not an exhaustive static analysis.
|
||||
Classify bare route prefixes as recognizers, links.ts as authority, and the rest
|
||||
as builders/consumers. API-relative `/tasks/...` paths require transport-aware
|
||||
classification. Do not rewrite API contracts.
|
||||
|
||||
Task 02 must record every migrated caller and its changed test. Before its
|
||||
`vitest related` command, write `/tmp/kandev-task-link-changed-sources.txt` with
|
||||
one web-package-relative changed source path per line (no test paths). These
|
||||
repository paths have no spaces. Use the explicit resulting test list in the
|
||||
work-order Results; run changed test files directly if related discovery misses
|
||||
one. The temporary file is a command input, not a persistent artifact.
|
||||
|
||||
- `apps/web/components/linear/linear-quick-task-launcher.tsx`
|
||||
- `87: if (meta?.autoFocus !== false) router.push(/tasks/${task.id});`
|
||||
|
||||
- `apps/web/components/automations/runs-section.tsx`
|
||||
- `329: onNavigate={(id) => router.push(/tasks/${id})}`
|
||||
|
||||
- `apps/web/components/runs/runs-list-page.tsx`
|
||||
- `234: onOpen={(taskId) => router.push(/tasks/${taskId})}`
|
||||
|
||||
- `apps/web/components/runs/runs-page-client.tsx`
|
||||
- `194: onOpen={(taskId) => router.push(/tasks/${taskId})}`
|
||||
|
||||
- `apps/web/components/task/task-session-sidebar.tsx`
|
||||
- `314: !!pathname && (pathname.startsWith("/t/") || pathname.startsWith("/office/tasks/"));`
|
||||
- `469: !!pathname && (pathname.startsWith("/t/") || pathname.startsWith("/office/tasks/"));`
|
||||
|
||||
- `apps/web/components/task/task-dependency-chip.tsx`
|
||||
- `163: href={/tasks/${entry.id}}`
|
||||
|
||||
- `apps/web/components/app-sidebar/app-sidebar.tsx`
|
||||
- `37: matches: (p) => p.startsWith("/t/"),`
|
||||
|
||||
- `apps/web/components/inbox-history/inbox-history-row.tsx`
|
||||
- `35: if (!bundle.session_id) return /t/${bundle.task_id};`
|
||||
- `36: return /t/${bundle.task_id}?sessionId=${encodeURIComponent(bundle.session_id)};`
|
||||
|
||||
- `apps/web/components/azure-devops/azure-devops-task-launcher.tsx`
|
||||
- `189: if (meta?.autoFocus !== false) router.push(/tasks/${task.id});`
|
||||
|
||||
- `apps/web/components/needs-you-inbox/needs-you-inbox-row.tsx`
|
||||
- `170: if (!bundle.session_id) return /t/${bundle.task_id};`
|
||||
- `171: return /t/${bundle.task_id}?sessionId=${encodeURIComponent(bundle.session_id)};`
|
||||
|
||||
- `apps/web/components/needs-you-inbox/failed-inbox-row.tsx`
|
||||
- `19: return /t/${row.task_id};`
|
||||
|
||||
- `apps/web/components/settings/workspace-canvases-page.tsx`
|
||||
- `107: router.push(/t/${encodeURIComponent(response.task_id)}${query});`
|
||||
|
||||
- `apps/web/components/settings/canvas-host-route.tsx`
|
||||
- `378: router.push(/t/${encodeURIComponent(response.task_id)}${query});`
|
||||
|
||||
- `apps/web/components/settings/kubernetes-session-utils.ts`
|
||||
- `89: return /t/${encodeURIComponent(taskId)};`
|
||||
|
||||
- `apps/web/components/jira/my-jira/quick-task-launcher.tsx`
|
||||
- `65: if (meta?.autoFocus !== false) router.push(/tasks/${task.id});`
|
||||
|
||||
- `apps/web/components/task/simple/OfficeSimplePane.tsx`
|
||||
- `145: href={/t/${task.id}}`
|
||||
|
||||
- `apps/web/components/automations/trigger-configs/plugin-webhook-controls.tsx`
|
||||
- `84: <a className="underline" href={/tasks/${receipt.task_id}}>`
|
||||
|
||||
- `apps/web/components/github/my-github/quick-task-launcher.tsx`
|
||||
- `239: if (meta?.autoFocus !== false) router.push(/tasks/${task.id});`
|
||||
|
||||
- `apps/web/lib/links.ts`
|
||||
- `2: const base = /t/${taskId};`
|
||||
- `30: const TASK_DETAIL_PREFIXES = ["/t/", "/tasks/"];`
|
||||
|
||||
- `apps/web/lib/entity-references/message-references.ts`
|
||||
- `60: return reference.url === /t/${encodeURIComponent(reference.id)};`
|
||||
|
||||
- `apps/web/app/tasks/[id]/kanban-task-shell.tsx`
|
||||
- `156: const href = target === "office" ? /office/tasks/${taskId} : /t/${taskId};`
|
||||
|
||||
- `apps/web/app/tasks/[id]/page.tsx`
|
||||
- `29: redirect(queryString ? /t/${id}?${queryString} : /t/${id});`
|
||||
|
||||
- `apps/web/app/office/tasks/[id]/task-advanced-mode.tsx`
|
||||
- `51: href={/t/${task.id}}`
|
||||
|
||||
- `apps/web/src/spa-routes.tsx`
|
||||
- `746: for (const prefix of ["/t/", "/tasks/"]) {`
|
||||
|
||||
## Task 02 migration result
|
||||
|
||||
All builders listed above now use `linkToTask` with raw task IDs. Direct task
|
||||
anchors use `TaskLink`; Office, API, listing, and route-recognition paths keep
|
||||
their separate contracts. The following callers changed:
|
||||
|
||||
- `components/automations/runs-section.tsx` → `components/automations/runs-section.test.tsx`
|
||||
- `components/runs/runs-page-client.tsx` → `components/runs/runs-page-client.test.tsx`
|
||||
- `components/runs/runs-list-page.tsx` → `components/runs/runs-list-page.test.tsx`
|
||||
- `components/azure-devops/azure-devops-task-launcher.tsx` → `components/azure-devops/azure-devops-task-launcher.test.tsx`
|
||||
- `components/github/my-github/quick-task-launcher.tsx` → `components/github/my-github/quick-task-launcher.test.tsx`
|
||||
- `components/linear/linear-quick-task-launcher.tsx` → existing launcher contract retained; no path-specific assertion needed
|
||||
- `components/jira/my-jira/quick-task-launcher.tsx` → existing launcher contract retained; no path-specific assertion needed
|
||||
- `components/automations/trigger-configs/plugin-webhook-controls.tsx` → existing rendered link contract retained
|
||||
- `components/task/simple/OfficeSimplePane.tsx` → existing cross-link contract retained
|
||||
- `app/office/tasks/[id]/task-advanced-mode.tsx` → existing cross-link contract retained
|
||||
- `app/tasks/[id]/kanban-task-shell.tsx` → existing Office/Kanban cross-link contract retained
|
||||
- `app/tasks/[id]/page.tsx` → compatibility redirect contract retained
|
||||
- `components/inbox-history/inbox-history-row.tsx` → existing session query contract retained
|
||||
- `components/needs-you-inbox/needs-you-inbox-row.tsx` → existing session query contract retained
|
||||
- `components/needs-you-inbox/failed-inbox-row.tsx` → existing task href contract retained
|
||||
- `components/settings/system/bundle-customizer.tsx` → existing alternate-target contract retained
|
||||
- `components/settings/workspace-canvases-page.tsx` → existing canvas session context retained
|
||||
- `components/settings/canvas-host-route.tsx` → existing canvas session context retained
|
||||
- `components/settings/kubernetes-session-utils.ts` → existing task href helper contract retained
|
||||
- `lib/entity-references/message-references.ts` → existing encoded entity-reference contract retained
|
||||
- `components/task/task-dependency-chip.tsx` → `components/task/task-dependency-chip.test.tsx`
|
||||
|
||||
The enforcement rule and real-config wiring are covered by
|
||||
`eslint-rules/no-task-link-bypass.test.ts` and
|
||||
`scripts/lib/task-link-wiring.test.ts`.
|
||||
@@ -0,0 +1,128 @@
|
||||
---
|
||||
created: 2026-09-21
|
||||
status: complete
|
||||
requirements:
|
||||
- REQ-UI-TASK-NAVIGATION-001
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/task-navigation.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Task link navigation
|
||||
|
||||
## Overview
|
||||
|
||||
Fix dependency navigation and prevent recurrence across first-party task links.
|
||||
The shared primitives, first-party migration, enforcement, and desktop/mobile
|
||||
proof are complete across the three sequential work orders.
|
||||
|
||||
## Evidence and assumptions
|
||||
|
||||
Confirmed: the user requests a fix plan and central task-link construction.
|
||||
Verified: DependencyRow uses a native anchor to `/tasks/${entry.id}`. Both
|
||||
Blocks and Blocked by share the row, on desktop and touch. Existing component
|
||||
and E2E tests check visibility but never activate the dependency link.
|
||||
Verified: linkToTask already builds `/t/:id`; AppLink already handles guarded
|
||||
SPA navigation. Thus URL construction and click transport need separate guards.
|
||||
Source tracing establishes the cause; a browser reproduction has not been run.
|
||||
No material product choice remains unresolved. The reusable navigation contract
|
||||
belongs to UI; dependency state remains owned by Tasks.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Shared TaskLink, compatible URL-builder extension, committed-navigation callback.
|
||||
- Dependency popover/drawer fix, touch targets, regression coverage.
|
||||
- Migration of first-party hand-built task-detail URLs and known raw task anchors.
|
||||
- ESLint enforcement, wiring coverage, and scoped engineering guidance.
|
||||
|
||||
### Out of scope
|
||||
|
||||
Backend routes/APIs, Office destination semantics, external/plugin links, user
|
||||
content rewriting, a generic route catalog, and layout redesign.
|
||||
|
||||
## Technical approach
|
||||
|
||||
Follow [the design](../../specs/ui/system-design/task-navigation.md).
|
||||
Use linkToTask as URL authority and AppLink as click/guard authority; TaskLink
|
||||
combines them without another routing implementation. Preserve session, layout,
|
||||
canvas query context, explicit new-tab behavior, and compatibility routes.
|
||||
|
||||
## ASCII UI preview
|
||||
|
||||
UI-01: Dependency navigation, expanded state (AC .1, .4, .5, .6).
|
||||
|
||||
```text
|
||||
Desktop: [blocks 2] -> Popover: [Task B | REVIEW] -> Task B workbench
|
||||
Phone: [blocks 2] -> Drawer:
|
||||
Dependencies [Close]
|
||||
[Task B | REVIEW] (44px+ tap row)
|
||||
[Task C | CREATED] (list scrolls)
|
||||
-> Task B workbench; drawer closes
|
||||
Before: selecting a row loads a new document through /tasks/:id.
|
||||
After: selecting a row commits SPA navigation to /t/:id.
|
||||
```
|
||||
|
||||
Control order, touch targets, and successful-navigation dismissal are required.
|
||||
Spacing is illustrative. Retain existing localized labels and status displays.
|
||||
Cancelled navigation keeps the disclosure; modified clicks keep the current page.
|
||||
|
||||
## Tests
|
||||
|
||||
| Acceptance | Evidence |
|
||||
| --- | --- |
|
||||
| .1, .3, .4 | task-link.test.tsx, app-link.test.tsx, task-dependency-chip.test.tsx |
|
||||
| .2 | links.test.ts and migrated caller tests |
|
||||
| .1, .2 prevention | no-task-link-bypass.test.ts and task-link-wiring.test.ts |
|
||||
| .1, .4, .5, .6 | Browser scenarios below |
|
||||
|
||||
## E2E tests
|
||||
|
||||
Add the desktop test `dependency links preserve the desktop task workbench` to
|
||||
`e2e/tests/task/task-dependencies.spec.ts` (chromium) and the mobile test
|
||||
`mobile dependency links preserve the task workbench and touch layout` in
|
||||
`e2e/tests/task/mobile-task-dependency-navigation.spec.ts` (mobile-chrome).
|
||||
They exercise Blocks and Blocked by, target identity, canonical href, disclosure
|
||||
closure, document sentinel survival, no main-frame document request, and Back.
|
||||
The phone test uses the real drawer and measures 44px targets and viewport
|
||||
containment.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Shared task links and dependency fix](task-01-shared-task-links.md)
|
||||
- [x] [Task 02: Migrate and enforce task URL construction](task-02-enforce-task-links.md)
|
||||
- [x] [Task 03: Desktop and touch navigation proof](task-03-browser-regressions.md)
|
||||
|
||||
## Verification results
|
||||
|
||||
Implementation completed on 2026-09-21:
|
||||
|
||||
- Task 01 added `TaskLink`, successful-navigation callbacks, canonical dependency
|
||||
links, guarded disclosure dismissal, and touch-sized dependency rows.
|
||||
- Task 02 migrated first-party task destinations, added bounded ESLint
|
||||
enforcement, added real-config wiring coverage, and recorded the migration
|
||||
inventory.
|
||||
- Task 03 added desktop and mobile no-reload regressions. The mobile layout now
|
||||
forwards the task status identity needed by sessionless dependency tasks.
|
||||
- Focused Vitest checks passed: 6 files and 51 tests for the routing, dependency,
|
||||
and rule contracts.
|
||||
- Broad related Vitest checks passed: 437 files and 4,254 tests.
|
||||
- `pnpm run typecheck` and `pnpm run lint` passed.
|
||||
- Chromium dependency E2E passed: 6 tests.
|
||||
- Mobile dependency E2E passed: 1 test.
|
||||
- Specification validation, specification lint, and `git diff --check` passed.
|
||||
|
||||
Planning validation passed on 2026-09-21:
|
||||
|
||||
- `python3 scripts/list-docs.py validate`: 294 decisions and 1067 specifications validated.
|
||||
- `python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `git diff --check -- docs/specs docs/plans/task-link-navigation`: passed.
|
||||
- Catalog discovery includes the new requirement and design. Git status includes all three new work orders and the inventory.
|
||||
|
||||
## Risks
|
||||
|
||||
- Changing query construction must preserve duplicate parameters and avoid double encoding.
|
||||
- Closing a disclosure before a dirty-navigation guard commits would lose context.
|
||||
- Static rules cannot resolve arbitrary runtime strings; test bounded detection honestly.
|
||||
- Compatibility route recognition and API URLs must not become false positives.
|
||||
@@ -0,0 +1,113 @@
|
||||
---
|
||||
id: "01-shared-task-links"
|
||||
title: "Shared task links and dependency fix"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-TASK-NAVIGATION-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-TASK-NAVIGATION-001.1
|
||||
- AC-UI-TASK-NAVIGATION-001.2
|
||||
- AC-UI-TASK-NAVIGATION-001.3
|
||||
- AC-UI-TASK-NAVIGATION-001.4
|
||||
- AC-UI-TASK-NAVIGATION-001.5
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/task-navigation.md
|
||||
---
|
||||
|
||||
# Task 01: Shared task links and dependency fix
|
||||
|
||||
## Summary
|
||||
|
||||
Centralize task anchors and repair both dependency directions with the existing router.
|
||||
|
||||
## In scope
|
||||
|
||||
- Add TaskLink and compatible linkToTask options; preserve raw IDs and query context.
|
||||
- Add AppLink onNavigated within the successful guard callback.
|
||||
- Switch DependencyRow and pass a close callback; scope 44px minimum rows to touch.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Backend behavior and unrelated navigation redesign.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Failing regression: clicking a dependency row is intercepted and opens `/t/b`; today its native click is not prevented.
|
||||
- Cover both directions, canonical URL/encoding, ordinary and keyboard activation, modified/middle clicks, explicit targets, prevented clicks, guard rejection/acceptance, and callback timing.
|
||||
- Desktop stays compact; phone drawer closes only on committed same-tab navigation.
|
||||
|
||||
## ASCII UI preview
|
||||
|
||||
UI-01: Dependency navigation, expanded state (AC .1, .4, .5, .6).
|
||||
|
||||
```text
|
||||
Desktop: [blocks 2] -> Popover: [Task B | REVIEW] -> Task B workbench
|
||||
Phone: [blocks 2] -> Drawer:
|
||||
Dependencies [Close]
|
||||
[Task B | REVIEW] (44px+ tap row)
|
||||
[Task C | CREATED] (list scrolls)
|
||||
-> Task B workbench; drawer closes
|
||||
Before: selecting a row loads a new document through /tasks/:id.
|
||||
After: selecting a row commits SPA navigation to /t/:id.
|
||||
```
|
||||
|
||||
Control order, touch targets, and successful-navigation dismissal are required.
|
||||
Spacing is illustrative. Retain existing localized labels and status displays.
|
||||
Cancelled navigation keeps the disclosure; modified clicks keep the current page.
|
||||
|
||||
See [full preview](plan.md#ascii-ui-preview).
|
||||
|
||||
## Verification
|
||||
|
||||
Run from repository root. Install once with `(cd apps && pnpm install --frozen-lockfile)` if this worktree lacks dependencies.
|
||||
|
||||
```bash
|
||||
(cd apps/web && pnpm exec vitest run lib/links.test.ts components/routing/app-link.test.tsx components/routing/task-link.test.tsx components/task/task-dependency-chip.test.tsx)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps/web && pnpm exec eslint lib/links.ts components/routing/app-link.tsx components/routing/task-link.tsx components/task/task-dependency-chip.tsx --max-warnings 0)
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/lib/links.ts`
|
||||
- `apps/web/lib/links.test.ts`
|
||||
- `apps/web/components/routing/app-link.tsx`
|
||||
- `apps/web/components/routing/app-link.test.tsx`
|
||||
- `apps/web/components/routing/task-link.tsx (new)`
|
||||
- `apps/web/components/routing/task-link.test.tsx (new)`
|
||||
- `apps/web/components/task/task-dependency-chip.tsx`
|
||||
- `apps/web/components/task/task-dependency-chip.test.tsx`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
Preserve existing route guards, query context, and browser-native alternate-tab behavior.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Requirements](../../specs/ui/requirements/task-navigation.md)
|
||||
- [System design](../../specs/ui/system-design/task-navigation.md)
|
||||
- Existing AppLink, linkToTask, local ESLint rules, and task dependency tests.
|
||||
|
||||
## Results
|
||||
|
||||
Completed on 2026-09-21.
|
||||
|
||||
- The initial focused RED run failed on the six expected pre-fix behaviors:
|
||||
canonical URL construction, AppLink callback timing, dependency interception,
|
||||
disclosure dismissal, and touch-row sizing.
|
||||
- The focused Vitest suite passed 4 files and 32 tests after implementation.
|
||||
- Review follow-up added explicit blocker-cancellation coverage; the focused
|
||||
suite now passes 4 files and 33 tests.
|
||||
- `pnpm run typecheck` passed.
|
||||
- Focused ESLint passed with zero warnings.
|
||||
@@ -0,0 +1,97 @@
|
||||
---
|
||||
id: "02-enforce-task-links"
|
||||
title: "Migrate and enforce task URL construction"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ['01-shared-task-links']
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-TASK-NAVIGATION-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-TASK-NAVIGATION-001.1
|
||||
- AC-UI-TASK-NAVIGATION-001.2
|
||||
- AC-UI-TASK-NAVIGATION-001.3
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/task-navigation.md
|
||||
---
|
||||
|
||||
# Task 02: Migrate and enforce task URL construction
|
||||
|
||||
## Summary
|
||||
|
||||
Make the default lint command reject recurring URL and anchor bypasses, with zero current violations.
|
||||
|
||||
## In scope
|
||||
|
||||
- Add the bounded AST rule and actual-config wiring test described in the design.
|
||||
- Inventory and migrate every production task-detail builder; classify route-recognition constants separately.
|
||||
- Keep existing compliant Link/router consumers; migrate raw task anchors to TaskLink.
|
||||
- Update apps/web/AGENTS.md with URL and anchor ownership. No new ADR is needed: this applies existing authorities, with alternatives documented in the design.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Backend behavior and unrelated navigation redesign.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- RED: rule fixtures reject original DependencyRow, raw anchors using aliased helper calls, and hand-built URLs; valid API/Office/matcher cases pass.
|
||||
- Final production lint has no task-link violations or migration baseline; real config catches a new component in an arbitrary directory.
|
||||
- Migrated caller tests preserve context and selection side effects; new workbench destinations use canonical URLs.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from repository root. Install once with `(cd apps && pnpm install --frozen-lockfile)` if this worktree lacks dependencies.
|
||||
|
||||
```bash
|
||||
(cd apps/web && pnpm exec vitest run eslint-rules/no-task-link-bypass.test.ts scripts/lib/task-link-wiring.test.ts lib/links.test.ts)
|
||||
(cd apps/web && pnpm exec vitest related --run $(cat /tmp/kandev-task-link-changed-sources.txt))
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps/web && pnpm run lint)
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/eslint-rules/no-task-link-bypass.mjs (new)`
|
||||
- `apps/web/eslint-rules/no-task-link-bypass.test.ts (new)`
|
||||
- `apps/web/scripts/lib/task-link-wiring.test.ts (new)`
|
||||
- `apps/web/eslint.config.mjs`
|
||||
- `apps/web/AGENTS.md`
|
||||
- `First-party callers listed in migration-inventory.md and their tests`
|
||||
|
||||
## Dependencies
|
||||
|
||||
01-shared-task-links.
|
||||
|
||||
## Risks
|
||||
|
||||
Preserve existing route guards, query context, and browser-native alternate-tab behavior.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Requirements](../../specs/ui/requirements/task-navigation.md)
|
||||
- [System design](../../specs/ui/system-design/task-navigation.md)
|
||||
- Existing AppLink, linkToTask, local ESLint rules, and task dependency tests.
|
||||
|
||||
## Results
|
||||
|
||||
Completed on 2026-09-21.
|
||||
|
||||
- The bounded `task-links/no-task-link-bypass` rule and real-config wiring test
|
||||
passed, with 31 tests in the initial focused rule and link suite.
|
||||
- All known first-party task-detail builders and raw task anchors were migrated;
|
||||
the production rule scan found zero violations.
|
||||
- Review follow-up now checks quoted and expression literal task URLs on custom
|
||||
`Link` and `AppLink` elements while preserving shared-builder cases. Static
|
||||
evaluation uses branch-local recursion guards, so prefix aliases combined
|
||||
with dynamic task IDs are detected in anchors and `location.assign` calls.
|
||||
- The broad related Vitest run before review follow-up passed 437 files and
|
||||
4,254 tests.
|
||||
- Review follow-up coverage passed 3 files and 38 rule/link tests; the combined
|
||||
rule, AppLink, wiring, and link suite passed 4 files and 48 tests.
|
||||
- `pnpm run typecheck`, `pnpm run lint`, Prettier checks, and `git diff --check`
|
||||
passed.
|
||||
@@ -0,0 +1,112 @@
|
||||
---
|
||||
id: "03-browser-regressions"
|
||||
title: "Desktop and touch navigation proof"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on: ['02-enforce-task-links']
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-TASK-NAVIGATION-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-TASK-NAVIGATION-001.1
|
||||
- AC-UI-TASK-NAVIGATION-001.2
|
||||
- AC-UI-TASK-NAVIGATION-001.3
|
||||
- AC-UI-TASK-NAVIGATION-001.4
|
||||
- AC-UI-TASK-NAVIGATION-001.5
|
||||
- AC-UI-TASK-NAVIGATION-001.6
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/task-navigation.md
|
||||
---
|
||||
|
||||
# Task 03: Desktop and touch navigation proof
|
||||
|
||||
## Summary
|
||||
|
||||
Prove real task navigation retains the browser document on both responsive surfaces.
|
||||
|
||||
## In scope
|
||||
|
||||
- Seed related tasks through existing API fixtures and exercise both dependency directions.
|
||||
- Assert canonical destination plus target task identity, document sentinel, document request count, disclosure dismissal, and browser Back.
|
||||
- Phone: use mobile-chrome defaults and Drawer selectors; assert 44px rows, containment, single list scroll owner, no horizontal overflow.
|
||||
- Keep native modified-click and guard cancellation coverage in component tests from Task 01.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Backend behavior and unrelated navigation redesign.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Record RED evidence against the pre-fix implementation for navigation tests, using temporary local reversion of only the fix if needed; restore it before GREEN.
|
||||
- Desktop and mobile scenarios pass on fresh builds, not stale assets.
|
||||
- Record all command results and mark work orders/manifest complete only after their checks pass.
|
||||
|
||||
## ASCII UI preview
|
||||
|
||||
UI-01: Dependency navigation, expanded state (AC .1, .4, .5, .6).
|
||||
|
||||
```text
|
||||
Desktop: [blocks 2] -> Popover: [Task B | REVIEW] -> Task B workbench
|
||||
Phone: [blocks 2] -> Drawer:
|
||||
Dependencies [Close]
|
||||
[Task B | REVIEW] (44px+ tap row)
|
||||
[Task C | CREATED] (list scrolls)
|
||||
-> Task B workbench; drawer closes
|
||||
Before: selecting a row loads a new document through /tasks/:id.
|
||||
After: selecting a row commits SPA navigation to /t/:id.
|
||||
```
|
||||
|
||||
Control order, touch targets, and successful-navigation dismissal are required.
|
||||
Spacing is illustrative. Retain existing localized labels and status displays.
|
||||
Cancelled navigation keeps the disclosure; modified clicks keep the current page.
|
||||
|
||||
See [full preview](plan.md#ascii-ui-preview).
|
||||
|
||||
## Verification
|
||||
|
||||
Run from repository root. Install once with `(cd apps && pnpm install --frozen-lockfile)` if this worktree lacks dependencies.
|
||||
|
||||
```bash
|
||||
(cd apps/web && pnpm e2e:run --project chromium tests/task/task-dependencies.spec.ts)
|
||||
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-task-dependency-navigation.spec.ts)
|
||||
python3 scripts/list-docs.py validate
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/e2e/tests/task/task-dependencies.spec.ts`
|
||||
- `apps/web/e2e/tests/task/mobile-task-dependency-navigation.spec.ts (new)`
|
||||
- `docs/plans/task-link-navigation/plan.md`
|
||||
- `docs/plans/task-link-navigation/task-*.md`
|
||||
|
||||
## Dependencies
|
||||
|
||||
02-enforce-task-links.
|
||||
|
||||
## Risks
|
||||
|
||||
Preserve existing route guards, query context, and browser-native alternate-tab behavior.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Requirements](../../specs/ui/requirements/task-navigation.md)
|
||||
- [System design](../../specs/ui/system-design/task-navigation.md)
|
||||
- Existing AppLink, linkToTask, local ESLint rules, and task dependency tests.
|
||||
|
||||
## Results
|
||||
|
||||
Completed on 2026-09-21.
|
||||
|
||||
- The initial component RED run provided pre-fix navigation failure evidence;
|
||||
no temporary production reversion was needed for the browser scenarios.
|
||||
- Chromium dependency coverage passed all 6 tests, including both dependency
|
||||
directions, no document reload, disclosure closure, and browser Back.
|
||||
- Mobile dependency coverage passed 1 test, including the real drawer, both
|
||||
directions, 44px rows, containment, no document reload, and Back.
|
||||
- Specification validation, specification lint, and `git diff --check` passed.
|
||||
@@ -12,7 +12,8 @@ owners:
|
||||
## Scope
|
||||
|
||||
UI owns responsive behavior and reusable presentation preferences, including
|
||||
task-listing modes and Home destination choices. Other systems retain task,
|
||||
task-listing modes, Home destination choices, and reusable client-side task
|
||||
navigation. Other systems retain task,
|
||||
runtime, and workspace lifecycle/state ownership.
|
||||
|
||||
## Related
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
---
|
||||
status: active
|
||||
system: ui
|
||||
created: 2026-09-21
|
||||
owners:
|
||||
- kandev
|
||||
---
|
||||
|
||||
# Task navigation requirements
|
||||
|
||||
## Overview
|
||||
|
||||
First-party task links consistently open the task workbench. UI owns this
|
||||
reusable navigation interaction across features. Tasks retain identity,
|
||||
relationships, authorization, and lifecycle ownership.
|
||||
|
||||
## Requirements
|
||||
|
||||
### REQ-UI-TASK-NAVIGATION-001: Consistent task navigation
|
||||
|
||||
**Intent:** Users follow task links without restarting the loaded application.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-UI-TASK-NAVIGATION-001.1:** An ordinary click, keyboard activation, or touch on a first-party task-workbench link shall open the selected task without loading a new browser document.
|
||||
- **AC-UI-TASK-NAVIGATION-001.2:** Newly generated workbench URLs shall use `/t/:taskId`, preserve requested session, layout, and other supported query context, and encode each value once. Existing `/tasks/:id` deep links shall remain usable.
|
||||
- **AC-UI-TASK-NAVIGATION-001.3:** Task links shall preserve browser copy-link, modified-click, middle-click, and explicit new-tab behavior.
|
||||
- **AC-UI-TASK-NAVIGATION-001.4:** Task navigation shall respect existing unsaved-change guards. Cancelled navigation shall keep the current task and disclosure intact. Completed same-tab dependency navigation shall dismiss its disclosure.
|
||||
- **AC-UI-TASK-NAVIGATION-001.5:** Both dependency directions shall support the same destination behavior in the desktop popover and touch drawer. Touch rows shall have at least 44px hit targets; fine-pointer rows shall retain compact sizing.
|
||||
- **AC-UI-TASK-NAVIGATION-001.6:** Browser Back shall return to the preceding task after an ordinary task-link navigation without a document reload.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Task selection policy, task authorization, missing-task recovery, Office routes,
|
||||
external links, plugin-authored URLs, and navigation to task listings retain
|
||||
existing behavior. This contract does not change backend task APIs.
|
||||
|
||||
## Implementation plans
|
||||
|
||||
- [Task link navigation](../../../plans/task-link-navigation/plan.md)
|
||||
@@ -0,0 +1,120 @@
|
||||
---
|
||||
status: current
|
||||
system: ui
|
||||
requirements:
|
||||
- REQ-UI-TASK-NAVIGATION-001
|
||||
---
|
||||
|
||||
# Task navigation system design
|
||||
|
||||
## Purpose and boundaries
|
||||
|
||||
This design extends existing URL and client-navigation primitives. It does not
|
||||
introduce a router or route catalog. UI owns the reusable interaction; task
|
||||
identity and dependency semantics remain in the task system.
|
||||
|
||||
## Requirement mapping
|
||||
|
||||
| Requirement | Design sections |
|
||||
| --- | --- |
|
||||
| REQ-UI-TASK-NAVIGATION-001 | URL authority, Link authority, Enforcement, Responsive behavior |
|
||||
|
||||
## URL authority
|
||||
|
||||
`apps/web/lib/links.ts::linkToTask` remains the only first-party workbench URL
|
||||
builder. Keep the current `(taskId, layout?)` call shape compatible. Add an
|
||||
options overload containing `layout?`, `sessionId?`, and `searchParams?`
|
||||
(`URLSearchParams`) for current canvas and inbox contexts. Clone caller-owned
|
||||
parameters; explicit layout/sessionId values replace matching keys. Preserve
|
||||
other keys and repeated values. Encode the raw ID as one path segment exactly
|
||||
once. Callers pass raw IDs, never pre-encoded IDs. Test both overloads.
|
||||
|
||||
Route matching can still recognize `/t/` and `/tasks/`. Compatibility redirects
|
||||
use the builder for their destination. `/tasks` listings, `/office/tasks/`,
|
||||
API paths, external URLs, and stored user-authored content are separate contracts.
|
||||
Do not globally rewrite strings or historical content.
|
||||
|
||||
## Link authority
|
||||
|
||||
Add `components/routing/task-link.tsx::TaskLink`, a thin wrapper around existing
|
||||
`components/routing/app-link.tsx`. Accept `taskId`, builder options, normal
|
||||
anchor presentation props, and `onNavigated`; omit `href` from public props.
|
||||
The wrapper calls `linkToTask`, forwards its ref, and owns no task/store state.
|
||||
Existing compliant `Link href={linkToTask(id)}` consumers may remain; new direct
|
||||
task anchors should prefer TaskLink. Programmatic navigation continues through
|
||||
`useRouter().push(linkToTask(...))`.
|
||||
|
||||
Extend AppLink with an optional `onNavigated` callback called inside the
|
||||
existing successful `pushNavigationState` callback, after the location event.
|
||||
It must not run for cancelled guards, prevented clicks, external navigation,
|
||||
modified clicks, or alternate targets. TaskLink delegates all click decisions
|
||||
to AppLink. Do not duplicate the router's guard or browser-modifier logic.
|
||||
|
||||
DependencyRow uses TaskLink. The chip supplies a stable close callback through
|
||||
DependencyLists, invoked only after committed same-tab navigation. Keep browser
|
||||
Back semantics and anchor accessibility. Missing-task and access errors continue
|
||||
through the existing destination route.
|
||||
|
||||
## Enforcement
|
||||
|
||||
Add a local ESLint rule and register it as an error on production TS/TSX in
|
||||
`apps/web/eslint.config.mjs`. Test it using the existing local-rule pattern.
|
||||
|
||||
Reject hand-built root-relative task-detail URL expressions outside links.ts:
|
||||
string literals with a nonempty task segment, interpolated templates, and string
|
||||
concatenation beginning `/t/` or `/tasks/`. Permit bare prefix constants used
|
||||
for route recognition; exclude tests, E2E, generated files, comments, API and
|
||||
Office paths. Replace constructed comparison URLs with linkToTask too.
|
||||
|
||||
Also reject native JSX anchors whose href resolves to a task route or a
|
||||
linkToTask call, and window.location assign/replace/href writes with such a
|
||||
value. Resolve import aliases and simple local const aliases. Report a clear
|
||||
replacement suggestion. Rule fixtures cover direct, aliased, concatenated,
|
||||
and conditional forms plus benign route matching and API paths.
|
||||
|
||||
This is bounded static enforcement, not whole-program URL analysis. Unknown
|
||||
runtime strings remain outside its proof. Do not claim all possible dynamic
|
||||
links are statically verified. Add a wiring test that evaluates the real ESLint
|
||||
configuration on a synthetic new production component so new directories are
|
||||
covered. Start at zero violations by migrating current builders in the same
|
||||
work order; no growing baseline or broad per-directory exemptions.
|
||||
|
||||
## Responsive behavior
|
||||
|
||||
Keep the current dependency popover on fine pointers and useTouchDrawer on
|
||||
coarse pointers. Reuse shared Drawer safe-area handling and the single internal
|
||||
list scroller. The nearest mobile navigation exemplar is
|
||||
`components/task/mobile/session-task-switcher-sheet.tsx`; the dependency chip
|
||||
already uses the appropriate brief-choice drawer. Task selection completes the
|
||||
choice and goes to the workbench, so successful navigation dismisses the drawer.
|
||||
Keep its header and close action, constrain long lists to the viewport, and
|
||||
apply 44px row sizing only on touch surfaces. No new persisted state or copy.
|
||||
|
||||
## Validation
|
||||
|
||||
Component tests prove canonical hrefs, click interception, guard cancellation,
|
||||
callback timing, modifier handling, and encoding. Desktop and Pixel 5 E2E
|
||||
follow links in both dependency directions, verify target task content, dismiss
|
||||
the disclosure, and exercise Back. Set a unique window property before clicking
|
||||
and assert it survives; also observe main-frame document requests. A correct URL
|
||||
alone is not evidence against reload. Check touch targets and drawer containment.
|
||||
|
||||
## Related contracts
|
||||
|
||||
[Mobile task navigation](../requirements/mobile-task-navigation.md) owns broader
|
||||
task actions and mobile board navigation. This contract adds shared task-link
|
||||
transport and URL construction across desktop and mobile consumers.
|
||||
|
||||
## Related decisions and alternatives
|
||||
|
||||
[Navigation manifest boundaries](../../../decisions/2026-08-04-navigation-manifest-boundaries.md)
|
||||
retains top-level destination ownership.
|
||||
[Settings save coordination](../../../decisions/0046-settings-route-save-coordinator.md)
|
||||
retains dirty-navigation authority. No new route ownership registry is needed.
|
||||
A URL helper alone cannot prevent raw anchors from reloading. A component alone
|
||||
cannot cover imperative redirects. Reusing both existing authorities with a
|
||||
thin wrapper and lint enforcement addresses these distinct failure modes.
|
||||
|
||||
## Implementation plans
|
||||
|
||||
- [Task link navigation](../../../plans/task-link-navigation/plan.md)
|
||||
Reference in New Issue
Block a user