fix(app): address coderabbit review findings

Author: skulidropek

packages/api/src/services/terminal-sessions.ts

@@ -765,8 +765,51 @@ const writePtyInput = (pty: PtyBridge | null, data: string): void => {
 
 const shellQuote = (value: string): string => `'${value.replace(/'/gu, "'\\''")}'`
 
+// CHANGE: Predicate for when tmux should forward right-click pane events.
+// WHY: Mouse-aware apps and copy/view mode still need pane mouse events, while tmux menus must stay disabled.
+// QUOTE(TZ): issue #340 right-click must not open the default tmux menu in browser terminals.
+// REF: PR #342 tmux right-click handling.
+// SOURCE: n/a
+// FORMAT THEOREM: mouse-aware-or-copy-mode => predicate evaluates truthy in tmux.
+// PURITY: CORE
+// EFFECT: none
+// INVARIANT: The predicate contains only tmux format language and no shell interpolation.
+// COMPLEXITY: O(1) time/O(1) space.
+/**
+ * Tmux format predicate used by right-click pane bindings.
+ *
+ * @returns A tmux format expression, not a shell command.
+ * @pure true
+ * @effect none
+ * @invariant Expression is constant and contains no user-controlled input.
+ * @precondition tmux understands mouse_any_flag and pane mode format variables.
+ * @postcondition The value is safe to embed after shellQuote.
+ * @complexity O(1) time/O(1) space.
+ * @throws Never
+ */
 const tmuxRightClickForwardPredicate =
   "#{||:#{mouse_any_flag},#{&&:#{pane_in_mode},#{?#{m/r:(copy|view)-mode,#{pane_mode}},0,1}}}"
+// CHANGE: Pane right-click bindings that are overridden at tmux startup.
+// WHY: These cover down/drag/up/end and Meta-modified events that previously reached display-menu.
+// QUOTE(TZ): issue #340 right-click must not open the default tmux menu in browser terminals.
+// REF: PR #342 tmux right-click handling.
+// SOURCE: n/a
+// FORMAT THEOREM: every binding in the array is mapped to renderTmuxPaneRightClickBinding.
+// PURITY: CORE
+// EFFECT: none
+// INVARIANT: Each entry is a static tmux root-table mouse binding name.
+// COMPLEXITY: O(1) time/O(1) space.
+/**
+ * Tmux pane right-click binding names that should conditionally forward mouse events.
+ *
+ * @pure true
+ * @effect none
+ * @invariant The array contains only static tmux binding identifiers.
+ * @precondition tmux root key table supports these binding names.
+ * @postcondition Consumers can map each entry to a shell-safe bind-key command.
+ * @complexity O(1) time/O(1) space.
+ * @throws Never
+ */
 const tmuxRightClickPaneBindings: ReadonlyArray<string> = [
   "MouseDown3Pane",
   "MouseDrag3Pane",
@@ -777,6 +820,27 @@ const tmuxRightClickPaneBindings: ReadonlyArray<string> = [
   "M-MouseDragEnd3Pane",
   "M-MouseUp3Pane"
 ]
+// CHANGE: Non-pane right-click bindings that are suppressed at tmux startup.
+// WHY: Status and border right-clicks are the tmux menu entry points that cannot be forwarded to pane apps.
+// QUOTE(TZ): issue #340 right-click must not open the default tmux menu in browser terminals.
+// REF: PR #342 tmux right-click handling.
+// SOURCE: n/a
+// FORMAT THEOREM: every binding in the array is mapped to renderTmuxRightClickSuppressBinding.
+// PURITY: CORE
+// EFFECT: none
+// INVARIANT: Each entry is a static tmux root-table mouse binding name.
+// COMPLEXITY: O(1) time/O(1) space.
+/**
+ * Tmux status/border right-click binding names that should be unbound.
+ *
+ * @pure true
+ * @effect none
+ * @invariant The array contains only static tmux binding identifiers.
+ * @precondition tmux root key table supports these binding names.
+ * @postcondition Consumers can map each entry to a shell-safe unbind-key command.
+ * @complexity O(1) time/O(1) space.
+ * @throws Never
+ */
 const tmuxRightClickSuppressBindings: ReadonlyArray<string> = [
   "MouseDown3Status",
   "MouseDown3StatusLeft",
@@ -788,14 +852,82 @@ const tmuxRightClickSuppressBindings: ReadonlyArray<string> = [
   "M-MouseDown3Border"
 ]
 
+// CHANGE: Render one tmux bind-key command for a right-click pane event.
+// WHY: Pane events must reach mouse-aware programs without allowing tmux display-menu.
+// QUOTE(TZ): issue #340 right-click must not open the default tmux menu in browser terminals.
+// REF: PR #342 tmux right-click handling.
+// SOURCE: n/a
+// FORMAT THEOREM: static binding => shellQuote(protected fragments) in result.
+// PURITY: CORE
+// EFFECT: none
+// INVARIANT: Dynamic shell fragments are emitted through shellQuote.
+// COMPLEXITY: O(1) time/O(1) space.
+/**
+ * Builds a tmux root-table command for a pane right-click binding.
+ *
+ * @param binding - Static tmux mouse binding name.
+ * @returns Shell command that binds the event to conditional pane forwarding.
+ * @pure true
+ * @effect none
+ * @invariant Shell-interpreted tmux format/action fragments are quoted.
+ * @precondition binding is one of tmuxRightClickPaneBindings.
+ * @postcondition The command exits successfully even when tmux rejects a binding.
+ * @complexity O(1) time/O(1) space.
+ * @throws Never
+ */
 const renderTmuxPaneRightClickBinding = (binding: string): string =>
   `tmux bind-key -T root ${binding} if-shell -F -t = ${shellQuote(tmuxRightClickForwardPredicate)} ${
     shellQuote("select-pane -t = ; send-keys -M")
   } >/dev/null 2>&1 || true`
 
+// CHANGE: Render one tmux unbind-key command for a suppressed right-click event.
+// WHY: Non-pane right-click targets are tmux UI affordances and should not open display-menu.
+// QUOTE(TZ): issue #340 right-click must not open the default tmux menu in browser terminals.
+// REF: PR #342 tmux right-click handling.
+// SOURCE: n/a
+// FORMAT THEOREM: static binding => deterministic unbind command.
+// PURITY: CORE
+// EFFECT: none
+// INVARIANT: Result contains no user-controlled input.
+// COMPLEXITY: O(1) time/O(1) space.
+/**
+ * Builds a tmux root-table command that suppresses a non-pane right-click binding.
+ *
+ * @param binding - Static tmux mouse binding name.
+ * @returns Shell command that unbinds the event and tolerates unsupported bindings.
+ * @pure true
+ * @effect none
+ * @invariant The returned command contains only static text plus binding.
+ * @precondition binding is one of tmuxRightClickSuppressBindings.
+ * @postcondition The command exits successfully even when the binding is absent.
+ * @complexity O(1) time/O(1) space.
+ * @throws Never
+ */
 const renderTmuxRightClickSuppressBinding = (binding: string): string =>
   `tmux unbind-key -T root ${binding} >/dev/null 2>&1 || true`
 
+// CHANGE: Aggregate all tmux right-click startup commands.
+// WHY: Terminal session startup needs one ordered command list for pane forwarding and UI suppression.
+// QUOTE(TZ): PR #342 preserves right-click copy while tmux mouse tracking is active.
+// REF: PR #342 tmux right-click handling.
+// SOURCE: n/a
+// FORMAT THEOREM: result length = paneBindings length + suppressBindings length.
+// PURITY: CORE
+// EFFECT: none
+// INVARIANT: Pane commands precede suppress commands.
+// COMPLEXITY: O(n) time/O(n) space where n is the total binding count.
+/**
+ * Renders the complete tmux right-click binding setup command list.
+ *
+ * @returns Readonly array of shell commands for tmux startup.
+ * @pure true
+ * @effect none
+ * @invariant Pane forwarding commands are emitted before suppressing status/border commands.
+ * @precondition Binding arrays contain static tmux binding identifiers.
+ * @postcondition The result contains one command per configured binding.
+ * @complexity O(n) time/O(n) space where n is total binding count.
+ * @throws Never
+ */
 const renderTmuxRightClickBindingCommands = (): ReadonlyArray<string> => [
   ...tmuxRightClickPaneBindings.map(renderTmuxPaneRightClickBinding),
   ...tmuxRightClickSuppressBindings.map(renderTmuxRightClickSuppressBinding)

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

@@ -19,6 +19,23 @@ import { applyCreateBufferToValues } from "./menu-create-step-apply.js"
 import { resolveCreateDisplaySteps, resolveCreateFlowSteps } from "./menu-create-steps.js"
 import type { CreateInputs, CreateStep } from "./menu-types.js"
 
+/**
+ * Creates the initial repo-url prompt view for the create flow.
+ *
+ * @pure true
+ * @invariant step = 0 and values are empty
+ * @complexity O(1)
+ */
+// CHANGE: expose a deterministic initial create-flow view constructor
+// WHY: CLI and browser callers need the same pure starting state
+// QUOTE(ТЗ): "fix CodeRabbit review comments"
+// REF: issue-339
+// SOURCE: n/a
+// FORMAT THEOREM: forall b: initial(b).step = 0
+// PURITY: CORE
+// EFFECT: n/a
+// INVARIANT: initial values contain no committed create inputs
+// COMPLEXITY: O(1)
 export const createInitialFlowView = (buffer = ""): CreateModeFlowView => ({
   mode: "create",
   step: 0,
@@ -37,6 +54,23 @@ const resolveDisplayFlowStep = (view: CreateFlowView): number => {
   return clampCreateSettingsStep(displayStep === -1 ? view.step : displayStep, displaySteps.length - 1)
 }
 
+/**
+ * Converts a create-flow view into the browser display-settings projection.
+ *
+ * @pure true
+ * @invariant values are preserved exactly
+ * @complexity O(s) where s = number of create steps
+ */
+// CHANGE: map create-mode progress onto browser display settings
+// WHY: display mode keeps all rows visible while preserving committed values
+// QUOTE(ТЗ): "fix CodeRabbit review comments"
+// REF: issue-339
+// SOURCE: n/a
+// FORMAT THEOREM: forall v: display(v).values = v.values
+// PURITY: CORE
+// EFFECT: n/a
+// INVARIANT: display step is clamped to the settings range
+// COMPLEXITY: O(s) where s = number of create steps
 export const createDisplayFlowView = (view: CreateFlowView): DisplayModeFlowView => ({
   mode: "display",
   step: resolveDisplayFlowStep(view),
@@ -130,6 +164,23 @@ const withActiveCreateDisplayContext = (
   return active === null ? null : onActive(active)
 }
 
+/**
+ * Applies the current browser display-settings row without moving selection.
+ *
+ * @pure true
+ * @invariant display mode remains display mode on Continue
+ * @complexity O(k) where k = number of stored create inputs
+ */
+// CHANGE: apply browser display settings through the shared pure step applicator
+// WHY: display mode must preserve row position while committing one decoded value
+// QUOTE(ТЗ): "fix CodeRabbit review comments"
+// REF: issue-339
+// SOURCE: n/a
+// FORMAT THEOREM: active(view) -> result in {Continue, Error}
+// PURITY: CORE
+// EFFECT: n/a
+// INVARIANT: inactive rows return null
+// COMPLEXITY: O(k) where k = number of stored create inputs
 export const applyCreateDisplaySettingsStep = (
   contextOrCwd: string | CreateFlowContext,
   view: DisplayModeFlowView
@@ -140,6 +191,23 @@ export const applyCreateDisplaySettingsStep = (
       (nextValues) => continueCreateDisplayFlow(view, nextValues)
     ))
 
+/**
+ * Applies the current browser display-settings row and advances one row.
+ *
+ * @pure true
+ * @invariant successful application clears the buffer
+ * @complexity O(k + s) where s = number of display steps
+ */
+// CHANGE: compose display setting application with wrapped display navigation
+// WHY: Enter in browser settings should commit the row and move to the next editable row
+// QUOTE(ТЗ): "fix CodeRabbit review comments"
+// REF: issue-339
+// SOURCE: n/a
+// FORMAT THEOREM: Continue(v) -> step(next(v)) = wrappedSuccessor(v.step)
+// PURITY: CORE
+// EFFECT: n/a
+// INVARIANT: errors do not advance selection
+// COMPLEXITY: O(k + s) where s = number of display steps
 export const advanceCreateDisplaySettingsStep = (
   contextOrCwd: string | CreateFlowContext,
   view: DisplayModeFlowView
@@ -153,6 +221,23 @@ export const advanceCreateDisplaySettingsStep = (
   return movedView === null ? applied : { ...applied, view: movedView }
 }
 
+/**
+ * Completes browser display settings, applying a non-empty active buffer first.
+ *
+ * @pure true
+ * @invariant completion resolves total CreateInputs
+ * @complexity O(k) where k = number of stored create inputs
+ */
+// CHANGE: finish browser settings with optional final-row validation
+// WHY: a typed buffer should not be discarded when the user presses Done
+// QUOTE(ТЗ): "fix CodeRabbit review comments"
+// REF: issue-339
+// SOURCE: n/a
+// FORMAT THEOREM: complete(view) -> Complete(resolve(values')) or Error
+// PURITY: CORE
+// EFFECT: n/a
+// INVARIANT: invalid active buffers return typed parse errors
+// COMPLEXITY: O(k) where k = number of stored create inputs
 export const completeCreateDisplaySettingsFlow = (
   contextOrCwd: string | CreateFlowContext,
   view: DisplayModeFlowView
@@ -179,8 +264,25 @@ const resolveNextCreateFlowStep = (
 ): number =>
   currentStep === "repoUrl"
     ? firstCreateSettingsStepIndex
-    : clampCreateSettingsStep(currentStepIndex, nextSteps.length - 1)
+    : clampCreateSettingsStep(currentStepIndex + 1, nextSteps.length - 1)
 
+/**
+ * Advances create mode by applying the active prompt buffer.
+ *
+ * @pure true
+ * @invariant non-repo steps advance to the next remaining settings index when continuing
+ * @complexity O(k + s) where s = number of remaining create steps
+ */
+// CHANGE: advance normal create-flow settings after committing the active prompt
+// WHY: applying a non-repo step must move forward instead of reselecting the same index
+// QUOTE(ТЗ): "after applying a non-repoUrl step it advances to currentStepIndex + 1"
+// REF: issue-339
+// SOURCE: n/a
+// FORMAT THEOREM: step != repoUrl -> nextStep = clamp(stepIndex + 1)
+// PURITY: CORE
+// EFFECT: n/a
+// INVARIANT: next step is always within the current settings range when continuing
+// COMPLEXITY: O(k + s) where s = number of remaining create steps
 export const advanceCreateFlow = (
   contextOrCwd: string | CreateFlowContext,
   view: CreateModeFlowView,
@@ -209,6 +311,23 @@ export const advanceCreateFlow = (
   )
 }
 
+/**
+ * Dispatches an advance result to imperative TUI handlers.
+ *
+ * @pure false
+ * @effect AdvanceCreateFlowHandlers
+ * @complexity O(1)
+ */
+// CHANGE: keep create-flow result handling at the shell boundary
+// WHY: pure transition results are interpreted by caller-provided side-effect handlers
+// QUOTE(ТЗ): "fix CodeRabbit review comments"
+// REF: issue-339
+// SOURCE: n/a
+// FORMAT THEOREM: forall r: exactly one matching handler is invoked, except null
+// PURITY: SHELL
+// EFFECT: AdvanceCreateFlowHandlers
+// INVARIANT: null results invoke no handler
+// COMPLEXITY: O(1)
 export const handleAdvanceCreateFlowResult = (
   next: AdvanceCreateFlowResult | null,
   handlers: AdvanceCreateFlowHandlers