diff --git a/apps/backend/internal/github/client_helpers.go b/apps/backend/internal/github/client_helpers.go index 2845b8cb1d..649e56bcd2 100644 --- a/apps/backend/internal/github/client_helpers.go +++ b/apps/backend/internal/github/client_helpers.go @@ -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, diff --git a/apps/web/components/github/pr-detail-panel.tsx b/apps/web/components/github/pr-detail-panel.tsx index f0762020f7..8119979faa 100644 --- a/apps/web/components/github/pr-detail-panel.tsx +++ b/apps/web/components/github/pr-detail-panel.tsx @@ -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(); - 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 <> addAsContext("review", msg)} /> addAsContext("check", msg)} /> addAsContext("comment", msg)} /> diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index cf35a0b973..b3dd1b3c90 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -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; diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index 87d13fa679..6d7c3fa01c 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -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 { 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 { + 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). */ diff --git a/apps/web/e2e/tests/pr-watcher-dockview-layout.spec.ts b/apps/web/e2e/tests/pr-watcher-dockview-layout.spec.ts new file mode 100644 index 0000000000..c6678c757b --- /dev/null +++ b/apps/web/e2e/tests/pr-watcher-dockview-layout.spec.ts @@ -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 }); + }); +}); diff --git a/apps/web/lib/state/dockview-store.ts b/apps/web/lib/state/dockview-store.ts index 9937e327d6..ff0ecc95d7 100644 --- a/apps/web/lib/state/dockview-store.ts +++ b/apps/web/lib/state/dockview-store.ts @@ -531,6 +531,10 @@ function performBuildDefault( const { userDefaultLayout } = get(); const intent = intentName ? resolveNamedIntent(intentName) : null; const freshPinned = new Map(); + // 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 }); });