From 0678d696c8f330b253cf94e53ff3d056b0bd44fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fiere?= Date: Mon, 10 Aug 2026 14:32:38 +0200 Subject: [PATCH] fix(improve-kandev): re-run bootstrap when the dialog reopens (#2484) --- .../components/improve-kandev-dialog.test.tsx | 22 +++- apps/web/components/improve-kandev-dialog.tsx | 13 +- apps/web/e2e/tests/improve-kandev.spec.ts | 77 ++++++++++++ .../plan.md | 116 ++++++++++++++++++ .../task-01-reopen-bootstrap-regression.md | 103 ++++++++++++++++ docs/specs/improve-kandev/spec.md | 7 ++ 6 files changed, 335 insertions(+), 3 deletions(-) create mode 100644 docs/plans/improve-kandev-dialog-reopen-bootstrap/plan.md create mode 100644 docs/plans/improve-kandev-dialog-reopen-bootstrap/task-01-reopen-bootstrap-regression.md diff --git a/apps/web/components/improve-kandev-dialog.test.tsx b/apps/web/components/improve-kandev-dialog.test.tsx index c9ef375db7..5740af5da0 100644 --- a/apps/web/components/improve-kandev-dialog.test.tsx +++ b/apps/web/components/improve-kandev-dialog.test.tsx @@ -1,7 +1,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { ImproveKandevDialog } from "./improve-kandev-dialog"; -import { IMPROVE_KANDEV_WORKSPACE_NAME } from "./improve-kandev-dialog-model"; +import { + IMPROVE_KANDEV_SKIP_INTRO_KEY, + IMPROVE_KANDEV_WORKSPACE_NAME, +} from "./improve-kandev-dialog-model"; import type { ImproveKandevBootstrapResponse } from "@/lib/api/domains/improve-kandev-api"; const ACTIVE_WORKSPACE = { id: "ws-active", name: "Active Workspace" }; @@ -121,6 +124,23 @@ describe("ImproveKandevDialog bootstrap workspace wiring", () => { ); }); + it("re-runs bootstrap when the dialog reopens with the dedicated workspace present (skip-intro)", async () => { + setStoreWorkspaces([ACTIVE_WORKSPACE, IMPROVE_WORKSPACE]); + window.localStorage.setItem(IMPROVE_KANDEV_SKIP_INTRO_KEY, "true"); + const { rerender } = render( + {}} workspaceId="ws-active" />, + ); + + await waitFor(() => expect(mocks.bootstrap).toHaveBeenCalledTimes(1)); + + // Close, then reopen — a re-open must re-run bootstrap instead of sitting + // at the idle "Preparing kandev repository" banner with submit blocked. + rerender( {}} workspaceId="ws-active" />); + rerender( {}} workspaceId="ws-active" />); + + await waitFor(() => expect(mocks.bootstrap).toHaveBeenCalledTimes(2)); + }); + it("shows the workspace-creation choice when the dedicated workspace is missing and defers bootstrap", async () => { setStoreWorkspaces([ACTIVE_WORKSPACE]); window.localStorage.setItem("kandev.improveKandev.skipIntro", "true"); diff --git a/apps/web/components/improve-kandev-dialog.tsx b/apps/web/components/improve-kandev-dialog.tsx index 58143c0f75..85123d01a9 100644 --- a/apps/web/components/improve-kandev-dialog.tsx +++ b/apps/web/components/improve-kandev-dialog.tsx @@ -62,11 +62,20 @@ export function ImproveKandevDialog(props: ImproveKandevDialogProps) { ); // When the dedicated workspace exists, bootstrap proceeds immediately. + // Keyed on `open` too: useResetOnClose clears the confirmation on close, and + // without this the effect only re-ran when improveWorkspaceMissing changed — + // a re-open with the workspace already known would sit forever at the idle + // "Preparing kandev repository" banner with submit blocked. + // + // The re-probe on every open is intentional: bootstrap is idempotent + // (backend reuses the existing workspace/workflow/repo) and re-checks the + // user's fork/write access, which can change between opens. Do not + // short-circuit it on a previously-successful result. useEffect(() => { - if (!improveWorkspaceMissing) { + if (open && !improveWorkspaceMissing) { setWorkspaceChoiceConfirmed(true); } - }, [improveWorkspaceMissing]); + }, [improveWorkspaceMissing, open]); useResetOnClose(open, { setMode, diff --git a/apps/web/e2e/tests/improve-kandev.spec.ts b/apps/web/e2e/tests/improve-kandev.spec.ts index 86330b367e..e3a08193bd 100644 --- a/apps/web/e2e/tests/improve-kandev.spec.ts +++ b/apps/web/e2e/tests/improve-kandev.spec.ts @@ -225,6 +225,83 @@ test.describe("Improve Kandev dialog", () => { .not.toContainEqual(expect.objectContaining({ title })); }); + test("a second bug can be filed after reopening the dialog in the dedicated workspace", async ({ + testPage, + apiClient, + seedData, + }) => { + // Seed the dedicated workspace under a temporary name (repositories and + // workflows cannot be created in the immutable workspace via the API), + // then rename it — same approach as the isolation test above. + const staging = await apiClient.createWorkspace("Improve Kandev Setup"); + const dedicatedWorkflow = await apiClient.createWorkflow( + staging.id, + "Improve Kandev", + "simple", + ); + const dedicatedRepo = await apiClient.createRepository( + staging.id, + seedData.repositoryPath, + "main", + ); + await apiClient.updateWorkspace(staging.id, { name: "Improve Kandev" }); + await apiClient.saveUserSettings({ agent_generated_task_titles: false }); + const dedicated = staging; + await mockImproveKandevApis(testPage, seedData, { + workspaceId: dedicated.id, + workflowId: dedicatedWorkflow.id, + issueWorkflowId: dedicatedWorkflow.id, + repositoryId: dedicatedRepo.id, + }); + + await testPage.goto("/"); + // Activate the dedicated workspace — the state a user is in after their + // first contribution navigates them there. + await testPage.getByTestId("sidebar-workspace-trigger").click(); + await testPage.getByTestId(`sidebar-workspace-item-${dedicated.id}`).click(); + await expect(testPage.getByTestId("sidebar-workspace-trigger")).toHaveText(/Improve Kandev/); + + // First bug: New Task opens the Improve dialog, dismiss the intro, submit. + await testPage.getByTestId("create-task-button").click(); + const introDialog = testPage.getByRole("dialog", { name: "Improve Kandev" }); + await introDialog.getByTestId("improve-kandev-skip-intro").click(); + await testPage.getByTestId("improve-kandev-proceed").click(); + + const createDialog = testPage.getByTestId("create-task-dialog"); + await expect(createDialog).toBeVisible({ timeout: 10_000 }); + await createDialog.getByTestId("task-title-input").fill("First contribution"); + await createDialog + .getByTestId("task-description-input") + .fill("First bug: column header overlap on narrow viewports."); + const submit = createDialog.getByTestId("submit-start-agent"); + await expect(submit).toBeEnabled({ timeout: 10_000 }); + await submit.click(); + await expect(createDialog).toBeHidden({ timeout: 10_000 }); + await expect + .poll(async () => (await apiClient.listTasks(dedicated.id)).tasks) + .toContainEqual(expect.objectContaining({ title: "First contribution" })); + + // Second bug: reopen the dialog in the same workspace. The bootstrap probe + // must re-run and unblock submit — regression: it stayed at the idle + // "Preparing kandev repository in background" banner with submit disabled + // forever because the workspace-choice gate was never re-asserted. + await testPage.getByTestId("create-task-button").click(); + await expect(createDialog).toBeVisible({ timeout: 10_000 }); + // Contributor banner proves this is the Improve dialog with bootstrap + // ready (the generic create dialog never renders it). + await expect(createDialog.getByText("@octocat")).toBeVisible({ timeout: 10_000 }); + await createDialog.getByTestId("task-title-input").fill("Second contribution"); + await createDialog + .getByTestId("task-description-input") + .fill("Second bug: missing keyboard shortcut for marking a task Done."); + await expect(submit).toBeEnabled({ timeout: 10_000 }); + await submit.click(); + await expect(createDialog).toBeHidden({ timeout: 10_000 }); + await expect + .poll(async () => (await apiClient.listTasks(dedicated.id)).tasks) + .toContainEqual(expect.objectContaining({ title: "Second contribution" })); + }); + test("New task button opens the Improve Kandev dialog in the dedicated workspace", async ({ testPage, apiClient, diff --git a/docs/plans/improve-kandev-dialog-reopen-bootstrap/plan.md b/docs/plans/improve-kandev-dialog-reopen-bootstrap/plan.md new file mode 100644 index 0000000000..d61bf1d6ad --- /dev/null +++ b/docs/plans/improve-kandev-dialog-reopen-bootstrap/plan.md @@ -0,0 +1,116 @@ +--- +spec: docs/specs/improve-kandev/spec.md +created: 2026-08-10 +status: implemented +--- + +# Implementation Plan: Improve Kandev Dialog Reopen Bootstrap + +## Overview + +After the first contribution is filed, the Improve Kandev dialog hangs on +reopen with the "Preparing kandev repository in background" banner and a +permanently blocked submit button. Root cause: `useResetOnClose` resets +`workspaceChoiceConfirmed` to `false` on close, but the effect that +auto-confirms the choice when the dedicated workspace exists only re-ran when +`improveWorkspaceMissing` changed — a value that is stable once the workspace +is known. The bootstrap gate (`!workspaceChoiceConfirmed`) then early-returns +on every reopen, so no bootstrap request is ever sent. Fix: re-assert the +confirmation on every dialog open by keying the effect on `open` as well. + +--- + +## Frontend + +### `components/improve-kandev-dialog.tsx` +- The auto-confirm effect becomes: + + ```ts + useEffect(() => { + if (open && !improveWorkspaceMissing) { + setWorkspaceChoiceConfirmed(true); + } + }, [improveWorkspaceMissing, open]); + ``` + + `open` in the dependency array makes the effect re-run on every open + transition, so `workspaceChoiceConfirmed` is re-asserted after + `useResetOnClose` clears it — regardless of whether `improveWorkspaceMissing` + changed. The `open &&` guard keeps the mount-time ordering safe (the effect + never confirms while the dialog is closed) and leaves the + workspace-creation choice gate untouched when the workspace is missing. + +### Tests +- `components/improve-kandev-dialog.test.tsx`: component regression test — + render with the dedicated workspace present and skip-intro set, wait for one + bootstrap call, rerender with `open={false}` then `open`, assert bootstrap + is called again. +- `e2e/tests/improve-kandev.spec.ts`: end-to-end regression test — activate + the dedicated workspace, file a first bug (skip intro, submit), reopen the + dialog, assert the contributor banner (`@octocat`) appears (bootstrap ready) + and a second bug can be submitted. + +--- + +## Tests + +- **What:** reopening the dialog in the dedicated workspace re-runs bootstrap + (spec regression scenario). **File:** + `apps/web/components/improve-kandev-dialog.test.tsx`. **How:** component + test asserting `bootstrapImproveKandev` is called on the second open. +- **What:** a second bug can be filed end-to-end with the dedicated workspace + active. **File:** `apps/web/e2e/tests/improve-kandev.spec.ts`. **How:** real + backend + mocked bootstrap; two submissions, both asserted in the dedicated + workspace. + +--- + +## E2E Tests + +- **Scenario:** GIVEN the dedicated `Improve Kandev` workspace already exists + and the intro has been dismissed, WHEN the user closes the dialog and + reopens it to file another report, THEN bootstrap re-runs and the submit + button becomes enabled. + **File:** `apps/web/e2e/tests/improve-kandev.spec.ts`. + **What to verify:** after reopening, `@octocat` (contributor banner = + bootstrap ready) is visible and the second task is created in the dedicated + workspace. + +--- + +## Verification Results + +Completed for task 01 (see its `## Results`). Summary on the final tree: + +- Unit: `pnpm vitest run` on the dialog/model/sidebar suites → 49/49 passed, + including the new reopen regression test. +- Lint: eslint clean on all three touched files. +- Typecheck: `tsc --noEmit` clean. +- E2E: `pnpm e2e:raw tests/improve-kandev.spec.ts + tests/mobile-improve-kandev.spec.ts` → 15 passed. +- Both new regression tests (component + e2e) were verified red before the + fix and green after. + +### PR fixup (PR #2484) + +- Remediation commit `5a034e86b` (comment-only) documents that the per-open + bootstrap re-probe is intentional, addressing the non-blocking review + suggestion; the second suggestion (pre-existing double `readSkipIntro()`) + was deferred per the reviewer. +- Final head `5a034e86b`: 27/27 checks green (incl. CodeRabbit, full E2E + matrix, frontend suite, Build), no failed or pending checks, no unresolved + review threads, `mergeable_state: clean`. Claude review at the substantive + head (`0d96473ad`) verdict: "Ready to merge", 0 blockers. + +--- + +## Implementation Waves And Parallel Candidates + +Wave 1 (single task, sequential): +- [x] [task-01-reopen-bootstrap-regression](task-01-reopen-bootstrap-regression.md) + +--- + +## Open Questions + +None. diff --git a/docs/plans/improve-kandev-dialog-reopen-bootstrap/task-01-reopen-bootstrap-regression.md b/docs/plans/improve-kandev-dialog-reopen-bootstrap/task-01-reopen-bootstrap-regression.md new file mode 100644 index 0000000000..1ef872e6c8 --- /dev/null +++ b/docs/plans/improve-kandev-dialog-reopen-bootstrap/task-01-reopen-bootstrap-regression.md @@ -0,0 +1,103 @@ +--- +id: "01-reopen-bootstrap-regression" +title: "Re-run bootstrap when the dialog reopens" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/improve-kandev/spec.md" +--- + +# Task 01: Re-run bootstrap when the dialog reopens + +## Root cause + +`useResetOnClose` in `apps/web/components/improve-kandev-dialog.tsx` resets +`workspaceChoiceConfirmed` to `false` every time the dialog closes. The effect +that auto-confirms the choice when the dedicated workspace exists only fires +when `improveWorkspaceMissing` changes, which is stable once the workspace is +known. On a reopen (e.g. filing a second bug), the bootstrap gate +(`!workspaceChoiceConfirmed`) early-returns and no bootstrap request is ever +sent: the dialog sits at the idle "Preparing kandev repository in background" +banner with submit blocked forever. + +Regression path: dedicated `Improve Kandev` workspace exists (typically the +active workspace), intro dismissed (skip-intro in local storage), dialog +closed then reopened. + +## Acceptance + +1. Reopening the dialog with the dedicated workspace present re-runs + `bootstrapImproveKandev` (component-level: a second call after + open→close→open). +2. End-to-end, a second bug can be filed after the first one in the dedicated + workspace: the contributor banner appears (bootstrap ready) and submit is + enabled. +3. Existing behavior is unchanged: when the dedicated workspace is missing, + the workspace-creation choice still gates bootstrap; the intro flow still + sets the confirmation on Contribute. + +## Verification + +```bash +cd apps/web && pnpm vitest run components/improve-kandev-dialog.test.tsx +cd apps/web && pnpm exec eslint components/improve-kandev-dialog.tsx components/improve-kandev-dialog.test.tsx e2e/tests/improve-kandev.spec.ts +cd apps/web && pnpm run typecheck +# e2e (requires built backend bin/kandev + bin/mock-agent and web dist): +cd apps/web && pnpm e2e:raw tests/improve-kandev.spec.ts +``` + +## Files likely touched + +- `apps/web/components/improve-kandev-dialog.tsx` — auto-confirm effect gains + an `open` dep and `open &&` guard. +- `apps/web/components/improve-kandev-dialog.test.tsx` — reopen regression + test. +- `apps/web/e2e/tests/improve-kandev.spec.ts` — second-bug e2e regression + test. + +## Dependencies + +None. + +## Parallelism + +sequential. + +## Inputs + +- Spec regression scenario: "GIVEN the dedicated Improve Kandev workspace + already exists and the intro has been dismissed, WHEN the user closes the + dialog and reopens it, THEN bootstrap re-runs and submit becomes enabled." +- `useResetOnClose` and `useBootstrapKandev` in + `apps/web/components/improve-kandev-dialog.tsx`. + +## Output contract + +Summary of change, exact commands run with results, task/plan status updated +in the same conversation. + +## Results + +- `cd apps/web && pnpm vitest run components/improve-kandev-dialog.test.tsx + components/improve-kandev-dialog-model.test.ts + components/app-sidebar/app-sidebar-new-task-item.test.tsx + components/app-sidebar/app-sidebar-footer.test.tsx` → 4 files, 49 tests + passed. The new reopen regression test failed before the fix + (`bootstrapImproveKandev` called once, never again after open→close→open) + and passes after. +- `pnpm exec eslint components/improve-kandev-dialog.tsx + components/improve-kandev-dialog.test.tsx e2e/tests/improve-kandev.spec.ts` + → clean. +- `NODE_OPTIONS="--max-old-space-size=8192" pnpm exec tsc --noEmit` → clean. +- `pnpm e2e:raw tests/improve-kandev.spec.ts tests/mobile-improve-kandev.spec.ts` + → 15 passed (14 improve-kandev incl. the new second-bug test, 1 mobile). + The new e2e test failed without the fix (contributor banner `@octocat` never + appears on reopen — bootstrap never runs) and passes with it. +- Red/green evidence: both new tests were run against the pre-fix tree + (stashed fix) and failed for the expected reason, then passed with the fix. +- PR fixup: comment-only remediation `5a034e86b` documenting the intentional + per-open bootstrap re-probe (Claude review suggestion, non-blocking). + Final head CI 27/27 green, threads resolved, mergeable clean. + +Security/trust: none. External side-effect boundaries: none. diff --git a/docs/specs/improve-kandev/spec.md b/docs/specs/improve-kandev/spec.md index fd409a8c9f..5770814dce 100644 --- a/docs/specs/improve-kandev/spec.md +++ b/docs/specs/improve-kandev/spec.md @@ -143,6 +143,13 @@ the user's own agent picks up immediately — turning every report into a contri called again, **THEN** the same workspace (and the same hidden workflow instances) are reused and the response's `workspace_id` is unchanged. +- **GIVEN** the dedicated `Improve Kandev` workspace already exists and the + intro has been dismissed, **WHEN** the user closes the Improve Kandev dialog + and reopens it to file another report, **THEN** the bootstrap probe runs + again automatically and the submit button becomes enabled once it completes — + the dialog never stays stuck at the "Preparing kandev repository in + background" banner with submission blocked. + - **GIVEN** the user's active workspace is not the dedicated workspace, **WHEN** the dialog submits a task, **THEN** the task appears in the dedicated workspace and no task is created in the active workspace.