fix(app): address coderabbit follow-up review

Author: skulidropek

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

@@ -266,8 +266,9 @@ const resolveNextCreateFlowStep = (
     : currentStepIndex + 1
 
 const shouldCompleteCreateFlow = (
-  nextSteps: ReadonlyArray<CreateStep>
-): boolean => nextSteps.length <= firstCreateSettingsStepIndex
+  nextSteps: ReadonlyArray<CreateStep>,
+  nextStep: number
+): boolean => nextSteps.length <= firstCreateSettingsStepIndex || nextStep >= nextSteps.length
 
 /**
  * Advances create mode by applying the active prompt buffer.
@@ -281,10 +282,10 @@ const shouldCompleteCreateFlow = (
 // QUOTE(ТЗ): "after applying a non-repoUrl step it advances to currentStepIndex + 1"
 // REF: issue-339
 // SOURCE: n/a
-// FORMAT THEOREM: remaining = empty -> Complete, otherwise Continue(next valid setting)
+// FORMAT THEOREM: remaining = empty or nextStep past end -> Complete, otherwise Continue(next valid setting)
 // PURITY: CORE
 // EFFECT: n/a
-// INVARIANT: completion requires no unsatisfied settings after repoUrl
+// INVARIANT: applying the final settings index completes instead of clamping back to it
 // COMPLEXITY: O(k + s) where s = number of remaining create steps
 export const advanceCreateFlow = (
   contextOrCwd: string | CreateFlowContext,
@@ -307,12 +308,10 @@ export const advanceCreateFlow = (
 
       const nextSteps = resolveCreateFlowSteps(nextValues)
       const nextStep = resolveNextCreateFlowStep(step, view.step)
-      if (shouldCompleteCreateFlow(nextSteps)) {
+      if (shouldCompleteCreateFlow(nextSteps, nextStep)) {
         return completeCreateFlow(context, nextValues)
       }
-      return nextStep < nextSteps.length
-        ? continueCreateFlow(clampCreateSettingsStep(nextStep, nextSteps.length - 1), nextValues)
-        : continueCreateFlow(nextSteps.length - 1, nextValues)
+      return continueCreateFlow(clampCreateSettingsStep(nextStep, nextSteps.length - 1), nextValues)
     }
   )
 }

packages/app/src/web/actions-project-terminal.ts

@@ -176,10 +176,11 @@ const showPendingTerminalError = (
   error: string
 ): void => {
   const wasFinalized = runtime.pendingSessionFinalized
-  runtime.pendingSessionFinalized = true
-  if (!wasFinalized) {
-    runtime.lifecycle.onFailure?.(error)
+  if (wasFinalized) {
+    return
   }
+  runtime.pendingSessionFinalized = true
+  runtime.lifecycle.onFailure?.(error)
   appendOutputLine(context, `[error] ${error}`)
   context.addTerminalSession(renderPendingTerminalSession(context, runtime, error, "error"))
 }

packages/app/src/web/app-ready-ssh-link-hook.ts

@@ -17,11 +17,16 @@ import {
   type SshLinkRequest,
   sshLinkRequestKey
 } from "./app-ready-ssh-link-core.js"
-import { attachProjectWorkspaceSessions, showProjectTerminalScreen } from "./app-ready-ssh-link-terminal.js"
+import {
+  attachLoadedSshSessionLink,
+  attachProjectWorkspaceSessions,
+  type LoadedSshSessionLink,
+  showProjectTerminalScreen
+} from "./app-ready-ssh-link-terminal.js"
 import { browserMenuIndex } from "./menu.js"
 import { projectPickerScreen } from "./screen.js"
 import { terminalSessionId } from "./terminal-state.js"
-import { type ActiveTerminalSession, buildProjectActiveTerminalSession, projectSshRoutePath } from "./terminal.js"
+import type { ActiveTerminalSession } from "./terminal.js"
 
 export {
   readSshLinkRequestFromHref,
@@ -64,14 +69,53 @@ const clearPendingSshLink = (args: SshLinkEffectArgs, requestKey: string): void
   }
 }
 
+const isPendingSshLink = (args: SshLinkEffectArgs, requestKey: string): boolean =>
+  args.pendingTokenRef.current === requestKey
+
 const markSshLinkHandled = (args: SshLinkEffectArgs, requestKey: string): void => {
-  if (args.pendingTokenRef.current !== requestKey) {
+  if (!isPendingSshLink(args, requestKey)) {
     return
   }
   args.pendingTokenRef.current = null
   args.handledTokenRef.current = requestKey
 }
 
+const handleTerminalSessionAttachFailure = (
+  args: SshLinkEffectArgs,
+  requestKey: string,
+  sessionId: string,
+  error: string
+): void => {
+  if (!isPendingSshLink(args, requestKey)) {
+    return
+  }
+  const fallbackPath = resolveMissingSshSessionFallbackPath(globalThis.location.href, sessionId, error)
+  if (fallbackPath !== null) {
+    clearPendingSshLink(args, requestKey)
+    args.handledTokenRef.current = null
+    args.deactivateTerminalWorkspace()
+    args.actionContext.setSelectedMenuIndex(browserMenuIndex("Select"))
+    args.actionContext.setActiveScreen(projectPickerScreen())
+    globalThis.history.replaceState(globalThis.history.state, "", fallbackPath)
+    args.actionContext.setMessage(`SSH terminal is no longer available: ${sessionId}.`)
+    return
+  }
+  clearPendingSshLink(args, requestKey)
+  args.actionContext.setMessage(error)
+}
+
+const handleTerminalSessionAttachSuccess = (
+  args: SshLinkEffectArgs,
+  requestKey: string,
+  { projectDisplayName, projectKey, session }: LoadedSshSessionLink
+): void => {
+  if (!isPendingSshLink(args, requestKey)) {
+    return
+  }
+  markSshLinkHandled(args, requestKey)
+  attachLoadedSshSessionLink(args, { projectDisplayName, projectKey, session })
+}
+
 const scheduleTerminalSessionAttach = (
   args: SshLinkEffectArgs,
   requestKey: string,
@@ -84,37 +128,10 @@ const scheduleTerminalSessionAttach = (
       loadTerminalSessionById(sessionId).pipe(
         Effect.match({
           onFailure: (error) => {
-            const fallbackPath = resolveMissingSshSessionFallbackPath(globalThis.location.href, sessionId, error)
-            if (fallbackPath !== null) {
-              clearPendingSshLink(args, requestKey)
-              args.handledTokenRef.current = null
-              args.deactivateTerminalWorkspace()
-              args.actionContext.setSelectedMenuIndex(browserMenuIndex("Select"))
-              args.actionContext.setActiveScreen(projectPickerScreen())
-              globalThis.history.replaceState(globalThis.history.state, "", fallbackPath)
-              args.actionContext.setMessage(`SSH terminal is no longer available: ${sessionId}.`)
-              return
-            }
-            clearPendingSshLink(args, requestKey)
-            args.actionContext.setMessage(error)
+            handleTerminalSessionAttachFailure(args, requestKey, sessionId, error)
           },
-          onSuccess: ({ projectDisplayName, projectKey, session }) => {
-            markSshLinkHandled(args, requestKey)
-            globalThis.history.replaceState(
-              globalThis.history.state,
-              "",
-              projectSshRoutePath(projectKey, session.id)
-            )
-            showProjectTerminalScreen(args.actionContext, session.projectId)
-            args.addTerminalSession(buildProjectActiveTerminalSession({
-              onExit: args.actionContext.reloadDashboard,
-              onReady: args.actionContext.reloadDashboard,
-              projectDisplayName,
-              projectId: session.projectId,
-              projectKey,
-              session
-            }))
-            args.actionContext.setMessage(`Attached SSH terminal for ${projectDisplayName}.`)
+          onSuccess: (link) => {
+            handleTerminalSessionAttachSuccess(args, requestKey, link)
           }
         })
       )
@@ -157,10 +174,16 @@ const scheduleProjectTerminalAttach = (
       loadProjectTerminalWorkspace(project.projectKey).pipe(
         Effect.match({
           onFailure: (error) => {
+            if (!isPendingSshLink(args, requestKey)) {
+              return
+            }
             clearPendingSshLink(args, requestKey)
             args.actionContext.setMessage(error)
           },
           onSuccess: ({ activeSessionId, sessions }) => {
+            if (!isPendingSshLink(args, requestKey)) {
+              return
+            }
             const selectedSession = selectWorkspaceTerminalSession(sessions, activeSessionId, request.terminalId)
             if (selectedSession === null) {
               if (request.terminalId !== undefined) {

packages/app/src/web/app-ready-ssh-link-terminal.ts

@@ -3,7 +3,7 @@ import type { TerminalSession } from "./api-types.js"
 import type { DashboardProject } from "./app-ready-ssh-link-core.js"
 import { browserMenuIndex } from "./menu.js"
 import { projectPickerScreen } from "./screen.js"
-import { type ActiveTerminalSession, buildProjectActiveTerminalSession } from "./terminal.js"
+import { type ActiveTerminalSession, buildProjectActiveTerminalSession, projectSshRoutePath } from "./terminal.js"
 
 type ProjectTerminalAttachArgs = {
   readonly actionContext: Pick<
@@ -14,6 +14,44 @@ type ProjectTerminalAttachArgs = {
   readonly selectTerminalSession: (sessionId: string) => void
 }
 
+export type LoadedSshSessionLink = {
+  readonly projectDisplayName: string
+  readonly projectKey: string
+  readonly session: TerminalSession
+}
+
+type LoadedSshSessionAttachArgs = {
+  readonly actionContext: Pick<
+    BrowserActionContext,
+    "reloadDashboard" | "setActiveScreen" | "setMessage" | "setSelectedMenuIndex" | "setSelectedProjectId"
+  >
+  readonly addTerminalSession: (session: ActiveTerminalSession) => void
+}
+
+/**
+ * Opens the project terminal screen for a selected project.
+ *
+ * @param actionContext - Browser screen/menu setters.
+ * @param projectId - Project id to select.
+ * @returns Nothing; state is written through actionContext.
+ * @pure false
+ * @effect BrowserActionContext screen/menu selection setters.
+ * @invariant selected menu is Select and selected project id equals projectId.
+ * @precondition actionContext is live and projectId is non-empty.
+ * @postcondition project picker screen is active for projectId.
+ * @complexity O(1)
+ * @throws Never
+ */
+// CHANGE: keep SSH link screen selection in a focused shell helper
+// WHY: async link handlers need one shared mutation path for project terminal focus
+// QUOTE(ТЗ): n/a
+// REF: PR #342 CodeRabbit review
+// SOURCE: n/a
+// FORMAT THEOREM: focus(projectId) -> selectedProjectId = projectId
+// PURITY: SHELL
+// EFFECT: BrowserActionContext -> setSelectedMenuIndex, setActiveScreen, setSelectedProjectId
+// INVARIANT: menu Select and project picker screen are selected together
+// COMPLEXITY: O(1)
 export const showProjectTerminalScreen = (
   actionContext: ProjectTerminalAttachArgs["actionContext"],
   projectId: string
@@ -23,6 +61,72 @@ export const showProjectTerminalScreen = (
   actionContext.setSelectedProjectId(projectId)
 }
 
+/**
+ * Attaches a loaded legacy SSH session link to browser terminal state.
+ *
+ * @param args - Browser action sinks used for route, terminal, and message updates.
+ * @param link - Loaded terminal session plus project display metadata.
+ * @returns Nothing; route and terminal state are written through shell dependencies.
+ * @pure false
+ * @effect globalThis.history plus BrowserActionContext setters and addTerminalSession.
+ * @invariant The browser route points to the attached session id.
+ * @precondition link.session belongs to link.projectKey.
+ * @postcondition The project terminal screen is focused and the loaded session is added.
+ * @complexity O(1)
+ * @throws Never
+ */
+// CHANGE: isolate loaded legacy session attach side effects
+// WHY: stale-link guards in the hook should precede one compact shell transition
+// QUOTE(ТЗ): n/a
+// REF: PR #342 CodeRabbit review
+// SOURCE: n/a
+// FORMAT THEOREM: attach(link) -> route = projectSshRoutePath(link.projectKey, link.session.id)
+// PURITY: SHELL
+// EFFECT: History, BrowserActionContext, addTerminalSession
+// INVARIANT: added terminal session is built from the loaded backend session
+// COMPLEXITY: O(1)
+export const attachLoadedSshSessionLink = (
+  args: LoadedSshSessionAttachArgs,
+  { projectDisplayName, projectKey, session }: LoadedSshSessionLink
+): void => {
+  globalThis.history.replaceState(globalThis.history.state, "", projectSshRoutePath(projectKey, session.id))
+  showProjectTerminalScreen(args.actionContext, session.projectId)
+  args.addTerminalSession(buildProjectActiveTerminalSession({
+    onExit: args.actionContext.reloadDashboard,
+    onReady: args.actionContext.reloadDashboard,
+    projectDisplayName,
+    projectId: session.projectId,
+    projectKey,
+    session
+  }))
+  args.actionContext.setMessage(`Attached SSH terminal for ${projectDisplayName}.`)
+}
+
+/**
+ * Builds an active terminal session from a backend terminal session.
+ *
+ * @param args - Browser callbacks used by the terminal session lifecycle.
+ * @param project - Dashboard project owning the session.
+ * @param session - Backend terminal session to wrap.
+ * @returns Active terminal session ready for the browser tab list.
+ * @pure true
+ * @effect n/a
+ * @invariant session.projectId is preserved in the ActiveTerminalSession.
+ * @precondition project and session describe the same project.
+ * @postcondition onExit and onReady both reload the dashboard.
+ * @complexity O(1)
+ * @throws Never
+ */
+// CHANGE: isolate ActiveTerminalSession construction for SSH-link workspace attach
+// WHY: route attach and workspace attach must use identical terminal metadata
+// QUOTE(ТЗ): n/a
+// REF: PR #342 CodeRabbit review
+// SOURCE: n/a
+// FORMAT THEOREM: build(project, session).session = session
+// PURITY: CORE
+// EFFECT: n/a
+// INVARIANT: project display/key/id are copied without mutation
+// COMPLEXITY: O(1)
 const buildProjectTerminalSession = (
   args: ProjectTerminalAttachArgs,
   project: DashboardProject,
@@ -37,6 +141,32 @@ const buildProjectTerminalSession = (
     session
   })
 
+/**
+ * Adds workspace terminal sessions in created-at order and selects the requested one.
+ *
+ * @param args - Browser terminal-session sinks.
+ * @param project - Dashboard project owning the sessions.
+ * @param sessions - Workspace terminal sessions returned by the backend.
+ * @param selectedSession - Session that must become active after attach.
+ * @returns Nothing; terminal tabs and selection are written through args.
+ * @pure false
+ * @effect args.addTerminalSession and args.selectTerminalSession.
+ * @invariant selectedSession is added after all other sessions.
+ * @precondition selectedSession belongs to sessions.
+ * @postcondition args.selectTerminalSession is called with selectedSession.id.
+ * @complexity O(n log n) time and O(n) space where n = sessions.length.
+ * @throws Never
+ */
+// CHANGE: preserve deterministic workspace session attach ordering
+// WHY: opening selected session last keeps browser tab ordering stable while selecting the intended terminal
+// QUOTE(ТЗ): n/a
+// REF: PR #342 CodeRabbit review
+// SOURCE: n/a
+// FORMAT THEOREM: attach(sessions, selected) -> selectedSession.id is selected
+// PURITY: SHELL
+// EFFECT: ProjectTerminalAttachArgs -> addTerminalSession*, selectTerminalSession
+// INVARIANT: non-selected sessions are added by ascending createdAt before selectedSession
+// COMPLEXITY: O(n log n) time and O(n) space
 export const attachProjectWorkspaceSessions = (
   args: ProjectTerminalAttachArgs,
   project: DashboardProject,