Skip to content

persist the terminals when clicking around the app #287

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.

Sign up for GitHub

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

Merged
webdevcody merged 1 commit into from
Dec 28, 2025
Merged

persist the terminals when clicking around the app #287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions apps/ui/src/components/views/terminal-view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ export function TerminalView() {
const [dragOverTabId, setDragOverTabId] = useState<string | null>(null);
const lastCreateTimeRef = useRef<number>(0);
const isCreatingRef = useRef<boolean>(false);
const prevProjectPathRef = useRef<string | null>(null);
const restoringProjectPathRef = useRef<string | null>(null);
const [newSessionIds, setNewSessionIds] = useState<Set<string>>(new Set());
const [serverSessionInfo, setServerSessionInfo] = useState<{
Expand Down Expand Up @@ -524,11 +523,15 @@ export function TerminalView() {
}, [terminalState.isUnlocked, fetchServerSettings]);

// Handle project switching - save and restore terminal layouts
// Uses terminalState.lastActiveProjectPath (persisted in store) instead of a local ref
// This ensures terminals persist when navigating away from terminal route and back
useEffect(() => {
const currentPath = currentProject?.path || null;
const prevPath = prevProjectPathRef.current;
// Read lastActiveProjectPath directly from store to avoid dependency issues
const prevPath = useAppStore.getState().terminalState.lastActiveProjectPath;

// Skip if no change
// Skip if no change - this now correctly handles route changes within the same project
// because lastActiveProjectPath persists in the store across component unmount/remount
if (currentPath === prevPath) {
return;
}
Expand All @@ -538,12 +541,13 @@ export function TerminalView() {

// Save layout for previous project (if there was one and has terminals)
// BUT don't save if we were mid-restore for that project (would save incomplete state)
if (prevPath && terminalState.tabs.length > 0 && restoringProjectPathRef.current !== prevPath) {
const currentTabs = useAppStore.getState().terminalState.tabs;
if (prevPath && currentTabs.length > 0 && restoringProjectPathRef.current !== prevPath) {
saveTerminalLayout(prevPath);
}

// Update the previous project ref
prevProjectPathRef.current = currentPath;
// Update the stored project path
useAppStore.getState().setTerminalLastActiveProjectPath(currentPath);

// Helper to kill sessions and clear state
const killAndClear = async () => {
Expand Down
13 changes: 13 additions & 0 deletions apps/ui/src/store/app-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ export interface TerminalState {
scrollbackLines: number; // Number of lines to keep in scrollback buffer
lineHeight: number; // Line height multiplier for terminal text
maxSessions: number; // Maximum concurrent terminal sessions (server setting)
lastActiveProjectPath: string | null; // Last project path to detect route changes vs project switches
}

// Persisted terminal layout - now includes sessionIds for reconnection
Expand Down Expand Up @@ -816,6 +817,7 @@ export interface AppActions {
setTerminalScrollbackLines: (lines: number) => void;
setTerminalLineHeight: (lineHeight: number) => void;
setTerminalMaxSessions: (maxSessions: number) => void;
setTerminalLastActiveProjectPath: (projectPath: string | null) => void;
addTerminalTab: (name?: string) => string;
removeTerminalTab: (tabId: string) => void;
setActiveTerminalTab: (tabId: string) => void;
Expand Down Expand Up @@ -951,6 +953,7 @@ const initialState: AppState = {
scrollbackLines: 5000,
lineHeight: 1.0,
maxSessions: 100,
lastActiveProjectPath: null,
},
terminalLayoutByProject: {},
specCreatingForProject: null,
Expand Down Expand Up @@ -2037,6 +2040,8 @@ export const useAppStore = create<AppState & AppActions>()(
scrollbackLines: current.scrollbackLines,
lineHeight: current.lineHeight,
maxSessions: current.maxSessions,
// Preserve lastActiveProjectPath - it will be updated separately when needed
lastActiveProjectPath: current.lastActiveProjectPath,
},
});
},
Expand Down Expand Up @@ -2121,6 +2126,13 @@ export const useAppStore = create<AppState & AppActions>()(
});
},

setTerminalLastActiveProjectPath: (projectPath) => {
const current = get().terminalState;
set({
terminalState: { ...current, lastActiveProjectPath: projectPath },
});
},

addTerminalTab: (name) => {
const current = get().terminalState;
const newTabId = `tab-${Date.now()}`;
Expand Down Expand Up @@ -2669,6 +2681,7 @@ export const useAppStore = create<AppState & AppActions>()(
activeTabId: state.terminalState?.activeTabId ?? null,
activeSessionId: state.terminalState?.activeSessionId ?? null,
maximizedSessionId: state.terminalState?.maximizedSessionId ?? null,
lastActiveProjectPath: state.terminalState?.lastActiveProjectPath ?? null,
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This line appears to attempt restoring lastActiveProjectPath from persisted storage. However, the partialize function (around line 2698) doesn't include terminalState in the data saved to storage, meaning state.terminalState will be undefined during migration. Consequently, lastActiveProjectPath will always initialize to null, which is the correct and intended behavior for this session-only tracking feature. While this line is currently harmless, it could be misleading for future developers. Consider removing it to avoid confusion about what is being persisted.

// Restore persisted settings
defaultFontSize: persistedSettings.defaultFontSize ?? 14,
defaultRunScript: persistedSettings.defaultRunScript ?? '',
Expand Down