dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(web): constrain ci checks popover height (#1441)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-06-18 22:28:56 +01:00
committed by GitHub
parent f100fc97a5
commit ffda4638e9
7 changed files with 201 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/orchestrator/prompt_idle_session_test.go
+88
View File
@@ -5,10 +5,14 @@ import (
"errors"
"fmt"
"strings"
"sync/atomic"
"testing"
"time"
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
"github.com/kandev/kandev/internal/orchestrator/executor"
"github.com/kandev/kandev/internal/orchestrator/queue"
"github.com/kandev/kandev/internal/orchestrator/scheduler"
"github.com/kandev/kandev/internal/task/models"
v1 "github.com/kandev/kandev/pkg/api/v1"
)
@@ -400,3 +404,87 @@ func TestEnsureSessionRunning_WaitsForPromptReadyWhenExecutionExists(t *testing.
t.Fatal("ensureSessionRunning did not return after prompt readiness")
}
}
func TestPromptTask_LazyResumeExecutionNotFoundFallsBackToFreshLaunch(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedTaskAndSession(t, repo, "task1", "session1", models.TaskSessionStateIdle)
session, err := repo.GetTaskSession(ctx, "session1")
if err != nil {
t.Fatalf("failed to load session: %v", err)
}
session.AgentExecutionID = "exec-before-restart"
session.AgentProfileID = "profile1"
if err := repo.UpdateTaskSession(ctx, session); err != nil {
t.Fatalf("failed to update session: %v", err)
}
seedExecutorRunning(t, repo, session.ID, session.TaskID, "exec-before-restart")
var launchCalls atomic.Int32
launchPrompts := make(chan string, 2)
agentMgr := &mockAgentManager{
repoForExecutionLookup: repo,
promptErr: lifecycle.ErrExecutionNotFound,
isAgentRunningFn: func(_ context.Context, _ string) bool {
return launchCalls.Load() > 0
},
isAgentReadyFn: func(_ context.Context, _ string) bool {
return launchCalls.Load() > 0
},
launchAgentFunc: func(_ context.Context, req *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
call := launchCalls.Add(1)
launchPrompts <- req.TaskDescription
if call == 1 {
go func(sessionID string) {
tick := time.NewTicker(5 * time.Millisecond)
defer tick.Stop()
timeout := time.After(5 * time.Second)
for {
select {
case <-tick.C:
sess, err := repo.GetTaskSession(context.Background(), sessionID)
if err == nil && sess != nil && sess.State == models.TaskSessionStateStarting {
sess.State = models.TaskSessionStateWaitingForInput
sess.UpdatedAt = time.Now().UTC()
_ = repo.UpdateTaskSession(context.Background(), sess)
return
}
case <-timeout:
return
}
}
}(req.SessionID)
}
return &executor.LaunchAgentResponse{AgentExecutionID: fmt.Sprintf("exec-resumed-%d", call)}, nil
},
}
taskRepo := newMockTaskRepo()
taskRepo.tasks["task1"] = &v1.Task{
ID: "task1",
Title: "Test Task",
State: v1.TaskStateInProgress,
}
svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
exec := executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
svc.executor = exec
svc.scheduler = scheduler.NewScheduler(queue.NewTaskQueue(100), exec, taskRepo, testLogger(), scheduler.DefaultSchedulerConfig())
prompt := "follow-up after restart"
if _, err := svc.PromptTask(ctx, "task1", "session1", prompt, "", false, nil, false); err != nil {
t.Fatalf("expected fresh-launch fallback to recover missing execution, got: %v", err)
}
if got := launchCalls.Load(); got != 2 {
t.Fatalf("expected resume launch plus fresh fallback launch, got %d launches", got)
}
<-launchPrompts // resume prompt; empty for the lazy resume path.
freshPrompt := <-launchPrompts
if !strings.Contains(freshPrompt, prompt) {
t.Fatalf("fresh launch prompt %q does not contain original prompt %q", freshPrompt, prompt)
}
if len(agentMgr.capturedPromptCalls) != 1 {
t.Fatalf("expected one failed PromptAgent attempt before fallback, got %d", len(agentMgr.capturedPromptCalls))
}
}
apps/backend/internal/orchestrator/task_operations.go
+12
View File
@@ -2095,6 +2095,8 @@ func (s *Service) PromptTask(ctx context.Context, taskID, sessionID string, prom
// Ensure the agent process is actually running. After a lazy backend restart,
// the session may be in WAITING_FOR_INPUT but no agent process exists yet.
_, hadExecutionBeforeEnsure := s.executor.GetExecutionBySession(sessionID)
resumedForPrompt := !hadExecutionBeforeEnsure
if err := s.ensureSessionRunning(ctx, sessionID, session); err != nil {
return nil, fmt.Errorf("failed to ensure session is running: %w", err)
}
@@ -2139,6 +2141,16 @@ func (s *Service) PromptTask(ctx context.Context, taskID, sessionID string, prom
promptCtx := context.WithoutCancel(ctx)
result, err := s.executor.Prompt(promptCtx, taskID, sessionID, effectivePrompt, attachments, dispatchOnly, session)
if err != nil {
if resumedForPrompt && errors.Is(err, executor.ErrExecutionNotFound) {
s.logger.Warn("prompt after lazy resume hit missing execution; falling back to fresh launch",
zap.String("task_id", taskID),
zap.String("session_id", sessionID))
if freshErr := s.fallbackFreshLaunchOnMissingExecution(ctx, taskID, sessionID, prompt, planMode, nil, attachments); freshErr == nil {
return &PromptResult{}, nil
} else {
err = freshErr
}
}
return nil, s.handlePromptError(ctx, taskID, sessionID, previousSessionState, err)
}
return &PromptResult{
apps/web/components/github/pr-ci-popover.tsx
+3
View File
@@ -36,6 +36,9 @@ type CountsView = {
const CHECK_GROUP_ORDER: CheckBucket[] = ["passed", "in_progress", "failed"];
export const PR_CI_DESKTOP_POPOVER_SCROLL_CLASS =
"max-h-[min(28rem,calc(100vh-8rem))] overflow-y-auto overscroll-contain";
function countForBucket(counts: CountsView, kind: CheckBucket): number {
if (kind === "passed") return counts.passed;
if (kind === "in_progress") return counts.inProgress;
apps/web/components/github/pr-status-chip.test.tsx
+22
View File
@@ -5,6 +5,7 @@ import { TooltipProvider } from "@kandev/ui/tooltip";
import { StateProvider } from "@/components/state-provider";
import { ToastProvider } from "@/components/toast-provider";
import { PRStatusChip, aggregateChipStatus } from "./pr-status-chip";
import { PR_CI_DESKTOP_POPOVER_SCROLL_CLASS } from "./pr-ci-popover";
import type { AppState } from "@/lib/state/store";
import type { TaskPR } from "@/lib/types/github";
@@ -90,6 +91,17 @@ function multiState(prs: TaskPR[]): Partial<AppState> {
return { taskPRs: { byTaskId: { "task-1": prs } } };
}
async function expectDesktopHoverPopoverConstrained() {
fireEvent.pointerEnter(screen.getByTestId(CHIP_TESTID));
const inner = await screen.findByTestId("pr-topbar-popover-inner");
const content = inner.closest<HTMLElement>(".overflow-y-auto");
expect(content).not.toBeNull();
const classNames = content!.className.split(/\s+/);
expect(classNames).toEqual(
expect.arrayContaining(PR_CI_DESKTOP_POPOVER_SCROLL_CLASS.split(/\s+/)),
);
}
describe("PRStatusChip", () => {
it("returns null when the task has no PR", () => {
renderWithStore(undefined, <PRStatusChip taskId="missing" />);
@@ -127,6 +139,11 @@ describe("PRStatusChip", () => {
expect(document.querySelector(DRAWER_SELECTOR)).toBeNull();
});
it("constrains the hover popover to the available viewport height", async () => {
renderWithStore(seededState, <PRStatusChip taskId="task-1" />);
await expectDesktopHoverPopoverConstrained();
});
it("exposes the canonical data attributes that desktop tests rely on", () => {
renderWithStore(seededState, <PRStatusChip taskId="task-1" />);
const chip = screen.getByTestId(CHIP_TESTID);
@@ -239,6 +256,11 @@ describe("PRStatusChip — multiple PRs", () => {
expect(chip.getAttribute(ATTR_STATUS)).toBe("failed");
});
it("constrains the aggregate hover popover to the available viewport height", async () => {
renderWithStore(multiState(TWO_OPEN), <PRStatusChip taskId="task-1" />);
await expectDesktopHoverPopoverConstrained();
});
it("stays visible while at least one PR is still open", () => {
renderWithStore(
multiState([makePR({ id: "a", state: "merged" }), makePR({ id: "b", pr_number: 2 })]),
apps/web/components/github/pr-status-chip.tsx
+3 -3
View File
@@ -21,7 +21,7 @@ import {
import { Button } from "@kandev/ui/button";
import { useTaskPR } from "@/hooks/domains/github/use-task-pr";
import { usePRFeedbackBackgroundSync } from "@/hooks/domains/github/use-pr-ci-popover";
import { PRCIPopover } from "@/components/github/pr-ci-popover";
import { PR_CI_DESKTOP_POPOVER_SCROLL_CLASS, PRCIPopover } from "@/components/github/pr-ci-popover";
import { MultiPRCIPopover } from "@/components/github/multi-pr-ci-popover";
import {
isPRAwaitingReview,
@@ -176,7 +176,7 @@ function PRStatusChipHoverCard({ pr }: { pr: TaskPR }) {
side="top"
align="start"
sideOffset={8}
className="w-80 p-2.5"
className={`w-80 p-2.5 ${PR_CI_DESKTOP_POPOVER_SCROLL_CLASS}`}
onPointerDownOutside={onPointerDownOutside}
>
<PRCIPopover pr={pr} enabled={true} />
@@ -233,7 +233,7 @@ function PRStatusChipMultiHoverCard({ prs }: { prs: TaskPR[] }) {
side="top"
align="start"
sideOffset={8}
className="w-96 p-2.5"
className={`w-96 p-2.5 ${PR_CI_DESKTOP_POPOVER_SCROLL_CLASS}`}
onPointerDownOutside={onPointerDownOutside}
>
<MultiPRCIPopover prs={prs} enabled={true} />
apps/web/components/github/pr-topbar-button.tsx
+3 -3
View File
@@ -29,7 +29,7 @@ import {
isPRReadyToMerge,
} from "@/components/github/pr-task-icon";
import { prTaskKey } from "@/components/github/pr-detail-panel";
import { PRCIPopover } from "@/components/github/pr-ci-popover";
import { PR_CI_DESKTOP_POPOVER_SCROLL_CLASS, PRCIPopover } from "@/components/github/pr-ci-popover";
import { MultiPRCIPopover } from "@/components/github/multi-pr-ci-popover";
import { useAppStore } from "@/components/state-provider";
import type { TaskPR } from "@/lib/types/github";
@@ -186,7 +186,7 @@ function PRSingleButton({ pr }: { pr: TaskPR }) {
data-testid="pr-topbar-popover"
align="end"
sideOffset={4}
className="w-80"
className={`w-80 ${PR_CI_DESKTOP_POPOVER_SCROLL_CLASS}`}
onMouseEnter={handleEnter}
onMouseLeave={handleLeave}
onOpenAutoFocus={(e) => e.preventDefault()}
@@ -258,7 +258,7 @@ function PRMultiButton({ prs }: { prs: TaskPR[] }) {
data-testid="pr-topbar-popover"
align="end"
sideOffset={4}
className="w-96"
className={`w-96 ${PR_CI_DESKTOP_POPOVER_SCROLL_CLASS}`}
onMouseEnter={handleEnter}
onMouseLeave={handleLeave}
onOpenAutoFocus={(e) => e.preventDefault()}
apps/web/e2e/tests/pr/pr-topbar-popover.spec.ts
+70
View File
@@ -2,6 +2,7 @@ import { test, expect } from "../../fixtures/test-base";
import { KanbanPage } from "../../pages/kanban-page";
import { SessionPage } from "../../pages/session-page";
import type { ApiClient } from "../../helpers/api-client";
import type { Locator, Page } from "@playwright/test";
const OWNER = "acme";
const REPO = "demo";
@@ -90,6 +91,37 @@ async function associatePR(
});
}
function manyRunningChecks(count: number) {
return Array.from({ length: count }, (_, index) => ({
name: `E2E Shard ${index + 1}/${count} / run`,
status: "in_progress",
html_url: `https://example.com/checks/${index + 1}`,
}));
}
async function expectScrollablePopoverWithinViewport(testPage: Page, locator: Locator) {
const metrics = await locator.evaluate((node) => {
const content = node.classList.contains("overflow-y-auto")
? node
: node.closest(".overflow-y-auto");
const rect = content?.getBoundingClientRect();
return {
top: rect?.top ?? -1,
bottom: rect?.bottom ?? -1,
clientHeight: content?.clientHeight ?? 0,
scrollHeight: content?.scrollHeight ?? 0,
overflowY: content ? getComputedStyle(content).overflowY : "",
overscrollBehavior: content ? getComputedStyle(content).overscrollBehavior : "",
};
});
const viewportHeight = testPage.viewportSize()?.height ?? 0;
expect(metrics.top).toBeGreaterThanOrEqual(0);
expect(metrics.bottom).toBeLessThanOrEqual(viewportHeight);
expect(metrics.overflowY).toBe("auto");
expect(metrics.overscrollBehavior).toBe("contain");
expect(metrics.scrollHeight).toBeGreaterThan(metrics.clientHeight);
}
async function openTaskAndWait(
testPage: import("@playwright/test").Page,
apiClient: ApiClient,
@@ -211,6 +243,44 @@ test.describe("PR top-bar CI popover", () => {
await expect(session.prWorkflowRow("E2E")).toContainText("4/5 passed");
});
test("desktop CI popovers scroll instead of overflowing the viewport", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(120_000);
const title = "Many Running Checks";
const seed = await seedTask(
apiClient,
seedData.workspaceId,
seedData.agentProfileId,
seedData.repositoryId,
title,
);
await associatePR(apiClient, seed.taskId, {
checks_state: "pending",
checks_total: 30,
checks_passing: 0,
});
await apiClient.mockGitHubSeedPRFeedback({
owner: OWNER,
repo: REPO,
pr_number: PR_NUMBER,
checks: manyRunningChecks(30),
});
const session = await openTaskAndWait(testPage, apiClient, seed, title);
await expect(session.prStatusChip()).toBeVisible();
await session.prStatusChip().hover();
const popoverInner = testPage.getByTestId("pr-topbar-popover-inner");
await expect(popoverInner).toBeVisible({ timeout: 10_000 });
await expectScrollablePopoverWithinViewport(testPage, popoverInner);
await testPage.mouse.move(0, 0);
await session.hoverPRTopbar();
await expectScrollablePopoverWithinViewport(testPage, session.prTopbarPopover());
});
test("review row shows N / M required + unresolved comments count", async ({
testPage,
apiClient,