-
Notifications
You must be signed in to change notification settings - Fork 27
🤖 refactor: simplify workspace creation and title generation #578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fixes two issues introduced in PR #500: 1. Runtime saving regression: The FirstMessageInput component wasn't persisting runtime selection to localStorage after workspace creation. Added runtime preference saving using updatePersistedState helper. 2. UI layout jank: Controls specific to new workspace creation (trunk branch selector and runtime selector) were mixed with general chat controls. Separated them into two rows for better visual organization. Also fixed: Use persistedState helpers (readPersistedState, updatePersistedState) instead of direct localStorage calls throughout FirstMessageInput for consistency and cross-component synchronization. Added note to AGENTS.md about using persistedState helpers and keeping edits minimal/token-efficient. _Generated with `cmux`_
- Merged FirstMessageInput into ChatInput using discriminated union - Created ChatInput/ directory with focused, modular components: - index.tsx: Main unified component - types.ts: Variant type definitions - useCreationWorkspace.ts: Creation state & logic hook - CreationCenterContent.tsx: Welcome screen component - CreationControls.tsx: Runtime & trunk branch selectors - utils.ts: Shared error message extraction - Eliminated logic duplication by reusing useSendMessageOptions - Single source of truth for chat input behavior - Strong typing prevents invalid prop combinations - Consistent UX between workspace and creation modes Net: -257 LOC (1273 → 1016), cleaner architecture
Creation variant needs 'flex h-full flex-1 flex-col' wrapper to: - Make center content vertically centered - Pin input section to bottom Uses conditional wrapper pattern to avoid affecting workspace variant.
Problem:
- Runtime config & trunk branch weren't persisted when creating workspace
- Settings lost when navigating away from draft workspace
- Piecemeal field-by-field persistence scattered across components
- Select elements had excessive vertical spacing (browser defaults)
Solution:
1. Created useDraftWorkspaceSettings hook - centralized persistence
- Model, mode, thinking level, use1M (synced from global state)
- Runtime mode, SSH host, trunk branch (project-scoped)
- All settings auto-persist via usePersistedState
- Single source of truth for draft workspace settings
2. Created reusable Select component
- Consistent styling across codebase
- Normalizes string[] or SelectOption[] inputs
- Replaces raw <select> elements to prevent style duplication
3. Updated useCreationWorkspace
- Removed useNewWorkspaceOptions (now part of useDraftWorkspaceSettings)
- Removed manual persistence logic (auto-persisted by hook)
- Settings restore when returning to same project
4. Updated CreationControls to use Select component
Storage keys:
- trunkBranch:{projectPath} - Trunk branch preference per project
- runtime:{projectPath} - Runtime config per project (existing)
- model:__project__{projectPath} - Model preference per project (existing)
+119 lines (useDraftWorkspaceSettings)
+52 lines (Select component)
Net: +163 LOC with much cleaner architecture
Bug: Focus was stolen by ChatInput during streaming, interrupting user interaction with other UI elements (Review tab, scrolling, etc.) Root cause: Auto-focus effect in commit 9731da3 changed dependency from [workspaceId, focusMessageInput] to [variant, props, focusMessageInput]. The 'props' object includes streaming-related fields (isCompacting, canInterrupt, editingMessage) which change frequently during streaming, causing the effect to re-run and steal focus. Fix: Extract workspaceId from props for dependency array, so effect only triggers when workspace actually changes, not on every props update. Before: [variant, props, focusMessageInput] ❌ Triggers on every render After: [variant, workspaceIdForFocus, focusMessageInput] ✅ Only on workspace change
Destructuring props in function signatures duplicates field names and makes refactoring more cumbersome. Using props.fieldName provides a single source of truth and easier maintenance. Also added guideline to AGENTS.md discouraging prop destructuring.
Changed padding from 8px horizontal, 4px vertical (px-2 py-1) to 4px horizontal, 2px vertical (px-1 py-0.5) for tighter UI spacing. Affects: - Select component (src/components/Select.tsx) - SSH host input in CreationControls
Fixes from code review: 1. Runtime mode coupling - use RuntimeMode type instead of typeof RUNTIME_MODE.LOCAL | typeof RUNTIME_MODE.SSH 2. Remove unnecessary prop destructuring in ChatInput - use props. notation for cleaner refactoring 3. Centralize Escape keybind - use KEYBINDS.CANCEL instead of hardcoded e.key === "Escape" Affected files: - CreationControls.tsx: Use RuntimeMode type - useCreationWorkspace.ts: Use RuntimeMode type - ChatInput/index.tsx: Remove destructuring, use KEYBINDS.CANCEL
Problem: Scope ID patterns like `__project__${projectPath}` were duplicated
across many files, creating maintenance burden and inconsistency.
Solution: Added storage key helpers in storage.ts:
- getProjectScopeId(projectPath) - Project-scoped preferences
- getPendingScopeId(projectPath) - Pending workspace input
- GLOBAL_SCOPE_ID constant - Global preferences
Updated all usages:
- ChatInput/index.tsx
- ChatInput/useCreationWorkspace.ts
- hooks/useDraftWorkspaceSettings.ts
- contexts/ModeContext.tsx
- contexts/ThinkingContext.tsx
Benefits:
- Single source of truth for scope ID format
- Consistent "/" delimiter handling
- Easier to modify scope ID format in future
- Clearer intent in code
Changed 'SSH Remote' to 'SSH' for cleaner, more concise UI. 'Remote' is redundant since SSH inherently implies remote connection. Updated: - CreationControls.tsx - NewWorkspaceModal.tsx
Problem: displayName added unnecessary complexity - two names for every workspace (displayName + name), causing confusion and duplication. Solution: Removed displayName entirely from codebase: 1. Types: Removed displayName from WorkspaceMetadata, WorkspaceEntry, WorkspaceMetadataSchema 2. Generator: generateWorkspaceNames() → generateWorkspaceName() - Only generates git-safe branch name (e.g., "feature-auth") - No separate title generation 3. Backend: Workspace creation uses single name field 4. Config: Removed displayName handling from updateWorkspaceMetadata() 5. UI: WorkspaceListItem shows workspace name directly 6. Title generation: Name regeneration updates name field only Benefits: - Simpler mental model - one name per workspace - Workspace name is git-safe branch name shown in UI - Less code duplication - Clearer semantics (name = what user sees) Note: displayName field remains in schema for backwards compatibility during config load, but is ignored (existing workspaces have both displayName and name, so name is used).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
When user selects SSH runtime without entering a host, the mode now persists so the SSH host input remains visible and editable. Previously buildRuntimeString returned undefined for empty host, causing mode to snap back to Local. Changes: - buildRuntimeString returns 'ssh' for SSH mode with empty host - parseRuntimeModeAndHost handles bare 'ssh' string - Added comprehensive tests for runtime parsing/building Fixes: Codex P1 comment on PR #578
Simplifies workspace creation flow by removing the displayName concept and centralizing draft workspace settings.
Key changes:
useDraftWorkspaceSettingshook that persists runtime mode, SSH host, and trunk branch when navigating away from draft workspace. Settings restore when returning to same project.getProjectScopeId,getPendingScopeId,GLOBAL_SCOPE_ID) to eliminate duplicate patterns across files.Backwards compatibility: Schema still accepts displayName during load (ignored). No migration needed.
Generated with
mux