diff --git a/apps/web/components/task-create-dialog-handlers.test.ts b/apps/web/components/task-create-dialog-handlers.test.ts index a7c8513acf..d621daca75 100644 --- a/apps/web/components/task-create-dialog-handlers.test.ts +++ b/apps/web/components/task-create-dialog-handlers.test.ts @@ -175,6 +175,34 @@ describe("repository source changes", () => { expect(fs.setExecutorId).toHaveBeenCalledWith(""); expect(fs.setExecutorProfileId).toHaveBeenCalledWith(""); }); + + it("clears the executor when leaving repository-less mode", () => { + const setNoRepository = vi.fn(); + const setUseRemote = vi.fn(); + const setExecutorId = vi.fn(); + const setExecutorProfileId = vi.fn(); + const setWorkspacePath = vi.fn(); + const fs = { + noRepository: true, + useRemote: false, + executorId: "executor-local", + executorProfileId: LOCAL_PROFILE_ID, + repositories: [], + setNoRepository, + setUseRemote, + setExecutorId, + setExecutorProfileId, + setWorkspacePath, + } as unknown as DialogFormState; + const { result } = renderHook(() => useDialogHandlers(fs, [])); + + act(() => result.current.handleToggleNoRepository()); + + expect(setNoRepository).toHaveBeenCalledWith(false); + expect(setWorkspacePath).toHaveBeenCalledWith(""); + expect(setExecutorId).toHaveBeenCalledWith(""); + expect(setExecutorProfileId).toHaveBeenCalledWith(""); + }); }); describe("task title handling", () => { diff --git a/apps/web/components/task-create-dialog-handlers.ts b/apps/web/components/task-create-dialog-handlers.ts index 63630d275a..2ddce4dd37 100644 --- a/apps/web/components/task-create-dialog-handlers.ts +++ b/apps/web/components/task-create-dialog-handlers.ts @@ -372,12 +372,15 @@ function useGitHubAndFreshBranchHandlers(fs: DialogFormState) { const handleToggleNoRepository = useCallback(() => { const next = !fs.noRepository; fs.setNoRepository(next); + // Clear the executor selection in both directions so the destination + // source mode can resolve its own policy. None mode will re-pick Local; + // Repo mode will re-pick the workspace default or Worktree fallback. + fs.setExecutorId(""); + fs.setExecutorProfileId(""); if (next) { fs.setUseRemote(false); - // Clear the executor selection so the auto-fill effect re-picks a - // non-worktree default (worktree is unworkable in no-repo mode). - fs.setExecutorId(""); - fs.setExecutorProfileId(""); + // None mode excludes Worktree, so its auto-fill effect picks a + // non-worktree default. syncTaskCreateLastUsed({ repository_id: null, branch: null, diff --git a/apps/web/e2e/tests/task/create-task-executor-default.spec.ts b/apps/web/e2e/tests/task/create-task-executor-default.spec.ts index 53d97441e0..3811d7d91b 100644 --- a/apps/web/e2e/tests/task/create-task-executor-default.spec.ts +++ b/apps/web/e2e/tests/task/create-task-executor-default.spec.ts @@ -110,4 +110,36 @@ test.describe("Task-create executor safety defaults", () => { await saveTaskCreatePreference(apiClient, seedData, worktreeProfile.id); } }); + + test("returns to Worktree after leaving repository-less mode", async ({ + testPage, + apiClient, + seedData, + }) => { + const { localProfile, worktreeProfile } = await executorProfiles(apiClient); + const { workspaces } = await apiClient.listWorkspaces(); + const workspace = workspaces.find((item) => item.id === seedData.workspaceId); + expect(workspace, "the seeded workspace is required by the fixture").toBeDefined(); + const originalDefaultExecutorId = workspace?.default_executor_id ?? ""; + + try { + await apiClient.updateWorkspace(seedData.workspaceId, { default_executor_id: "" }); + await saveTaskCreatePreference(apiClient, seedData, localProfile.id); + + await openCreateTask(testPage); + const executorSelector = testPage.getByTestId("executor-profile-selector"); + await expect(executorSelector).toContainText(worktreeProfile.name); + + await testPage.getByTestId("source-mode-scratch").click(); + await expect(executorSelector).toContainText(localProfile.name); + + await testPage.getByTestId("source-mode-workspace").click(); + await expect(executorSelector).toContainText(worktreeProfile.name); + } finally { + await apiClient.updateWorkspace(seedData.workspaceId, { + default_executor_id: originalDefaultExecutorId, + }); + await saveTaskCreatePreference(apiClient, seedData, worktreeProfile.id); + } + }); }); diff --git a/docs/plans/task-create-source-executor-reset/plan.md b/docs/plans/task-create-source-executor-reset/plan.md new file mode 100644 index 0000000000..d3a50a3829 --- /dev/null +++ b/docs/plans/task-create-source-executor-reset/plan.md @@ -0,0 +1,117 @@ +--- +spec: docs/specs/tasks/task-create-executor-default.md +created: 2026-08-18 +status: complete +--- + +# Implementation Plan: Reset Executor When Returning to Repo + +## Overview + +Reset the task-create executor selection whenever the dialog leaves repository-less mode so the +existing source-aware default effect can resolve the Repo destination again. Lock the transition +down with a focused handler regression, then exercise the complete Repo to None to Repo sequence in +the existing executor-default Playwright suite. No backend, persistence, API, copy, or layout change +is required. + +## Confirmed root cause + +`handleToggleNoRepository` in +`apps/web/components/task-create-dialog-handlers.ts` clears `executorId` and +`executorProfileId` only when entering repository-less mode. The source-aware auto-pick effect in +`apps/web/components/task-create-dialog-effects.ts` intentionally returns early while either +selection is populated. After repository-less mode selects Local, leaving the mode clears only +`workspacePath`. The stale Local profile therefore prevents Repo mode from reapplying the workspace +default or Worktree fallback. + +The current focused handler and executor-effect suites pass 36 tests, but they do not cover the +repository-less-to-Repo transition. + +## Frontend + +### Source transition reset + +- Update `handleToggleNoRepository` to clear `executorId` and `executorProfileId` when switching in + either direction. +- Preserve the current mutually exclusive source flags, repository and branch selections, remote + repository state, workspace-path cleanup, and last-used source cleanup. +- Reuse `useDefaultSelectionsEffect` for destination-specific selection. Repository-less mode + continues to prefer direct Local. Repo mode honors a valid workspace default and otherwise + prefers Worktree. +- Do not hardcode a Worktree profile in the handler. The existing policy remains authoritative for + workspaces that explicitly default to Local and for fallback behavior when Worktree is absent. + +### Mobile parity + +- **Desktop and mobile outcome:** the responsive Create Task dialog applies the same executor policy + after the Repo to None to Repo sequence. +- **Nearest shipped mobile exemplar:** the current mobile Create Task dialog remains the interaction + and composition baseline. +- **Presentation and state:** no markup, navigation, overlay, scrolling, safe-area, focus, or touch + behavior changes. The fix stays in the shared dialog handlers and selection effects. +- **Coverage:** the focused unit regression plus the desktop real-dialog E2E establish the shared + state transition. A new mobile-only Playwright test is not required because this is state + normalization inside an unchanged responsive component. + +## Tests + +- **What:** leaving repository-less mode invalidates the Local executor selection so destination + policy can run again. + - **File:** `apps/web/components/task-create-dialog-handlers.test.ts` + - **How:** render `useDialogHandlers` with repository-less mode active and a Local executor. + Invoke `handleToggleNoRepository`. Assert that both executor setters receive an empty value and + that the handler clears the workspace path. +- **What:** entering repository-less mode retains its current reset behavior. + - **File:** `apps/web/components/task-create-dialog-handlers.test.ts` + - **How:** keep or add the paired entry assertion so a later refactor cannot make the reset + one-directional again. +- **What:** an empty repository-backed selection still resolves to Worktree through existing policy. + - **File:** `apps/web/components/task-create-dialog-effects-executor.test.ts` + - **How:** retain the existing executor-first/default-policy cases as companion coverage. This + fix does not introduce a new policy branch. + +## E2E Tests + +- **Scenario:** **GIVEN** Repo with no executor default, **WHEN** the user selects None and Repo, + **THEN** the visible executor changes from Worktree to Local to Worktree. + - **File:** `apps/web/e2e/tests/task/create-task-executor-default.spec.ts` + - **Verification:** use the source-mode test IDs and the visible executor-profile selector. + Restore the workspace default and task-create preference during cleanup. Use the managed + production build. + +## Verification Results + +Task 01 and Task 02 are complete. During diagnosis, the unchanged baseline command +`cd apps/web && pnpm test -- --run components/task-create-dialog-handlers.test.ts components/task-create-dialog-effects-executor.test.ts` +passed 36 tests across 2 files. The Task 01 implementation command passed 37 tests across 2 files, +and the web typecheck passed. The managed Chromium command +`cd apps/web && pnpm e2e:run --project chromium tests/task/create-task-executor-default.spec.ts` +passed all 3 tests after rebuilding the production backend and Vite assets. + +## Implementation Tasks + +Wave 1: + +- [x] [Task 01: Reset executor on source transition](task-01-reset-source-executor.md) + +Wave 2: + +- [x] [Task 02: Prove the real dialog transition](task-02-dialog-transition-e2e.md) + +Execution is sequential in the primary conversation. No subagent delegation is planned or +authorized. + +## Risks + +- The executor-ID and executor-profile effects settle through microtasks. Unit assertions prove + invalidation at the handler boundary. Playwright proves the final visible selection. +- Hardcoding Worktree in the toggle handler would break explicit Local workspace defaults. The + handler must only invalidate stale state and let the existing policy select the destination. +- The E2E fixture persists workspace and task-create settings across tests, so cleanup must restore + both values even after a failed assertion. + +## Out of scope + +- Changing executor-default precedence or persistence. +- Changing Repo, Remote, or None presentation and labels. +- Changing repository, branch, or remote-source retention while switching modes. diff --git a/docs/plans/task-create-source-executor-reset/task-01-reset-source-executor.md b/docs/plans/task-create-source-executor-reset/task-01-reset-source-executor.md new file mode 100644 index 0000000000..8f98456b54 --- /dev/null +++ b/docs/plans/task-create-source-executor-reset/task-01-reset-source-executor.md @@ -0,0 +1,93 @@ +--- +id: "01-reset-source-executor" +title: "Reset executor on source transition" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/tasks/task-create-executor-default.md" +--- + +# Task 01: Reset Executor on Source Transition + +## Intent + +Invalidate the executor selected for repository-less mode when the user returns to Repo so the +existing destination-source policy can choose the correct profile. + +## Acceptance + +- Entering repository-less mode clears the previous executor and continues to resolve direct Local. +- Leaving repository-less mode also clears Local so Repo resolves the valid workspace default or + Worktree fallback. +- Source flags, selected repositories and branches, remote rows, and last-used persistence behavior + remain unchanged. + +## TDD sequence + +1. Add a focused `useDialogHandlers` regression for leaving repository-less mode with a populated + Local executor. +2. Run the regression. Verify that it fails because the executor setters are not called. +3. Add or retain the paired entering-mode assertion. +4. Move the executor invalidation to the shared part of `handleToggleNoRepository` and run the + focused handler and executor-policy suites to GREEN. +5. Run the web typecheck. + +## Files likely touched + +- `apps/web/components/task-create-dialog-handlers.ts` +- `apps/web/components/task-create-dialog-handlers.test.ts` + +## Dependencies + +None. + +## Parallelism + +`sequential`. This task owns the shared state transition required by Task 02. + +## Inputs + +- Spec scenario for Repo to repository-less mode to Repo. +- Plan sections `Confirmed root cause` and `Source transition reset`. +- Existing `useDefaultSelectionsEffect` policy and ADR + `docs/decisions/2026-08-01-repository-task-executor-defaults.md`. + +## Verification + +```bash +cd apps && pnpm install --frozen-lockfile && pnpm --filter @kandev/web test -- components/task-create-dialog-handlers.test.ts components/task-create-dialog-effects-executor.test.ts && cd web && pnpm run typecheck +``` + +## Risks + +- Do not select Worktree directly in the handler. Explicit workspace defaults and fallback policy + belong to the existing effects. +- Assert both executor ID and profile invalidation because either populated field can suppress or + conflict with recomputation. + +## Output contract + +Report the RED failure, changed files, exact results, blockers, and risks. Then update this task and +`plan.md` in the same conversation. + +## Results + +RED: + +- `cd apps/web && pnpm test -- --run components/task-create-dialog-handlers.test.ts -t "clears the executor when leaving repository-less mode"` +- Failed with the expected assertion because the executor setters received no empty value. + +GREEN and task checks: + +- `cd apps/web && pnpm test -- --run components/task-create-dialog-handlers.test.ts -t "clears the executor when leaving repository-less mode"` — 1 passed. +- `cd apps && pnpm install --frozen-lockfile` — dependencies were ready. +- `cd apps && pnpm --filter @kandev/web test -- components/task-create-dialog-handlers.test.ts components/task-create-dialog-effects-executor.test.ts` — 37 passed across 2 files. +- `cd apps/web && pnpm run typecheck` — passed. + +Changed files: + +- `apps/web/components/task-create-dialog-handlers.ts` +- `apps/web/components/task-create-dialog-handlers.test.ts` + +Cleanup and side effects: no temporary files or external services were created. diff --git a/docs/plans/task-create-source-executor-reset/task-02-dialog-transition-e2e.md b/docs/plans/task-create-source-executor-reset/task-02-dialog-transition-e2e.md new file mode 100644 index 0000000000..89fedaadb6 --- /dev/null +++ b/docs/plans/task-create-source-executor-reset/task-02-dialog-transition-e2e.md @@ -0,0 +1,93 @@ +--- +id: "02-dialog-transition-e2e" +title: "Prove the real dialog transition" +status: done +wave: 2 +depends_on: ["01-reset-source-executor"] +plan: "plan.md" +spec: "../../specs/tasks/task-create-executor-default.md" +--- + +# Task 02: Prove the Real Dialog Transition + +## Intent + +Exercise the exact user-reported source-tab sequence through the production-built Create Task +dialog and prove that its visible executor follows the active source. + +## Acceptance + +- Repo mode initially shows Worktree when the workspace has no executor default. +- None mode shows a direct Local profile, and returning to Repo shows Worktree again. +- The test restores shared workspace and task-create preference state even when an assertion fails. + +## TDD sequence + +1. Add the Repo to None to Repo scenario to the existing executor-default Playwright suite. +2. Temporarily revert Task 01, then verify RED at the final Worktree assertion. +3. Restore Task 01 and run the focused managed Chromium spec against rebuilt production assets. +4. Verify GREEN. Reconcile the cleanup and recorded result with the task and plan. + +## Files likely touched + +- `apps/web/e2e/tests/task/create-task-executor-default.spec.ts` + +## Dependencies + +- Task 01 must be complete. + +## Parallelism + +`sequential`. It validates the shared source-transition behavior implemented by Task 01. + +## Inputs + +- Spec scenario for Repo to repository-less mode to Repo. +- Existing `executorProfiles`, `saveTaskCreatePreference`, `openCreateTask`, source-mode test IDs, + and executor-profile selector patterns in the target spec. +- E2E fixture rule that shared workspace and user settings must be restored in `finally` cleanup. + +## Verification + +```bash +cd apps/web && pnpm e2e:run --project chromium tests/task/create-task-executor-default.spec.ts +``` + +## Mobile parity + +No mobile-only E2E is added. This repair changes shared state normalization inside the existing +responsive dialog and does not change layout, navigation, scrolling, focus, touch targets, or +pointer behavior. Existing mobile Create Task specs remain the interaction and layout coverage. + +## Risks + +- Wait for the visible selector text after each source change. Do not assert transient setter order. +- Restore `default_executor_id` and a Worktree task-create preference in cleanup so the worker-scoped + backend cannot leak Local into neighboring tests. + +## Output contract + +Report the focused Playwright result, build evidence, changed files, cleanup evidence, blockers, +and risks. Then update this task and `plan.md` in the same conversation. + +## Results + +RED: + +- With the Task 01 production change temporarily reverted, + `cd apps/web && pnpm e2e:run --project chromium tests/task/create-task-executor-default.spec.ts` + ran 3 tests and failed the new transition scenario. The selector stayed at `LocalLocal` after + returning to Repo. + +GREEN: + +- `cd apps/web && pnpm e2e:run --project chromium tests/task/create-task-executor-default.spec.ts` + rebuilt the backend and Vite production assets, then passed all 3 tests. + +Changed files: + +- `apps/web/e2e/tests/task/create-task-executor-default.spec.ts` + +Cleanup and side effects: the managed runner used isolated temporary E2E data and cleaned it up. +No mobile-only test was added because the repair changes shared selection state without changing +responsive composition or touch behavior. diff --git a/docs/specs/tasks/task-create-executor-default.md b/docs/specs/tasks/task-create-executor-default.md index c72044283c..2eaf3cd6ab 100644 --- a/docs/specs/tasks/task-create-executor-default.md +++ b/docs/specs/tasks/task-create-executor-default.md @@ -1,6 +1,7 @@ --- status: approved created: 2026-08-01 +updated: 2026-08-18 owner: Kandev decision: ADR-2026-08-01-repository-task-executor-defaults --- @@ -9,9 +10,9 @@ decision: ADR-2026-08-01-repository-task-executor-defaults ## Why -Repository-backed tasks should start in isolated workspaces unless the user or workspace has made -an explicit contrary choice. A portable last-used Local executor profile must not silently turn a -later ordinary task into an in-place run, including after a Kandev update or in another browser. +Repository-backed tasks must start in isolated workspaces unless the user or workspace makes an +explicit contrary choice. A portable last-used Local executor profile must not change a later +ordinary task to an in-place run. This rule also applies after an update and in another browser. ## Broken behavior @@ -20,16 +21,23 @@ it resolves the task source and workspace default. After any Local task, that po can therefore select Local for the next ordinary repository-backed task even when the workspace has no Local default. +The same stale selection can occur within one open dialog. Repository-less mode requires Local and +clears the previous executor when entered. Repo mode can then retain Local instead of resolving the +repository-backed default again. + ## What - The task-create dialog resolves the default executor before it considers the last-used executor profile. -- An ordinary repository-backed task uses the workspace's valid configured default executor; when - the workspace has no valid default, it uses Worktree when Worktree is available. +- An ordinary repository-backed task uses the workspace's valid configured default executor. + When the workspace has no valid default, it uses Worktree when Worktree is available. - A task with no repository, or a task created from an explicit unmanaged local path, continues to prefer a direct Local executor. - The backend-owned last-used executor profile may refine the profile choice only when that profile belongs to the already-resolved default executor. It cannot switch the executor. +- Changing between repository-backed and repository-less source modes resolves the executor policy + for the destination mode. Returning to Repo does not retain Local only because repository-less + mode required it. - A valid explicit workspace default, including Local, remains authoritative. - A user may manually select a different executor profile for the task being created. Successful task creation continues to record that profile in backend user settings, but the recorded value @@ -64,6 +72,8 @@ within the executor selected by policy, not a portable override of that policy. - **GIVEN** Local was selected manually for one ordinary task and the workspace still has no executor default, **WHEN** the next ordinary repository-backed task dialog opens, **THEN** it returns to Worktree. +- **GIVEN** Repo with no executor default, **WHEN** the user selects None and Repo, **THEN** the + executor changes from Worktree to Local to Worktree. - **GIVEN** the preferred executor has no usable profile, **WHEN** the dialog resolves defaults, **THEN** it selects the first existing eligible fallback rather than leaving an invalid profile.