fix(improve-kandev): re-run bootstrap when the dialog reopens (#2484)
This commit is contained in:
Clément Fiere
@@ -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(
|
||||
<ImproveKandevDialog open onOpenChange={() => {}} 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(<ImproveKandevDialog open={false} onOpenChange={() => {}} workspaceId="ws-active" />);
|
||||
rerender(<ImproveKandevDialog open onOpenChange={() => {}} 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");
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
+103
@@ -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.
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user