fix(web): prevent dockview layout from not filling viewport after session switch (#471)
This commit is contained in:
@@ -47,6 +47,16 @@ func getPRFeedback(ctx context.Context, c Client, owner, repo string, number int
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Ensure non-nil slices so JSON serialization produces [] instead of null.
|
||||
if reviews == nil {
|
||||
reviews = []PRReview{}
|
||||
}
|
||||
if comments == nil {
|
||||
comments = []PRComment{}
|
||||
}
|
||||
if checks == nil {
|
||||
checks = []CheckRun{}
|
||||
}
|
||||
hasIssues := hasFailingChecks(checks) || hasChangesRequested(reviews)
|
||||
return &PRFeedback{
|
||||
PR: pr,
|
||||
|
||||
@@ -100,11 +100,12 @@ type PRPanelMetrics = {
|
||||
|
||||
function computeLiveReviewState(feedback: PRFeedback, fallbackState: TaskPR["review_state"]) {
|
||||
const requestedReviewers = feedback.pr.requested_reviewers ?? [];
|
||||
if (feedback.reviews.length === 0) {
|
||||
const reviews = feedback.reviews ?? [];
|
||||
if (reviews.length === 0) {
|
||||
return requestedReviewers.length > 0 ? "pending" : fallbackState || "";
|
||||
}
|
||||
const latestByAuthor = new Map<string, { state: string; createdAt: number }>();
|
||||
for (const review of feedback.reviews) {
|
||||
for (const review of reviews) {
|
||||
const current = latestByAuthor.get(review.author);
|
||||
const createdAt = new Date(review.created_at).getTime();
|
||||
if (!current || createdAt > current.createdAt) {
|
||||
@@ -133,9 +134,9 @@ function derivePanelMetrics(taskPR: TaskPR, feedback: PRFeedback | null): PRPane
|
||||
}
|
||||
const pendingReviewCount = feedback.pr.requested_reviewers?.length ?? taskPR.pending_review_count;
|
||||
return {
|
||||
reviewCount: feedback.reviews.length,
|
||||
reviewCount: (feedback.reviews ?? []).length,
|
||||
pendingReviewCount,
|
||||
commentCount: feedback.comments.length,
|
||||
commentCount: (feedback.comments ?? []).length,
|
||||
reviewState: computeLiveReviewState(feedback, taskPR.review_state),
|
||||
};
|
||||
}
|
||||
@@ -168,7 +169,7 @@ function ApproveButton({
|
||||
const liveState = feedback?.pr.state ?? taskPR.state;
|
||||
if (liveState !== "open") return null;
|
||||
|
||||
const alreadyApproved = feedback?.reviews.some(
|
||||
const alreadyApproved = feedback?.reviews?.some(
|
||||
(r) => r.state === "APPROVED" && r.author === feedback.pr.author_login,
|
||||
);
|
||||
if (alreadyApproved) return null;
|
||||
@@ -262,7 +263,7 @@ function PRDetailContent({ taskPR, sessionId }: { taskPR: TaskPR; sessionId: str
|
||||
<>
|
||||
<DescriptionSection body={feedback.pr.body ?? ""} />
|
||||
<ReviewsSection
|
||||
reviews={feedback.reviews}
|
||||
reviews={feedback.reviews ?? []}
|
||||
requestedReviewers={feedback.pr.requested_reviewers ?? []}
|
||||
prUrl={taskPR.pr_url}
|
||||
reviewState={metrics.reviewState}
|
||||
@@ -270,11 +271,11 @@ function PRDetailContent({ taskPR, sessionId }: { taskPR: TaskPR; sessionId: str
|
||||
onAddAsContext={(msg) => addAsContext("review", msg)}
|
||||
/>
|
||||
<ChecksSection
|
||||
checks={feedback.checks}
|
||||
checks={feedback.checks ?? []}
|
||||
onAddAsContext={(msg) => addAsContext("check", msg)}
|
||||
/>
|
||||
<CommentsSection
|
||||
comments={feedback.comments}
|
||||
comments={feedback.comments ?? []}
|
||||
prUrl={taskPR.pr_url}
|
||||
onAddAsContext={(msg) => addAsContext("comment", msg)}
|
||||
/>
|
||||
|
||||
@@ -496,6 +496,36 @@ export class ApiClient {
|
||||
await this.request("POST", "/api/v1/github/mock/task-prs", data);
|
||||
}
|
||||
|
||||
// --- GitHub Review Watch ---
|
||||
|
||||
async createReviewWatch(
|
||||
workspaceId: string,
|
||||
workflowId: string,
|
||||
workflowStepId: string,
|
||||
agentProfileId: string,
|
||||
opts?: {
|
||||
repos?: Array<{ owner: string; name: string }>;
|
||||
prompt?: string;
|
||||
review_scope?: string;
|
||||
poll_interval_seconds?: number;
|
||||
},
|
||||
): Promise<{ id: string }> {
|
||||
return this.request("POST", "/api/v1/github/watches/review", {
|
||||
workspace_id: workspaceId,
|
||||
workflow_id: workflowId,
|
||||
workflow_step_id: workflowStepId,
|
||||
agent_profile_id: agentProfileId,
|
||||
repos: opts?.repos ?? [],
|
||||
prompt: opts?.prompt ?? "",
|
||||
review_scope: opts?.review_scope ?? "user_and_teams",
|
||||
poll_interval_seconds: opts?.poll_interval_seconds ?? 300,
|
||||
});
|
||||
}
|
||||
|
||||
async triggerReviewWatch(watchId: string): Promise<{ new_prs: number }> {
|
||||
return this.request("POST", `/api/v1/github/watches/review/${watchId}/trigger`);
|
||||
}
|
||||
|
||||
async mockGitHubGetStatus(): Promise<{
|
||||
authenticated: boolean;
|
||||
username: string;
|
||||
|
||||
@@ -340,13 +340,45 @@ export class SessionPage {
|
||||
|
||||
/**
|
||||
* Assert the layout is in the default (non-maximized) state:
|
||||
* chat, terminal, files, and sidebar are all visible.
|
||||
* chat, terminal, files, and sidebar are all visible, and layout fills the viewport.
|
||||
*/
|
||||
async expectDefaultLayout(): Promise<void> {
|
||||
await expect(this.chat).toBeVisible({ timeout: 10_000 });
|
||||
await expect(this.terminal).toBeVisible({ timeout: 10_000 });
|
||||
await expect(this.files).toBeVisible({ timeout: 10_000 });
|
||||
await expect(this.sidebar).toBeVisible();
|
||||
await this.expectNoLayoutGap();
|
||||
}
|
||||
|
||||
/**
|
||||
* Assert the dockview layout columns fill the container with no large empty gap.
|
||||
* Catches bugs where columns don't expand after api.fromJSON() + setConstraints
|
||||
* (e.g. missing api.layout() call).
|
||||
*/
|
||||
async expectNoLayoutGap(maxGapPx = 20): Promise<void> {
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
return this.page.evaluate((maxGap: number) => {
|
||||
const dv = document.querySelector(".dv-dockview");
|
||||
if (!dv) return false;
|
||||
const dvRect = dv.getBoundingClientRect();
|
||||
// Find the rightmost edge among all top-level column views
|
||||
const views = dv.querySelectorAll(
|
||||
".dv-split-view-container.dv-horizontal > .dv-view-container > .dv-view",
|
||||
);
|
||||
if (views.length === 0) return false;
|
||||
let maxRight = 0;
|
||||
for (const v of views) {
|
||||
const r = v.getBoundingClientRect();
|
||||
if (r.width > 0) maxRight = Math.max(maxRight, r.right);
|
||||
}
|
||||
return dvRect.right - maxRight <= maxGap;
|
||||
}, maxGapPx);
|
||||
},
|
||||
{ timeout: 5_000, message: "Layout has an empty gap on the right side (squished layout)" },
|
||||
)
|
||||
.toBe(true);
|
||||
}
|
||||
|
||||
/** Git operation error message in chat (shown when a git operation fails). */
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
import { test, expect } from "../fixtures/test-base";
|
||||
import { KanbanPage } from "../pages/kanban-page";
|
||||
import { SessionPage } from "../pages/session-page";
|
||||
|
||||
test.describe("PR watcher dockview layout stability", () => {
|
||||
/**
|
||||
* Verifies the dockview layout stays correct when switching between
|
||||
* PR tasks auto-created by the review watcher.
|
||||
*
|
||||
* Flow:
|
||||
* 1. PR watcher detects 3 PRs and auto-creates tasks in the background
|
||||
* 2. Open PR task 1 from the kanban homepage
|
||||
* 3. Toggle plan mode (adds plan panel)
|
||||
* 4. Switch to PR task 2 via sidebar → should have default layout
|
||||
* 5. Switch to PR task 3 via sidebar → should have default layout
|
||||
*
|
||||
* Setup:
|
||||
* Review step (no auto_start — tasks sit idle after creation)
|
||||
* Review watch triggers initial poll → 3 tasks created (not started)
|
||||
*/
|
||||
test("layout remains correct when switching between PR watcher tasks with plan mode", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
|
||||
// --- Seed workflow ---
|
||||
const workflow = await apiClient.createWorkflow(
|
||||
seedData.workspaceId,
|
||||
"PR Watcher Layout Workflow",
|
||||
);
|
||||
|
||||
const reviewStep = await apiClient.createWorkflowStep(workflow.id, "Review", 0);
|
||||
|
||||
await apiClient.saveUserSettings({
|
||||
workspace_id: seedData.workspaceId,
|
||||
workflow_filter_id: workflow.id,
|
||||
enable_preview_on_click: false,
|
||||
});
|
||||
|
||||
// --- Seed mock GitHub data ---
|
||||
await apiClient.mockGitHubReset();
|
||||
await apiClient.mockGitHubSetUser("test-user");
|
||||
|
||||
// Add 3 PRs with requested_reviewers so the review watcher picks them up
|
||||
await apiClient.mockGitHubAddPRs([
|
||||
{
|
||||
number: 101,
|
||||
title: "Fix auth bug",
|
||||
state: "open",
|
||||
head_branch: "fix/auth",
|
||||
base_branch: "main",
|
||||
author_login: "alice",
|
||||
repo_owner: "testorg",
|
||||
repo_name: "testrepo",
|
||||
additions: 10,
|
||||
deletions: 2,
|
||||
requested_reviewers: [{ login: "test-user", type: "User" }],
|
||||
},
|
||||
{
|
||||
number: 202,
|
||||
title: "Add dashboard",
|
||||
state: "open",
|
||||
head_branch: "feat/dashboard",
|
||||
base_branch: "main",
|
||||
author_login: "bob",
|
||||
repo_owner: "testorg",
|
||||
repo_name: "testrepo",
|
||||
additions: 80,
|
||||
deletions: 5,
|
||||
requested_reviewers: [{ login: "test-user", type: "User" }],
|
||||
},
|
||||
{
|
||||
number: 303,
|
||||
title: "Update docs",
|
||||
state: "open",
|
||||
head_branch: "docs/update",
|
||||
base_branch: "main",
|
||||
author_login: "charlie",
|
||||
repo_owner: "testorg",
|
||||
repo_name: "testrepo",
|
||||
additions: 30,
|
||||
deletions: 10,
|
||||
requested_reviewers: [{ login: "test-user", type: "User" }],
|
||||
},
|
||||
]);
|
||||
|
||||
// Navigate to kanban BEFORE creating the review watch so the WS is
|
||||
// subscribed when task.created/task.updated events fire.
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
|
||||
// --- Create review watch (triggers initial poll → auto-creates 3 tasks) ---
|
||||
await apiClient.createReviewWatch(
|
||||
seedData.workspaceId,
|
||||
workflow.id,
|
||||
reviewStep.id,
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
repos: [{ owner: "testorg", name: "testrepo" }],
|
||||
prompt: "Review {{pr.link}}",
|
||||
},
|
||||
);
|
||||
|
||||
// --- Wait for all 3 PR tasks to appear in the Review column ---
|
||||
const prTask1Title = "PR #101: Fix auth bug";
|
||||
const prTask2Title = "PR #202: Add dashboard";
|
||||
const prTask3Title = "PR #303: Update docs";
|
||||
|
||||
await expect(kanban.taskCardInColumn(prTask1Title, reviewStep.id)).toBeVisible({
|
||||
timeout: 60_000,
|
||||
});
|
||||
await expect(kanban.taskCardInColumn(prTask2Title, reviewStep.id)).toBeVisible({
|
||||
timeout: 60_000,
|
||||
});
|
||||
await expect(kanban.taskCardInColumn(prTask3Title, reviewStep.id)).toBeVisible({
|
||||
timeout: 60_000,
|
||||
});
|
||||
|
||||
// --- Click PR task 1 to enter session view ---
|
||||
await kanban.taskCardInColumn(prTask1Title, reviewStep.id).click();
|
||||
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
|
||||
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
|
||||
// Verify initial default layout for task 1
|
||||
await expect(session.chat).toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.sidebar).toBeVisible();
|
||||
|
||||
// --- Toggle plan mode on task 1 ---
|
||||
await session.togglePlanMode();
|
||||
await expect(session.planPanel).toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.chat).toBeVisible();
|
||||
await expect(session.sidebar).toBeVisible();
|
||||
|
||||
// --- Switch to PR task 2 via sidebar ---
|
||||
await session.taskInSidebar(prTask2Title).click();
|
||||
await expect(testPage).toHaveURL(/\/s\//, { timeout: 30_000 });
|
||||
|
||||
// Task 2: full layout intact — sidebar, chat accessible, changes panel visible,
|
||||
// plan panel must NOT leak from task 1, and layout fills the viewport
|
||||
await expect(session.sidebar).toBeVisible({ timeout: 15_000 });
|
||||
await session.expectNoLayoutGap();
|
||||
await expect(session.planPanel).not.toBeVisible({ timeout: 5_000 });
|
||||
// Changes panel verifies the right column rendered
|
||||
await session.clickTab("Changes");
|
||||
await expect(session.changes).toBeVisible({ timeout: 10_000 });
|
||||
// Chat may be behind a PR detail tab — click Agent tab to verify it exists
|
||||
await session.clickTab("Agent");
|
||||
await expect(session.chat).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// --- Switch to PR task 3 via sidebar ---
|
||||
await session.taskInSidebar(prTask3Title).click({ timeout: 15_000 });
|
||||
await expect(testPage).toHaveURL(/\/s\//, { timeout: 30_000 });
|
||||
|
||||
// Task 3: same layout checks
|
||||
await expect(session.sidebar).toBeVisible({ timeout: 15_000 });
|
||||
await session.expectNoLayoutGap();
|
||||
await expect(session.planPanel).not.toBeVisible({ timeout: 5_000 });
|
||||
await session.clickTab("Changes");
|
||||
await expect(session.changes).toBeVisible({ timeout: 10_000 });
|
||||
await session.clickTab("Agent");
|
||||
await expect(session.chat).toBeVisible({ timeout: 10_000 });
|
||||
});
|
||||
});
|
||||
@@ -531,6 +531,10 @@ function performBuildDefault(
|
||||
const { userDefaultLayout } = get();
|
||||
const intent = intentName ? resolveNamedIntent(intentName) : null;
|
||||
const freshPinned = new Map<string, number>();
|
||||
// Capture dimensions before layout change — api.width can become stale
|
||||
// after fromJSON inside applyLayout
|
||||
const safeWidth = api.width;
|
||||
const safeHeight = api.height;
|
||||
set({ isRestoringLayout: true, pinnedWidths: freshPinned });
|
||||
|
||||
const basePreset = intent?.preset as BuiltInPreset | undefined;
|
||||
@@ -557,6 +561,7 @@ function performBuildDefault(
|
||||
}
|
||||
|
||||
requestAnimationFrame(() => {
|
||||
api.layout(safeWidth, safeHeight);
|
||||
syncPinnedWidthsFromApi(api, set);
|
||||
set({ isRestoringLayout: false });
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user