fix(app): advance create settings on enter (#329)

Author: skulidropek

packages/app/src/docker-git/menu-create-shared.ts

@@ -898,6 +898,29 @@ export const applyCreateDisplaySettingsStep = (
       (nextValues) => continueCreateDisplayFlow(view, nextValues)
     ))
 
+/**
+ * Applies one browser Create settings display row and advances selection downward.
+ *
+ * @pure true
+ * @effect none
+ * @invariant successful result preserves applied values and moves over the finite display row cycle
+ * @precondition view.step points at a settings display row
+ * @postcondition result._tag = "Continue" -> result.view.buffer = ""
+ * @complexity O(1)
+ */
+export const advanceCreateDisplaySettingsStep = (
+  contextOrCwd: string | CreateFlowContext,
+  view: DisplayModeFlowView
+): AdvanceCreateFlowResult | null => {
+  const applied = applyCreateDisplaySettingsStep(contextOrCwd, view)
+  if (applied === null || applied._tag !== "Continue" || !isDisplayModeFlowView(applied.view)) {
+    return applied
+  }
+
+  const movedView = moveCreateDisplaySettingsStep(applied.view, "down")
+  return movedView === null ? applied : { ...applied, view: movedView }
+}
+
 /**
  * Completes browser Create settings by applying a non-empty active buffer first.
  *

packages/app/src/web/app-ready-create.ts

@@ -3,8 +3,8 @@ import { type Dispatch, type SetStateAction, useEffect } from "react"
 import { formatParseError } from "../docker-git/cli/usage.js"
 import { nextBufferValue } from "../docker-git/menu-buffer-input.js"
 import {
+  advanceCreateDisplaySettingsStep,
   advanceCreateFlow,
-  applyCreateDisplaySettingsStep,
   completeCreateDisplaySettingsFlow,
   createDisplayFlowView,
   type CreateFlowView,
@@ -75,7 +75,7 @@ const resolveCreateSubmitResult = (
 ): ReturnType<typeof advanceCreateFlow> => {
   if (isDisplayModeFlowView(createView)) {
     return mode === "advance"
-      ? applyCreateDisplaySettingsStep(createContext, createView)
+      ? advanceCreateDisplaySettingsStep(createContext, createView)
       : completeCreateDisplaySettingsFlow(createContext, createView)
   }
   const next = advanceCreateFlow(createContext, createView, { quickCreate: mode === "quick-create" })

packages/app/src/web/panel-create-select.tsx

@@ -4,7 +4,6 @@ import {
   type CreateFlowContext,
   type CreateFlowView,
   type CreateSettingsChoiceDirection,
-  createSettingsHint,
   isCreateFlowRepoStep,
   isDisplayModeFlowView,
   renderCreateStepLabel,
@@ -20,6 +19,7 @@ import type { CreateSubmitMode } from "./app-ready-create.js"
 
 const renderStepColor = (active: boolean): string => active ? "#56f39a" : "#8fa6c4"
 
+const webCreateSettingsNavigationHint = "↑ - up, ↓ - down, Enter - apply + down"
 const webCreateSettingsChoiceHint = "←/→ - choose yes/no or GPU"
 
 const createPrompt = (
@@ -225,7 +225,7 @@ const CreateHintBlock = (
   <HelpLines
     lines={[
       ...(isRepoStep ? ["Repo URL or URL + CLI flags."] : []),
-      ...(isRepoStep ? [] : [createSettingsHint]),
+      ...(isRepoStep ? [] : [webCreateSettingsNavigationHint]),
       ...(isRepoStep ? [] : [webCreateSettingsChoiceHint]),
       ...(compact && isRepoStep ? ["↑/↓ = menu, ←/→ = project"] : []),
       `Current cwd: ${controllerCwd}`

packages/app/tests/docker-git/app-ready-create-fixture.ts

@@ -3,8 +3,8 @@ import { expect, vi } from "vitest"
 
 import {
   type CreateFlowView,
-  type DisplayModeFlowView,
   createInitialFlowView,
+  type DisplayModeFlowView,
   resolveCreateDisplaySteps
 } from "../../src/docker-git/menu-create-shared.js"
 import type { CreateInputs, CreateStep } from "../../src/docker-git/menu-types.js"

packages/app/tests/docker-git/app-ready-create-settings.test.ts

@@ -75,12 +75,12 @@ describe("app-ready-create settings", () => {
     expect(arrowView.values.gpu).toBeUndefined()
     expect(enterResult.handled).toBe(true)
     expect(enteredView.values.gpu).toBe("all")
-    expect(enteredView.step).toBe(resolveCreateDisplaySteps().indexOf("gpu"))
+    expect(enteredView.step).toBe(resolveCreateDisplaySteps().indexOf("runUp"))
     expect(enteredView.buffer).toBe("")
     expect(submitCreateInputsMock).not.toHaveBeenCalled()
   })
 
-  it("keeps an applied settings row selected and visible instead of submitting", () => {
+  it("wraps to the first settings row after applying the last settings row", () => {
     const createView: CreateFlowView = {
       ...createSettingsFlowViewAtStep("force", "y"),
       values: {
@@ -97,21 +97,36 @@ describe("app-ready-create settings", () => {
 
     expect(handled).toBe(true)
     expect(enteredView.values.force).toBe(true)
-    expect(enteredView.step).toBe(resolveCreateDisplaySteps().indexOf("force"))
+    expect(enteredView.step).toBe(resolveCreateDisplaySteps().indexOf("cpuLimit"))
     expect(enteredView.buffer).toBe("")
     expect(submitCreateInputsMock).not.toHaveBeenCalled()
   })
 
+  it("keeps the previous setting value when Enter applies an empty buffer", () => {
+    const createView: CreateFlowView = {
+      ...createSettingsFlowViewAtStep("runUp", ""),
+      values: {
+        ...createSettingsFlowView().values,
+        runUp: false
+      }
+    }
+    const emptyResult = runCreateKey(handleCreateKey, createView, "Enter")
+    const emptyView = requireCreateViewValue(emptyResult.setCreateViewSpy.mock.calls[0]?.[0])
+
+    expect(emptyResult.handled).toBe(true)
+    expect(emptyView.step).toBe(resolveCreateDisplaySteps().indexOf("mcpPlaywright"))
+    expect(emptyView.values.runUp).toBe(false)
+    expect(emptyView.buffer).toBe("")
+  })
+
   it("navigates to the next visible row after applying a settings row", () => {
     const enterResult = runCreateKey(handleCreateKey, createSettingsFlowViewAtStep("mcpPlaywright", "y"), "Enter")
     const enteredView = requireCreateViewValue(enterResult.setCreateViewSpy.mock.calls[0]?.[0])
-    const downResult = runCreateKey(handleCreateKey, enteredView, "ArrowDown")
-    const downView = requireCreateViewValue(downResult.setCreateViewSpy.mock.calls[0]?.[0])
 
-    expect(downResult.handled).toBe(true)
-    expect(downView.step).toBe(resolveCreateDisplaySteps().indexOf("force"))
-    expect(downView.values.enableMcpPlaywright).toBe(true)
-    expect(downView.buffer).toBe("")
+    expect(enterResult.handled).toBe(true)
+    expect(enteredView.step).toBe(resolveCreateDisplaySteps().indexOf("force"))
+    expect(enteredView.values.enableMcpPlaywright).toBe(true)
+    expect(enteredView.buffer).toBe("")
   })
 
   it("clears an unconfirmed preview when navigating away from a settings row", () => {

packages/app/tests/docker-git/create-flow-render.test.ts

@@ -7,8 +7,8 @@ import {
   type CreateFlowContext,
   type CreateFlowView,
   createInitialFlowView,
-  createSettingsHint,
   type CreateModeFlowView,
+  createSettingsHint,
   type DisplayModeFlowView,
   renderCreateStepLabel,
   resolveCreateDisplaySteps,
@@ -34,6 +34,7 @@ const createContext: CreateFlowContext = {
 const renderWithUi = (element: ReactElement): string =>
   renderToStaticMarkup(createElement(UiProvider, { primitives: webPrimitives }, element))
 
+const webCreateSettingsNavigationHint = "↑ - up, ↓ - down, Enter - apply + down"
 const webCreateSettingsChoiceHint = "←/→ - choose yes/no or GPU"
 const createSettingsView = (): DisplayModeFlowView => createFeatureRepoDisplaySettingsView(createContext)
 
@@ -54,6 +55,7 @@ const renderCreatePanel = (
 const activeStepMarker = "> "
 
 const countActiveStepMarkers = (html: string): number => html.split(activeStepMarker).length - 1
+const renderedHelpLine = (line: string): string => `>${line}</div>`
 
 const renderStepLabels = (createView: CreateFlowView): ReadonlyArray<string> => {
   const defaults = resolveCreateInputs(createContext, createView.values)
@@ -204,10 +206,18 @@ describe("Create flow rendering", () => {
   })
 
   it("renders the settings navigation hint only after leaving the repo URL step", () => {
-    expect(renderCreatePanel(createInitialFlowView(featureCreateRepoUrl))).not.toContain(createSettingsHint)
-    expect(renderCreatePanel(createInitialFlowView(featureCreateRepoUrl))).not.toContain(webCreateSettingsChoiceHint)
-    expect(renderCreatePanel(createSettingsView())).toContain(createSettingsHint)
-    expect(renderCreatePanel(createSettingsView())).toContain(webCreateSettingsChoiceHint)
+    expect(renderCreatePanel(createInitialFlowView(featureCreateRepoUrl))).not.toContain(
+      renderedHelpLine(createSettingsHint)
+    )
+    expect(renderCreatePanel(createInitialFlowView(featureCreateRepoUrl))).not.toContain(
+      renderedHelpLine(webCreateSettingsNavigationHint)
+    )
+    expect(renderCreatePanel(createInitialFlowView(featureCreateRepoUrl))).not.toContain(
+      renderedHelpLine(webCreateSettingsChoiceHint)
+    )
+    expect(renderCreatePanel(createSettingsView())).not.toContain(renderedHelpLine(createSettingsHint))
+    expect(renderCreatePanel(createSettingsView())).toContain(renderedHelpLine(webCreateSettingsNavigationHint))
+    expect(renderCreatePanel(createSettingsView())).toContain(renderedHelpLine(webCreateSettingsChoiceHint))
   })
 
   it("renders terminal Create hints with the same repo/settings split", () => {
@@ -216,8 +226,9 @@ describe("Create flow rendering", () => {
 
     expect(repoHtml).not.toContain("Enter = next, Esc = cancel.")
     expect(repoHtml).not.toContain("Shift+Enter")
-    expect(settingsHtml).toContain(createSettingsHint)
-    expect(settingsHtml).not.toContain(webCreateSettingsChoiceHint)
+    expect(settingsHtml).toContain(renderedHelpLine(createSettingsHint))
+    expect(settingsHtml).not.toContain(renderedHelpLine(webCreateSettingsNavigationHint))
+    expect(settingsHtml).not.toContain(renderedHelpLine(webCreateSettingsChoiceHint))
   })
 
   it("preserves hint visibility invariants for every Create step", () => {
@@ -231,10 +242,12 @@ describe("Create flow rendering", () => {
         const panelHtml = renderCreatePanel(view)
         const compactPanelHtml = renderCreatePanel(view, { compact: true })
 
-        expect(panelHtml.includes(createSettingsHint)).toBe(isSettings)
-        expect(compactPanelHtml.includes(createSettingsHint)).toBe(isSettings)
-        expect(panelHtml.includes(webCreateSettingsChoiceHint)).toBe(isSettings)
-        expect(compactPanelHtml.includes(webCreateSettingsChoiceHint)).toBe(isSettings)
+        expect(panelHtml).not.toContain(renderedHelpLine(createSettingsHint))
+        expect(compactPanelHtml).not.toContain(renderedHelpLine(createSettingsHint))
+        expect(panelHtml.includes(renderedHelpLine(webCreateSettingsNavigationHint))).toBe(isSettings)
+        expect(compactPanelHtml.includes(renderedHelpLine(webCreateSettingsNavigationHint))).toBe(isSettings)
+        expect(panelHtml.includes(renderedHelpLine(webCreateSettingsChoiceHint))).toBe(isSettings)
+        expect(compactPanelHtml.includes(renderedHelpLine(webCreateSettingsChoiceHint))).toBe(isSettings)
         expect(panelHtml).not.toContain("Enter = next, Esc = cancel.")
         expect(compactPanelHtml).not.toContain("Enter = next, Esc = cancel.")
         expect(panelHtml).not.toContain("Shift+Enter")
@@ -250,8 +263,9 @@ describe("Create flow rendering", () => {
         const view = step === 0 ? createInitialFlowView(featureCreateRepoUrl) : { ...terminalSettingsView, step }
         const terminalHtml = renderTerminalCreate(view)
 
-        expect(terminalHtml.includes(createSettingsHint)).toBe(step > 0)
-        expect(terminalHtml).not.toContain(webCreateSettingsChoiceHint)
+        expect(terminalHtml.includes(renderedHelpLine(createSettingsHint))).toBe(step > 0)
+        expect(terminalHtml).not.toContain(renderedHelpLine(webCreateSettingsNavigationHint))
+        expect(terminalHtml).not.toContain(renderedHelpLine(webCreateSettingsChoiceHint))
         expect(terminalHtml).not.toContain("Enter = next, Esc = cancel.")
         expect(terminalHtml).not.toContain("Shift+Enter")
       })

packages/app/tests/docker-git/menu-create-display-settings.test.ts

@@ -2,6 +2,7 @@ import * as fc from "fast-check"
 import { describe, expect, it } from "vitest"
 
 import {
+  advanceCreateDisplaySettingsStep,
   applyCreateDisplaySettingsStep,
   completeCreateDisplaySettingsFlow,
   type DisplayModeFlowView,
@@ -82,6 +83,35 @@ describe("menu-create-shared display settings", () => {
     expect(resolveCreateDisplaySteps()[next.step]).toBe("mcpPlaywright")
   })
 
+  it("applies a browser display setting and advances to the next row", () => {
+    const next = expectDisplayModeView(expectCreateContinueView(advanceCreateDisplaySettingsStep(
+      cwd,
+      { ...createFlowViewAtStep(createFeatureRepoDisplaySettingsView(cwd), "mcpPlaywright"), buffer: "y" }
+    )))
+
+    expect(next.step).toBe(resolveCreateDisplaySteps().indexOf("force"))
+    expect(next.buffer).toBe("")
+    expect(next.values.enableMcpPlaywright).toBe(true)
+  })
+
+  it("preserves an existing setting value on empty Enter and wraps after the last row", () => {
+    const view = createFlowViewAtStep(createFeatureRepoDisplaySettingsView(cwd), "force", "")
+    const next = expectDisplayModeView(expectCreateContinueView(advanceCreateDisplaySettingsStep(
+      cwd,
+      {
+        ...view,
+        values: {
+          ...view.values,
+          force: true
+        }
+      }
+    )))
+
+    expect(next.step).toBe(resolveCreateDisplaySteps().indexOf("cpuLimit"))
+    expect(next.buffer).toBe("")
+    expect(next.values.force).toBe(true)
+  })
+
   it("navigates browser display settings without skipping applied rows", () => {
     const view = createFeatureRepoDisplaySettingsView(cwd)
     const applied = expectDisplayModeView(expectCreateContinueView(applyCreateDisplaySettingsStep(