From 43c4cc625fbe2d3fe3e2a02e6f032405652a04ee Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Thu, 27 Aug 2026 20:18:05 +0300 Subject: [PATCH] fix(ui): open saved plans against their owning project Saved Project knowledge plans opened as an empty editor whenever the viewer could not resolve the owning project from the current directory: managed chats (openchamber:chats is not a registered project), worktrees outside the repo path, and plan tabs restored after a reload. Titles still rendered because the list reads the manifest through the correct owner. - Thread the owner explicitly (savedProjectPlan = { projectRef, planId }) from the panel, mobile surfaces, and persisted context tabs; PlanView no longer guesses the project. - An unrecognized directory resolves to no owner instead of borrowing the active project's knowledge. - Serialize plan writes per document (planSaveQueue) so close/switch within the autosave debounce no longer drops the last edits, saves cannot land out of order, and a recovered save clears the error banner. - Send saved-plan contents inline in Improve/Implement prompts (they have no file path); disable those actions for managed-chat plans, which have no project directory to create a session in. - Drop persisted plan tabs that carry an id without an owner rather than reopening them against a guessed project. --- packages/ui/src/apps/MobileApp.tsx | 5 +- .../ui/src/apps/MobileWorkspaceDrawer.tsx | 3 +- .../ui/src/components/layout/ContextPanel.tsx | 7 +- .../components/layout/RightSidebarTabs.tsx | 48 ++- .../session/project-context/DOCUMENTATION.md | 12 + .../session/project-context/PlansSection.tsx | 17 +- .../project-context/ProjectNotesTodoPanel.tsx | 9 +- packages/ui/src/components/views/PlanView.tsx | 334 ++++++++++++++---- .../src/hooks/useProjectContextOwner.test.ts | 42 +++ .../ui/src/hooks/useProjectContextOwner.ts | 11 +- packages/ui/src/lib/planSaveQueue.test.ts | 142 ++++++++ packages/ui/src/lib/planSaveQueue.ts | 61 ++++ packages/ui/src/lib/projectContextApi.ts | 11 + .../stores/useUIStore.contextPanel.test.ts | 177 ++++++++++ packages/ui/src/stores/useUIStore.ts | 72 +++- 15 files changed, 826 insertions(+), 125 deletions(-) create mode 100644 packages/ui/src/lib/planSaveQueue.test.ts create mode 100644 packages/ui/src/lib/planSaveQueue.ts diff --git a/packages/ui/src/apps/MobileApp.tsx b/packages/ui/src/apps/MobileApp.tsx index 5c4a5079..9178e36b 100644 --- a/packages/ui/src/apps/MobileApp.tsx +++ b/packages/ui/src/apps/MobileApp.tsx @@ -22,6 +22,7 @@ import { useUpdatePolling } from '@/hooks/useUpdatePolling'; import { useWindowTitle } from '@/hooks/useWindowTitle'; import { opencodeClient } from '@/lib/opencode/client'; import type { RuntimeAPIs } from '@/lib/api/types'; +import type { ProjectRef } from '@/lib/projectContextApi'; import { readTabletLayout, useOrientation, useTabletLayout } from '@/lib/device'; import { useHardwareKeyboard } from '@/lib/hardwareKeyboard'; import { useI18n } from '@/lib/i18n'; @@ -111,7 +112,7 @@ const MobileShell: React.FC<{ onActiveConnectionDeleted: () => void }> = ({ onAc const [workspaceTab, setWorkspaceTab] = React.useState('changes'); // A plan opened from the workspace drawer's Notes tab, shown as a fullscreen // layer on top of it (back returns to the notes). - const [openPlan, setOpenPlan] = React.useState<{ id: string; title: string } | null>(null); + const [openPlan, setOpenPlan] = React.useState<{ id: string; title: string; projectRef: ProjectRef } | null>(null); const [settingsInitialMobileStage, setSettingsInitialMobileStage] = React.useState<'nav' | 'page-content'>('nav'); // When set, the Changes surface opens directly into the per-file diff for this path. const [pendingChangesDiff, setPendingChangesDiff] = React.useState<{ path: string; staged: boolean } | null>(null); @@ -542,7 +543,7 @@ const MobileShell: React.FC<{ onActiveConnectionDeleted: () => void }> = ({ onAc > { closeSurface(); closeWorkspace(); diff --git a/packages/ui/src/apps/MobileWorkspaceDrawer.tsx b/packages/ui/src/apps/MobileWorkspaceDrawer.tsx index f0847aa1..40a3649a 100644 --- a/packages/ui/src/apps/MobileWorkspaceDrawer.tsx +++ b/packages/ui/src/apps/MobileWorkspaceDrawer.tsx @@ -9,6 +9,7 @@ import { ErrorBoundary } from '@/components/ui/ErrorBoundary'; import { SortableTabsStrip, type SortableTabsStripItem } from '@/components/ui/sortable-tabs-strip'; import { TerminalView } from '@/components/views/TerminalView'; import { useI18n } from '@/lib/i18n'; +import type { ProjectRef } from '@/lib/projectContextApi'; import { cn } from '@/lib/utils'; import { useDirectoryStore } from '@/stores/useDirectoryStore'; import { useMcpConfigStore } from '@/stores/useMcpConfigStore'; @@ -105,7 +106,7 @@ export const MobileWorkspaceDrawer: React.FC<{ /** When set, the Changes tab opens directly into the per-file diff. */ pendingChangesDiff: { path: string; staged: boolean } | null; /** Notes tab: opens a plan fullscreen (layered above the drawer). */ - onOpenPlan: (plan: { id: string; title: string }) => void; + onOpenPlan: (plan: { id: string; title: string; projectRef: ProjectRef }) => void; /** MCP tab: jump to the MCP settings page pre-seeded with a new server draft. */ onOpenMcpSettings: () => void; variant?: 'drawer' | 'panel'; diff --git a/packages/ui/src/components/layout/ContextPanel.tsx b/packages/ui/src/components/layout/ContextPanel.tsx index aa52be5d..cd22371e 100644 --- a/packages/ui/src/components/layout/ContextPanel.tsx +++ b/packages/ui/src/components/layout/ContextPanel.tsx @@ -943,7 +943,12 @@ export const ContextPanel: React.FC = () => { : activeTab?.mode === 'notes' ? : activeTab?.mode === 'plan' - ? + ? : null; const browserTabs = React.useMemo( diff --git a/packages/ui/src/components/layout/RightSidebarTabs.tsx b/packages/ui/src/components/layout/RightSidebarTabs.tsx index 842e4b88..3907a341 100644 --- a/packages/ui/src/components/layout/RightSidebarTabs.tsx +++ b/packages/ui/src/components/layout/RightSidebarTabs.tsx @@ -6,43 +6,53 @@ import { useProjectsStore } from '@/stores/useProjectsStore'; import { useDirectoryStore } from '@/stores/useDirectoryStore'; import { formatDirectoryName } from '@/lib/utils'; import { useSessionUIStore } from '@/sync/session-ui-store'; -import { CHAT_DRAFT_PROJECT_ID } from '@/lib/chatDirectories'; -import { useI18n } from '@/lib/i18n'; import { useProjectContextOwner } from '@/hooks/useProjectContextOwner'; +import { CHAT_DRAFT_PROJECT_ID } from '@/lib/chatDirectories'; +import type { ProjectRef } from '@/lib/projectContextApi'; +import { useI18n } from '@/lib/i18n'; export const ProjectContextPanel: React.FC<{ onActionComplete?: () => void; - onOpenPlan?: (plan: { id: string; title: string }) => void; + onOpenPlan?: (plan: { id: string; title: string; projectRef: ProjectRef }) => void; }> = ({ onActionComplete, onOpenPlan }) => { - const projects = useProjectsStore((state) => state.projects); const homeDirectory = useDirectoryStore((state) => state.homeDirectory); const { t } = useI18n(); const gitDirectories = useGitStore((state) => state.directories); const chatSessionDirectory = useSessionUIStore((state) => state.currentSessionDirectory); - const projectRef = useProjectContextOwner(chatSessionDirectory); - const isChatContext = projectRef?.id === CHAT_DRAFT_PROJECT_ID; - const activeProject = React.useMemo(() => { - if (isChatContext) return null; - return projects.find((project) => project.id === projectRef?.id) ?? null; - }, [isChatContext, projectRef?.id, projects]); + // One owner decision shared with the panel, agent memory, and PlanView: + // chats resolve to the Chats owner, worktrees to their project, and an + // unrecognized directory owns nothing (null) rather than borrowing + // whichever project happens to be active. + const projectRef = useProjectContextOwner(chatSessionDirectory); + + // Display-only lookup: a user-renamed project label wins over the directory + // name. The owner decision stays with the hook — this must not reintroduce + // a fallback. + const projects = useProjectsStore((state) => state.projects); + const labeledProject = React.useMemo( + () => (projectRef ? projects.find((project) => project.id === projectRef.id) ?? null : null), + [projectRef, projects], + ); const projectLabel = React.useMemo(() => { - if (isChatContext) return t('sessions.sidebar.activity.chatsTitle'); - if (!activeProject) { + if (!projectRef) { return null; } - return activeProject.label?.trim() - || formatDirectoryName(activeProject.path, homeDirectory) - || activeProject.path; - }, [activeProject, homeDirectory, isChatContext, t]); + if (projectRef.id === CHAT_DRAFT_PROJECT_ID) { + return t('sessions.sidebar.activity.chatsTitle'); + } + return labeledProject?.label?.trim() + || formatDirectoryName(projectRef.path, homeDirectory) + || projectRef.path; + }, [homeDirectory, labeledProject, projectRef, t]); const canCreateWorktree = React.useMemo(() => { - if (!activeProject) { + if (!projectRef || projectRef.id === CHAT_DRAFT_PROJECT_ID) { return false; } - return gitDirectories.get(activeProject.path)?.isGitRepo === true; - }, [activeProject, gitDirectories]); + return gitDirectories.get(projectRef.path)?.isGitRepo === true; + }, [gitDirectories, projectRef]); return ( /* The panel scrolls its own tab content; a scroller here would nest. */ diff --git a/packages/ui/src/components/session/project-context/DOCUMENTATION.md b/packages/ui/src/components/session/project-context/DOCUMENTATION.md index d2e010ee..5e7819a4 100644 --- a/packages/ui/src/components/session/project-context/DOCUMENTATION.md +++ b/packages/ui/src/components/session/project-context/DOCUMENTATION.md @@ -60,6 +60,18 @@ Leaving the section or the project closes it, so its editor never sits over a list it no longer matches. Hosts that own a fullscreen plan surface (mobile) still pass `onOpenPlan` and keep theirs. +The panel owns the only source of truth for which project a plan belongs to, +and it never lets the editor guess. `PlanView` receives the owner as +`savedProjectPlan={{ projectRef, planId }}` — load and autosave both go to that +exact project. An earlier version let the editor re-derive the project from the +current directory, which silently opened an empty document for plans stored +under the managed Chats owner (`openchamber:chats`), for plans opened from a +worktree the directory lookup missed, and for plan tabs restored after a +reload. Persisted plan tabs carry `projectPlanRef` for the same reason; a saved-plan +tab persisted with an id but no owner is dropped on rehydrate rather than +reopened against a guessed project. A plain session plan tab legitimately has +neither an id nor an owner and is kept. + ## Pins belong to one session Notes and plans are project data, but attaching one writes its id to the current diff --git a/packages/ui/src/components/session/project-context/PlansSection.tsx b/packages/ui/src/components/session/project-context/PlansSection.tsx index a5d00fc1..d6c7f560 100644 --- a/packages/ui/src/components/session/project-context/PlansSection.tsx +++ b/packages/ui/src/components/session/project-context/PlansSection.tsx @@ -5,7 +5,7 @@ import { toast } from '@/components/ui'; import { Icon } from '@/components/icon/Icon'; import { requestFileAccess } from '@/lib/desktop'; import { getCurrentIntlLocale, useI18n } from '@/lib/i18n'; -import { parsePlanMarkdown, type ProjectPlanLink, type ProjectRef } from '@/lib/projectContextApi'; +import { parsePlanMarkdown, resolveProjectContextId, type ProjectPlanLink, type ProjectRef } from '@/lib/projectContextApi'; import { runtimeFetch } from '@/lib/runtime-fetch'; import { cn } from '@/lib/utils'; import { useDirectoryStore } from '@/stores/useDirectoryStore'; @@ -23,8 +23,9 @@ export const PlansSection: React.FC<{ plans: ProjectPlanLink[]; /** Panel-wide filter, matched against plan titles. */ query: string; - /** Hosts without a ContextPanel (mobile) render their own plan viewer. */ - onOpenPlan?: (plan: { id: string; title: string }) => void; + /** Hosts without a ContextPanel (mobile) render their own plan viewer. The + plan carries its owner so the host viewer never guesses the project. */ + onOpenPlan?: (plan: { id: string; title: string; projectRef: ProjectRef }) => void; pinnedPlanIds: ReadonlySet; onTogglePinned: (planId: string, pinned: boolean) => Promise; }> = ({ projectRef, plans, query, onOpenPlan, pinnedPlanIds, onTogglePinned }) => { @@ -155,7 +156,7 @@ export const PlansSection: React.FC<{ const handleOpenPlan = React.useCallback( (plan: ProjectPlanLink) => { if (onOpenPlan) { - onOpenPlan({ id: plan.id, title: plan.title }); + onOpenPlan({ id: plan.id, title: plan.title, projectRef }); return; } const panelDirectory = currentDirectory?.trim() || projectRef.path.trim(); @@ -165,11 +166,15 @@ export const PlansSection: React.FC<{ openContextPanelTab(panelDirectory, { mode: 'plan', projectPlanId: plan.id, - dedupeKey: `plan:${plan.id}`, + projectPlanRef: projectRef, + // Storage identity is derived from the project path, not the settings + // id, so the tab identity uses the same derivation. Two projects + // sharing a settings id but not a path must not merge plan tabs. + dedupeKey: `plan:${resolveProjectContextId(projectRef)}:${plan.id}`, label: plan.title, }); }, - [currentDirectory, onOpenPlan, openContextPanelTab, projectRef.path] + [currentDirectory, onOpenPlan, openContextPanelTab, projectRef] ); return ( diff --git a/packages/ui/src/components/session/project-context/ProjectNotesTodoPanel.tsx b/packages/ui/src/components/session/project-context/ProjectNotesTodoPanel.tsx index 72b9fd20..9fad4bb6 100644 --- a/packages/ui/src/components/session/project-context/ProjectNotesTodoPanel.tsx +++ b/packages/ui/src/components/session/project-context/ProjectNotesTodoPanel.tsx @@ -29,8 +29,9 @@ interface ProjectNotesTodoPanelProps { canCreateWorktree?: boolean; onActionComplete?: () => void; /** When provided, opening a plan calls this instead of the desktop context - panel tab — hosts without ContextPanel (mobile) render their own viewer. */ - onOpenPlan?: (plan: { id: string; title: string }) => void; + panel tab — hosts without ContextPanel (mobile) render their own viewer. + The plan carries its owner so the host's viewer cannot guess wrong. */ + onOpenPlan?: (plan: { id: string; title: string; projectRef: ProjectRef }) => void; className?: string; } @@ -501,10 +502,10 @@ export const ProjectNotesTodoPanel: React.FC = ({ /> ) : null} - {activeTab === 'plans' && openPlan ? ( + {activeTab === 'plans' && openPlan && projectRef ? ( setOpenPlan(null)} /> diff --git a/packages/ui/src/components/views/PlanView.tsx b/packages/ui/src/components/views/PlanView.tsx index 24c07aaa..a1e9793e 100644 --- a/packages/ui/src/components/views/PlanView.tsx +++ b/packages/ui/src/components/views/PlanView.tsx @@ -38,7 +38,10 @@ import { useEffectiveDirectory } from '@/hooks/useEffectiveDirectory'; import { EditorView } from '@codemirror/view'; import { copyTextToClipboard } from '@/lib/clipboard'; import { generateBranchName } from '@/lib/git/branchNameGenerator'; -import { fetchProjectPlan, parsePlanMarkdown } from '@/lib/projectContextApi'; +import { fetchProjectPlan, parsePlanMarkdown, resolveProjectContextId, type SavedProjectPlanTarget } from '@/lib/projectContextApi'; +import { CHAT_DRAFT_PROJECT_ID } from '@/lib/chatDirectories'; +import { createPlanSaveQueue } from '@/lib/planSaveQueue'; +import { getRuntimeKey, subscribeRuntimeEndpointChanged } from '@/lib/runtime-switch'; import { useProjectContextStore } from '@/stores/useProjectContextStore'; import { createWorktreeSessionForNewBranch } from '@/lib/worktreeSessionCreator'; import { TodoSendDialog, type TodoSendExecution } from '@/components/session/TodoSendDialog'; @@ -49,9 +52,12 @@ import { useI18n } from '@/lib/i18n'; type PlanViewProps = { targetPath?: string | null; - /** Saved project plan to open. Project plans are server-owned and addressed - by id; they never carry a client-visible filesystem path. */ - projectPlanId?: string | null; + /** Saved project plan to open, with the project that owns it. The owner is + part of the prop so the view never guesses it from the current directory: + plan tabs outlive directory changes (persisted context tabs, mobile + overlays), and for managed chats the owner is not a registered project a + directory lookup could ever find. */ + savedProjectPlan?: SavedProjectPlanTarget | null; /** Called after a send action routes the user to the chat — hosts that show PlanView in an overlay (mobile fullscreen surface) close it here. */ onNavigatedToChat?: () => void; @@ -149,12 +155,16 @@ const resolveProjectRefForDirectory = ( return match ? { id: match.id, path: match.path } : null; }; +const subscribeActiveRuntimeKey = (onStoreChange: () => void): (() => void) => { + return subscribeRuntimeEndpointChanged(() => onStoreChange()); +}; + type SelectedLineRange = { start: number; end: number; }; -export const PlanView: React.FC = ({ targetPath = null, projectPlanId = null, onNavigatedToChat }) => { +export const PlanView: React.FC = ({ targetPath = null, savedProjectPlan = null, onNavigatedToChat }) => { const { t } = useI18n(); const currentSessionId = useSessionUIStore((state) => state.currentSessionId); const createSession = useSessionUIStore((state) => state.createSession); @@ -170,6 +180,7 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl const effectiveDirectory = useEffectiveDirectory() ?? ''; const setSessionSwitcherOpen = useUIStore((state) => state.setSessionSwitcherOpen); const runtimeApis = useRuntimeAPIs(); + const activeRuntimeKey = React.useSyncExternalStore(subscribeActiveRuntimeKey, getRuntimeKey, getRuntimeKey); const { isMobile } = useDeviceInfo(); const { currentTheme } = useThemeSystem(); @@ -190,9 +201,37 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl () => resolveProjectRefForDirectory(projectDirectory, projects, activeProjectId), [activeProjectId, projectDirectory, projects], ); + // Destructured to primitives so the load/save effects key on stable values + // instead of a descriptor object rebuilt on every parent render. + const savedPlanProjectId = savedProjectPlan?.projectRef.id ?? null; + const savedPlanProjectPath = savedProjectPlan?.projectRef.path ?? null; + const savedPlanProjectRef = React.useMemo( + () => savedPlanProjectId && savedPlanProjectPath + ? { id: savedPlanProjectId, path: savedPlanProjectPath } + : null, + [savedPlanProjectId, savedPlanProjectPath], + ); + const savedPlanId = savedProjectPlan?.planId ?? null; + // Stable logical identity, composed from primitives: an effect keyed on the + // descriptor object would reload — and flush — the same plan whenever a + // parent rebuilds the owner object with identical values. + const savedPlanKey = savedPlanProjectRef && savedPlanId + ? JSON.stringify(['saved-plan', activeRuntimeKey, resolveProjectContextId(savedPlanProjectRef), savedPlanId]) + : null; + // Managed chats have no project directory to create a session in: their + // sessions live in per-session directories under the chats root, which + // createSession cannot prepare. Until a managed-chat send path exists, + // Improve/Implement stay unavailable for plans stored under the Chats + // owner — an OpenCode session created directly in the shared root would + // break the managed-chats model. + const isManagedChatPlan = savedPlanProjectRef?.id === CHAT_DRAFT_PROJECT_ID; const canCreateWorktree = React.useMemo( - () => (currentProjectRef ? gitDirectories.get(currentProjectRef.path)?.isGitRepo === true : false), - [currentProjectRef, gitDirectories], + () => { + // Worktree creation follows the session the plan would be sent to. + const sendTarget = savedPlanProjectRef ?? currentProjectRef; + return sendTarget ? gitDirectories.get(sendTarget.path)?.isGitRepo === true : false; + }, + [currentProjectRef, gitDirectories, savedPlanProjectRef], ); const [pendingPlanSend, setPendingPlanSend] = React.useState(null); const [isPlanSendSubmitting, setIsPlanSendSubmitting] = React.useState(false); @@ -202,7 +241,6 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl // `resolvedPath` so nothing downstream can mistake a project plan for a file // the user could open, edit, or be shown a path for. const [loadedProjectPlanId, setLoadedProjectPlanId] = React.useState(null); - const savePlan = useProjectContextStore((state) => state.savePlan); const hasDocument = Boolean(resolvedPath) || Boolean(loadedProjectPlanId); const displayPath = React.useMemo(() => { if (!resolvedPath || !sessionDirectory || !homeDirectory) { @@ -214,6 +252,7 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl const { isPlaying: isTTSPlaying, play: playTTS, stop: stopTTS } = useMessageTTS(); const showMessageTTSButtons = useConfigStore((state) => state.showMessageTTSButtons); const [saveError, setSaveError] = React.useState(null); + const [loadError, setLoadError] = React.useState(null); const planFileLabel = React.useMemo(() => { return displayPath ? displayPath.split('/').pop() || t('planView.file.defaultName') : t('planView.file.defaultName'); }, [displayPath, t]); @@ -381,9 +420,96 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl return extensions; }, [currentTheme, resolvedPath, editorFontSize]); + // Pending-save bookkeeping for the open document. One ref record, not state: + // debounced writes and close-time flushes must read the newest buffer and + // revision without another render. `editRevision` advances on every editor + // change; `savedRevision` only after a successful write of that exact + // revision, so a slow in-flight save can never mark newer edits as saved. + // `key` and `runtimeKey` make every write self-identifying: content never + // crosses documents or runtimes, no matter when a queued write settles. + const docRef = React.useRef<{ + key: string | null; + target: SavedProjectPlanTarget | { filePath: string } | null; + content: string; + editRevision: number; + savedRevision: number; + runtimeKey: string; + }>({ key: null, target: null, content: '', editRevision: 0, savedRevision: 0, runtimeKey: '' }); + const saveQueue = React.useState(createPlanSaveQueue)[0]; + + // Filesystem writes keep the runtime adapter precedence the view always + // used: the active RuntimeAPIs first, the registry as fallback. + const writeDocument = React.useCallback(async (target: NonNullable, text: string): Promise => { + if ('filePath' in target) { + const files = runtimeApis.files ?? getRegisteredRuntimeAPIs()?.files; + if (files?.writeFile) { + const result = await files.writeFile(target.filePath, text); + if (!result?.success) { + throw new Error('Plan file write failed'); + } + return; + } + const response = await runtimeFetch('/api/fs/write', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ path: target.filePath, content: text }), + }); + if (!response.ok) { + throw new Error(`Failed to write plan file (${response.status})`); + } + return; + } + const saved = await useProjectContextStore.getState().savePlan(target.projectRef, target.planId, text); + if (!saved) { + throw new Error('Plan save rejected: the plan no longer exists'); + } + }, [runtimeApis.files]); + const writeDocumentRef = React.useRef(writeDocument); + writeDocumentRef.current = writeDocument; + + // Queue any unflushed edits. Runs on document switches and on unmount, both + // of which cancel the debounced save — without this the last 350ms of typing + // is silently dropped. The queue orders it behind any write already in + // flight for the same document, and the captured runtime key stops content + // from one host being written into another after a runtime switch. + const scheduleSave = React.useCallback(() => { + const doc = docRef.current; + if (!doc.key || !doc.target || doc.editRevision <= doc.savedRevision) { + return; + } + const captured = { + key: doc.key, + target: doc.target, + content: doc.content, + revision: doc.editRevision, + runtimeKey: doc.runtimeKey, + write: writeDocumentRef.current, + }; + saveQueue.schedule(captured.key, captured.revision, async () => { + if (getRuntimeKey() !== captured.runtimeKey) { + // The runtime switched while this write waited: writing through the + // new connection would land one host's edits on another. + return; + } + await captured.write(captured.target, captured.content); + const current = docRef.current; + if (current.key === captured.key) { + current.savedRevision = Math.max(current.savedRevision, captured.revision); + // A recovered save clears the stale failure banner. + setSaveError(null); + } + }).catch((error) => { + if (docRef.current.key === captured.key) { + setSaveError(error instanceof Error ? error.message : 'Plan save failed'); + } + }); + }, [saveQueue]); + React.useEffect(() => { // Saved project plans opened via context panel should work even when session plan mode is off. - if (!planModeEnabled && !targetPath && !projectPlanId) { + if (!planModeEnabled && !targetPath && !savedPlanId) { + scheduleSave(); + docRef.current = { key: null, target: null, content: '', editRevision: 0, savedRevision: 0, runtimeKey: '' }; setResolvedPath(null); setLoadedProjectPlanId(null); setContent(''); @@ -416,31 +542,49 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl }; const run = async () => { + // Flush the outgoing document before the bookkeeping is replaced, so + // edits typed within the debounce window survive a plan switch. React + // reuses this component instance across saved-plan tabs. + scheduleSave(); + docRef.current = { key: null, target: null, content: '', editRevision: 0, savedRevision: 0, runtimeKey: '' }; setResolvedPath(null); setLoadedProjectPlanId(null); setContent(''); setSaveError(null); + setLoadError(null); - if (projectPlanId) { - if (!currentProjectRef) { - return; - } + if (savedPlanId && savedPlanProjectRef && savedPlanKey) { + // A plan re-opened while its own flush is still writing must read the + // post-write state, not race it. The queue reset afterwards is safe: + // every write for this key has settled, and the reloaded document + // restarts its revision counter at zero. + await saveQueue.pendingFor(savedPlanKey); + if (cancelled) return; + saveQueue.reset(savedPlanKey); setLoading(true); try { - const plan = await fetchProjectPlan(currentProjectRef, projectPlanId); + const plan = await fetchProjectPlan(savedPlanProjectRef, savedPlanId); if (cancelled) return; if (!plan) { // The plan or its markdown is gone. Leave the view empty and // unsaveable rather than presenting an editor that would recreate // a document the user deleted. - setSaveError(t('planView.error.loadFailed')); + setLoadError('Plan not found'); return; } + docRef.current = { + key: savedPlanKey, + target: { projectRef: savedPlanProjectRef, planId: savedPlanId }, + content: plan.raw, + editRevision: 0, + savedRevision: 0, + runtimeKey: activeRuntimeKey, + }; setContent(plan.raw); - setLoadedProjectPlanId(projectPlanId); + setLoadedProjectPlanId(savedPlanId); } catch (error) { if (cancelled) return; - setSaveError(error instanceof Error ? error.message : t('planView.error.loadFailed')); + setLoadError(error instanceof Error ? error.message : 'Plan load failed'); } finally { if (!cancelled) setLoading(false); } @@ -448,10 +592,22 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl } if (targetPath) { + const fileKey = JSON.stringify(['plan-file', activeRuntimeKey, targetPath]); + await saveQueue.pendingFor(fileKey); + if (cancelled) return; + saveQueue.reset(fileKey); setLoading(true); try { const text = await readText(targetPath); if (cancelled) return; + docRef.current = { + key: fileKey, + target: { filePath: targetPath }, + content: text, + editRevision: 0, + savedRevision: 0, + runtimeKey: activeRuntimeKey, + }; setResolvedPath(targetPath); setContent(text); } catch { @@ -477,10 +633,9 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl const homePath = resolveTilde(buildHomePlanPath(session.time.created, session.slug), homeDirectory || null); let resolved: string | null = null; - let text: string | null = null; try { - text = await readText(repoPath); + await readText(repoPath); resolved = repoPath; } catch { // ignore @@ -488,7 +643,7 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl if (!resolved) { try { - text = await readText(homePath); + await readText(homePath); resolved = homePath; } catch { // ignore @@ -497,12 +652,26 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl if (cancelled) return; - if (!resolved || text === null) { + if (!resolved) { setResolvedPath(null); setContent(''); return; } + const sessionFileKey = JSON.stringify(['plan-file', activeRuntimeKey, resolved]); + await saveQueue.pendingFor(sessionFileKey); + if (cancelled) return; + const text = await readText(resolved); + if (cancelled) return; + saveQueue.reset(sessionFileKey); + docRef.current = { + key: sessionFileKey, + target: { filePath: resolved }, + content: text, + editRevision: 0, + savedRevision: 0, + runtimeKey: activeRuntimeKey, + }; setResolvedPath(resolved); setContent(text); } catch { @@ -519,55 +688,42 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl return () => { cancelled = true; }; - }, [currentProjectRef, homeDirectory, planModeEnabled, projectPlanId, runtimeApis.files, sessionDirectory, session?.slug, session?.time?.created, t, targetPath]); + }, [activeRuntimeKey, homeDirectory, planModeEnabled, runtimeApis.files, savedPlanId, savedPlanKey, savedPlanProjectRef, saveQueue, scheduleSave, session?.slug, session?.time?.created, sessionDirectory, targetPath]); + // Synchronous buffer tracking: if an edit and an unmount land in the same + // batch, the passive content effect would never run and a flush would save + // a stale buffer. + const handleContentChange = React.useCallback((next: string) => { + docRef.current.content = next; + docRef.current.editRevision += 1; + setContent(next); + }, []); + + // The debounced write and the close/switch flush go through the same queue + // (scheduleSave), so two saves of one document can never complete out of + // order and a flush never duplicates a debounce of the same revision. React.useEffect(() => { if (!resolvedPath && !loadedProjectPlanId) { return; } - const controller = window.setTimeout(async () => { - setSaveError(null); - try { - if (loadedProjectPlanId) { - if (!currentProjectRef) { - throw new Error(t('planView.error.writeFailed')); - } - const saved = await savePlan(currentProjectRef, loadedProjectPlanId, content); - if (!saved) { - throw new Error(t('planView.error.writeFailed')); - } - return; - } - - if (!resolvedPath) { - return; - } - - if (runtimeApis.files?.writeFile) { - const result = await runtimeApis.files.writeFile(resolvedPath, content); - if (!result?.success) { - throw new Error(t('planView.error.writeFailed')); - } - } else { - const response = await runtimeFetch('/api/fs/write', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ path: resolvedPath, content }), - }); - if (!response.ok) { - throw new Error(t('planView.error.writePlanFileFailed', { status: response.status })); - } - } - } catch (error) { - setSaveError(error instanceof Error ? error.message : t('planView.error.saveFailed')); - } + const controller = window.setTimeout(() => { + scheduleSave(); }, 350); return () => { window.clearTimeout(controller); }; - }, [content, currentProjectRef, loadedProjectPlanId, resolvedPath, runtimeApis.files, savePlan, t]); + }, [content, loadedProjectPlanId, resolvedPath, scheduleSave]); + + // Closing the view inside the 350ms debounce window would drop the last + // edits: the cleanup above cancels the timer. Same for switching documents, + // which the load effect handles before replacing the bookkeeping. + React.useEffect(() => { + return () => { + scheduleSave(); + }; + }, [scheduleSave]); React.useEffect(() => { return () => { @@ -584,7 +740,11 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl const handleConfirmPlanSend = React.useCallback( async (execution: TodoSendExecution) => { - if (!currentProjectRef || !pendingPlanSend) { + // A saved plan sends against its own project — the one it is stored + // under — not against whatever directory the viewer is currently in. + // For filesystem plans those are the same directory. + const sendTargetProject = savedPlanProjectRef ?? currentProjectRef; + if (!sendTargetProject || !pendingPlanSend || isManagedChatPlan) { return; } @@ -601,32 +761,45 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl plan_path: resolvedPath ?? '', }, ); - const syntheticParts = [{ synthetic: true as const, text: instructionsText }]; + // Saved project plans have no file path for the agent to read. Without + // this the instructions say "read that file" with an empty path and the + // plan contents never reach the session, so the plan substance rides + // along in the synthetic message instead. + const planSubstance = resolvedPath + ? instructionsText + : [ + instructionsText, + '', + 'The plan is not stored as a file in the repository and has no file path. Its full current contents follow below this note and are the source of truth for the plan. Where the instructions above refer to the plan file, treat the plan as stored in OpenChamber project knowledge (it is edited through the OpenChamber UI): propose plan revisions as plan text in the chat rather than editing a file.', + '', + content, + ].join('\n'); + const syntheticParts = [{ synthetic: true as const, text: planSubstance }]; setIsPlanSendSubmitting(true); try { routeToChat(); let sessionId: string | null = null; - let directoryHint: string | null = currentProjectRef.path; + let directoryHint: string | null = sendTargetProject.path; if (pendingPlanSend.target === 'worktree') { if (!canCreateWorktree) { return; } - const created = await createWorktreeSessionForNewBranch(currentProjectRef.path, generateBranchName()); + const created = await createWorktreeSessionForNewBranch(sendTargetProject.path, generateBranchName()); if (!created?.id) { return; } sessionId = created.id; directoryHint = created.path; } else { - const sessionResult = await createSession(undefined, currentProjectRef.path, null); + const sessionResult = await createSession(undefined, sendTargetProject.path, null); if (!sessionResult?.id) { return; } sessionId = sessionResult.id; - directoryHint = sessionResult.directory ?? currentProjectRef.path; + directoryHint = sessionResult.directory ?? sendTargetProject.path; initializeNewOpenChamberSession(sessionResult.id, useConfigStore.getState().agents ?? []); } @@ -664,8 +837,10 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl // source. Here we only compose header + full content. const goalObjective = execution.runAsGoal === true ? [ - `Implement the plan "${sendPromptTitle}" end-to-end${resolvedPath ? ` (plan file: ${resolvedPath})` : ''}.`, - 'Re-read that file for full details — it is the source of truth.', + `Implement the plan "${sendPromptTitle}" end-to-end${resolvedPath ? ` (plan file: ${resolvedPath})` : ' (the full plan follows)'}.`, + resolvedPath + ? 'Re-read that file for full details — it is the source of truth.' + : 'The full plan follows in this message and is the source of truth.', '', content, ].join('\n') @@ -687,7 +862,7 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl setIsPlanSendSubmitting(false); } }, - [canCreateWorktree, content, createSession, currentProjectRef, initializeNewOpenChamberSession, pendingPlanSend, resolvedPath, routeToChat, sendMessage, sendPromptTitle, setCurrentSession] + [canCreateWorktree, content, createSession, currentProjectRef, initializeNewOpenChamberSession, isManagedChatPlan, pendingPlanSend, resolvedPath, routeToChat, savedPlanProjectRef, sendMessage, sendPromptTitle, setCurrentSession] ); const blockWidgets = React.useMemo(() => { @@ -716,6 +891,11 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl
{parsedTitle}
+ {loadError ? ( +
+ {t('planView.error.loadFailed')} +
+ ) : null} {saveError ? (
{t('planView.error.saveFailed')} @@ -733,7 +913,7 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl size="sm" className="h-5 w-5 p-0" aria-label={t('planView.actions.improvePlanAria')} - disabled={!content.trim()} + disabled={!content.trim() || isManagedChatPlan} > @@ -742,7 +922,10 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl {t('planView.actions.improve')} - setPendingPlanSend({ action: 'improve', target: 'session' })}> + setPendingPlanSend({ action: 'improve', target: 'session' })} + disabled={isManagedChatPlan} + > {t('planView.actions.sendToNewSession')} = ({ targetPath = null, projectPl size="sm" className="h-5 w-5 p-0" aria-label={t('planView.actions.implementPlanAria')} - disabled={!content.trim()} + disabled={!content.trim() || isManagedChatPlan} > @@ -771,7 +954,10 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl {t('planView.actions.implement')} - setPendingPlanSend({ action: 'implement', target: 'session' })}> + setPendingPlanSend({ action: 'implement', target: 'session' })} + disabled={isManagedChatPlan} + > {t('planView.actions.sendToNewSession')} = ({ targetPath = null, projectPl } }} target={pendingPlanSend?.target ?? 'session'} - projectDirectory={currentProjectRef?.path ?? null} + projectDirectory={savedPlanProjectRef?.path ?? currentProjectRef?.path ?? null} submitting={isPlanSendSubmitting} allowRunAsGoal onConfirm={handleConfirmPlanSend} @@ -885,7 +1071,7 @@ export const PlanView: React.FC = ({ targetPath = null, projectPl
{ expect(owner).toEqual({ id: 'openchamber', path: '/workspace/openchamber' }); }); + + test('returns null for a recognized directory that owns nothing, instead of borrowing the active project', () => { + const owner = resolveProjectContextOwner({ + projects, + worktreesByProject: new Map(), + directory: '/some/other/project', + activeProjectId: 'openchamber', + chatDraftOpen: false, + chatDraftTarget: 'project', + homeDirectory: '/Users/test', + }); + + expect(owner).toBeNull(); + }); + + test('falls back to the active project only when there is no directory at all', () => { + const owner = resolveProjectContextOwner({ + projects, + worktreesByProject: new Map(), + directory: null, + activeProjectId: 'openchamber', + chatDraftOpen: false, + chatDraftTarget: 'project', + homeDirectory: '/Users/test', + }); + + expect(owner).toEqual({ id: 'openchamber', path: '/workspace/openchamber' }); + }); + + test('never falls back to the first project when the active project is unknown', () => { + const owner = resolveProjectContextOwner({ + projects, + worktreesByProject: new Map(), + directory: null, + activeProjectId: 'missing-project', + chatDraftOpen: false, + chatDraftTarget: 'project', + homeDirectory: '/Users/test', + }); + + expect(owner).toBeNull(); + }); }); diff --git a/packages/ui/src/hooks/useProjectContextOwner.ts b/packages/ui/src/hooks/useProjectContextOwner.ts index 4b9faac1..bc003020 100644 --- a/packages/ui/src/hooks/useProjectContextOwner.ts +++ b/packages/ui/src/hooks/useProjectContextOwner.ts @@ -47,7 +47,16 @@ export const resolveProjectContextOwner = ({ return { id: sessionProject.id, path: sessionProject.path }; } - const activeProject = projects.find((project) => project.id === activeProjectId) ?? projects[0] ?? null; + // A concrete directory that resolves to nothing owns nothing. Falling back + // to the active project here showed one project's knowledge under another + // project's name (the "plans open empty" bug), so the panel stays empty + // instead of lying. The active-project fallback is only for states with no + // directory at all, such as a new-session draft that has not landed yet. + if (normalizedDirectory) { + return null; + } + + const activeProject = projects.find((project) => project.id === activeProjectId) ?? null; return activeProject ? { id: activeProject.id, path: activeProject.path } : null; }; diff --git a/packages/ui/src/lib/planSaveQueue.test.ts b/packages/ui/src/lib/planSaveQueue.test.ts new file mode 100644 index 00000000..d1d4687a --- /dev/null +++ b/packages/ui/src/lib/planSaveQueue.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, test } from 'bun:test'; + +import { createPlanSaveQueue } from './planSaveQueue'; + +type Deferred = { promise: Promise; resolve: () => void; reject: () => void }; + +const deferred = (): Deferred => { + let resolve!: () => void; + let reject!: () => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; + +describe('planSaveQueue', () => { + test('runs writes for one document in schedule order even when they resolve out of order', async () => { + const queue = createPlanSaveQueue(); + const order: string[] = []; + const first = deferred(); + const second = deferred(); + + const firstDone = queue.schedule('doc', 1, async () => { + await first.promise; + order.push('first'); + }); + const secondDone = queue.schedule('doc', 2, async () => { + order.push('second'); + }); + + // Second started only after first settles, regardless of timing. + first.resolve(); + await firstDone; + second.resolve(); + await secondDone; + + expect(order).toEqual(['first', 'second']); + }); + + test('skips a revision at or below the last queued revision for the same document', async () => { + const queue = createPlanSaveQueue(); + let writes = 0; + + await queue.schedule('doc', 3, async () => { + writes += 1; + }); + await queue.schedule('doc', 3, async () => { + writes += 1; + }); + await queue.schedule('doc', 2, async () => { + writes += 1; + }); + + expect(writes).toBe(1); + }); + + test('never lets a write for one document block another document', async () => { + const queue = createPlanSaveQueue(); + const blocked = deferred(); + + const blockedDone = queue.schedule('a', 1, async () => { + await blocked.promise; + }); + let otherRan = false; + await queue.schedule('b', 1, async () => { + otherRan = true; + }); + + expect(otherRan).toBe(true); + blocked.resolve(); + await blockedDone; + }); + + test('pendingFor waits for the outstanding chain of that document only', async () => { + const queue = createPlanSaveQueue(); + const slow = deferred(); + let slowSettled = false; + + void queue.schedule('a', 1, async () => { + await slow.promise; + slowSettled = true; + }); + await queue.schedule('b', 1, async () => {}); + + await queue.pendingFor('b'); + expect(slowSettled).toBe(false); + + slow.resolve(); + await queue.pendingFor('a'); + expect(slowSettled).toBe(true); + }); + + test('reset clears the revision watermark so a reloaded document can save again', async () => { + const queue = createPlanSaveQueue(); + let writes = 0; + + await queue.schedule('doc', 5, async () => { + writes += 1; + }); + queue.reset('doc'); + await queue.schedule('doc', 1, async () => { + writes += 1; + }); + + expect(writes).toBe(2); + }); + + test('a failed write does not poison the chain for later writes', async () => { + const queue = createPlanSaveQueue(); + + const failing = queue.schedule('doc', 1, async () => { + throw new Error('write failed'); + }); + let secondRan = false; + const second = queue.schedule('doc', 2, async () => { + secondRan = true; + }); + + await expect(failing).rejects.toThrow('write failed'); + await second; + expect(secondRan).toBe(true); + await queue.pendingFor('doc'); + }); + + test('allows the same revision to retry after its write fails', async () => { + const queue = createPlanSaveQueue(); + let attempts = 0; + + const failing = queue.schedule('doc', 1, async () => { + attempts += 1; + throw new Error('write failed'); + }); + await expect(failing).rejects.toThrow('write failed'); + + await queue.schedule('doc', 1, async () => { + attempts += 1; + }); + + expect(attempts).toBe(2); + }); +}); diff --git a/packages/ui/src/lib/planSaveQueue.ts b/packages/ui/src/lib/planSaveQueue.ts new file mode 100644 index 00000000..647f91d6 --- /dev/null +++ b/packages/ui/src/lib/planSaveQueue.ts @@ -0,0 +1,61 @@ +/** + * Write queue for open plan documents. + * + * Debounced autosave and close-time flushes must reach the disk in edit order, + * and a document re-opened while its own write is still in flight must read + * the post-write state, not race it. The queue serializes writes per logical + * document key and deduplicates revisions so a flush of revision N can never + * run behind, or twice behind, a debounced save of the same revision. + */ + +interface PlanSaveQueue { + /** + * Queue one write for `key`. Writes for the same key run in schedule order; + * writes for different keys never block each other. A revision at or below + * the last queued revision for that key is skipped — the queued write + * already carries newer content — and the returned promise tracks the + * outstanding chain so callers can still await it. + */ + schedule: (key: string, revision: number, write: () => Promise) => Promise; + /** Resolves when every write queued for `key` has settled. */ + pendingFor: (key: string) => Promise; + /** + * Forgets the revision watermark for `key`. Call when a document is freshly + * loaded: its revision counter restarts, and stale watermarks from a + * previous open must not swallow the first real edit. + */ + reset: (key: string) => void; +} + +export const createPlanSaveQueue = (): PlanSaveQueue => { + const chains = new Map>(); + const lastRevision = new Map(); + + return { + schedule: (key, revision, write) => { + if (revision <= (lastRevision.get(key) ?? Number.NEGATIVE_INFINITY)) { + return chains.get(key) ?? Promise.resolve(); + } + lastRevision.set(key, revision); + const previous = chains.get(key) ?? Promise.resolve(); + // A failed write must not poison the chain: the next write for this + // document is still safe to attempt, and error surfacing belongs to the + // caller that owns UI state. + const next = previous.then(write, write); + chains.set(key, next.catch(() => { + // Keep newer queued revisions deduplicated, but let the caller retry + // this exact revision after its write has failed. + if (lastRevision.get(key) === revision) { + lastRevision.delete(key); + } + })); + return next; + }, + pendingFor: async (key) => { + await chains.get(key); + }, + reset: (key) => { + lastRevision.delete(key); + }, + }; +}; diff --git a/packages/ui/src/lib/projectContextApi.ts b/packages/ui/src/lib/projectContextApi.ts index ba555cb0..a1f991f9 100644 --- a/packages/ui/src/lib/projectContextApi.ts +++ b/packages/ui/src/lib/projectContextApi.ts @@ -57,6 +57,17 @@ export interface ProjectRef { path: string; } +/** + * A saved project plan plus the project that owns it, carried as one value so + * a viewer can never end up with a plan id whose owner it has to guess. + * PlanView resolves no owner on its own: the panel (or the persisted tab, + * or the mobile surface) that opened the plan knows the owner exactly. + */ +export interface SavedProjectPlanTarget { + projectRef: ProjectRef; + planId: string; +} + export const PROJECT_NOTE_BODY_MAX_LENGTH = 3000; export const PROJECT_TODO_TEXT_MAX_LENGTH = 120; diff --git a/packages/ui/src/stores/useUIStore.contextPanel.test.ts b/packages/ui/src/stores/useUIStore.contextPanel.test.ts index 9f89be00..749272f8 100644 --- a/packages/ui/src/stores/useUIStore.contextPanel.test.ts +++ b/packages/ui/src/stores/useUIStore.contextPanel.test.ts @@ -28,6 +28,183 @@ describe('useUIStore context panel tabs', () => { expect(tabs).toHaveLength(1); expect(tabs[0]?.readOnly).toBe(false); }); + + test('keeps a plan tab that carries its owning project', () => { + const directory = '/repo'; + const projectRef = { id: 'proj_1', path: '/repo' }; + + useUIStore.getState().openContextPanelTab(directory, { + mode: 'plan', + projectPlanId: 'plan-1', + projectPlanRef: projectRef, + dedupeKey: `plan:${projectRef.id}:plan-1`, + label: 'My plan', + }); + + const tabs = useUIStore.getState().contextPanelByDirectory[directory]?.tabs ?? []; + expect(tabs).toHaveLength(1); + expect(tabs[0]?.projectPlanId).toBe('plan-1'); + expect(tabs[0]?.projectPlanRef).toEqual(projectRef); + }); + + test('dedupes plan tabs by owner and plan id, not by plan id alone', () => { + const directory = '/repo'; + + useUIStore.getState().openContextPanelTab(directory, { + mode: 'plan', + projectPlanId: 'plan-1', + projectPlanRef: { id: 'proj_1', path: '/repo' }, + dedupeKey: 'plan:proj_1:plan-1', + }); + useUIStore.getState().openContextPanelTab(directory, { + mode: 'plan', + projectPlanId: 'plan-1', + projectPlanRef: { id: 'proj_1', path: '/repo' }, + dedupeKey: 'plan:proj_1:plan-1', + }); + + const tabs = useUIStore.getState().contextPanelByDirectory[directory]?.tabs ?? []; + expect(tabs).toHaveLength(1); + }); + + test('drops persisted plan tabs whose owner is missing instead of guessing it', () => { + const directory = '/repo'; + const persisted = { + contextPanelByDirectory: { + [directory]: { + isOpen: true, + expanded: false, + widthByMode: {}, + touchedAt: 1, + activeTabId: 'plan:plan-1', + tabs: [ + // Pre-owner tab: has an id but no projectPlanRef. + { + id: 'plan:plan-1', + mode: 'plan', + targetPath: null, + projectPlanId: 'plan-1', + projectPlanRef: null, + dedupeKey: 'plan:plan-1', + label: 'Old plan', + sessionTitleFallback: null, + readOnly: false, + stagedDiff: false, + diffScope: null, + touchedAt: 1, + }, + ], + }, + }, + }; + + // SAFETY: the object mirrors the persisted context-panel shape exactly; + // setState bypasses the persist middleware's typing, not its migration. + useUIStore.setState(persisted as never); + // Sanitization runs whenever panel state is touched; opening a valid tab + // is the ordinary touch that would flush stale persisted tabs out. + useUIStore.getState().openContextPanelTab(directory, { + mode: 'plan', + projectPlanId: 'plan-2', + projectPlanRef: { id: 'proj_1', path: '/repo' }, + dedupeKey: 'plan:proj_1:plan-2', + }); + + const tabs = useUIStore.getState().contextPanelByDirectory[directory]?.tabs ?? []; + expect(tabs).toHaveLength(1); + expect(tabs[0]?.projectPlanId).toBe('plan-2'); + }); + + test('keeps a generic filesystem plan tab that has no saved-plan identity', () => { + const directory = '/repo'; + useUIStore.getState().openContextSurface(directory, 'plan'); + // A later touch runs the same sanitizer rehydrate uses. + useUIStore.getState().openContextPanelTab(directory, { mode: 'diff' }); + + const tabs = useUIStore.getState().contextPanelByDirectory[directory]?.tabs ?? []; + const planTab = tabs.find((tab) => tab.mode === 'plan'); + expect(planTab).toBeDefined(); + expect(planTab?.projectPlanId).toBeNull(); + expect(planTab?.projectPlanRef).toBeNull(); + }); + + test('keeps a persisted generic plan tab through rehydration-like touches', () => { + const directory = '/repo'; + const persisted = { + contextPanelByDirectory: { + [directory]: { + isOpen: true, + expanded: false, + widthByMode: {}, + touchedAt: 1, + activeTabId: 'plan', + tabs: [ + { + id: 'plan', + mode: 'plan', + targetPath: null, + projectPlanId: null, + projectPlanRef: null, + dedupeKey: 'plan', + label: 'Plan', + sessionTitleFallback: null, + readOnly: false, + stagedDiff: false, + diffScope: null, + touchedAt: 1, + }, + ], + }, + }, + }; + + // SAFETY: the object mirrors the persisted context-panel shape exactly; + // setState bypasses the persist middleware's typing, not its migration. + useUIStore.setState(persisted as never); + useUIStore.getState().openContextPanelTab(directory, { mode: 'diff' }); + + const tabs = useUIStore.getState().contextPanelByDirectory[directory]?.tabs ?? []; + expect(tabs.some((tab) => tab.mode === 'plan')).toBe(true); + }); + + test('drops a persisted saved-plan tab carrying an owner but no plan id', () => { + const directory = '/repo'; + const persisted = { + contextPanelByDirectory: { + [directory]: { + isOpen: true, + expanded: false, + widthByMode: {}, + touchedAt: 1, + activeTabId: null, + tabs: [ + { + id: 'plan:proj_1:plan-1', + mode: 'plan', + targetPath: null, + projectPlanId: null, + projectPlanRef: { id: 'proj_1', path: '/repo' }, + dedupeKey: 'plan:proj_1:plan-1', + label: 'Half-identified', + sessionTitleFallback: null, + readOnly: false, + stagedDiff: false, + diffScope: null, + touchedAt: 1, + }, + ], + }, + }, + }; + + // SAFETY: the object mirrors the persisted context-panel shape exactly; + // setState bypasses the persist middleware's typing, not its migration. + useUIStore.setState(persisted as never); + useUIStore.getState().openContextPanelTab(directory, { mode: 'diff' }); + + const tabs = useUIStore.getState().contextPanelByDirectory[directory]?.tabs ?? []; + expect(tabs.some((tab) => tab.mode === 'plan')).toBe(false); + }); }); describe('useUIStore openContextSurface', () => { diff --git a/packages/ui/src/stores/useUIStore.ts b/packages/ui/src/stores/useUIStore.ts index 8ec57362..214bc2a1 100644 --- a/packages/ui/src/stores/useUIStore.ts +++ b/packages/ui/src/stores/useUIStore.ts @@ -8,6 +8,7 @@ import type { DraftStarterRef } from '@/lib/draftStarters'; import { DEFAULT_MONO_FONT, DEFAULT_UI_FONT, type MonoFontOption, type UiFontOption } from '@/lib/fontOptions'; import { getStoredMobileKeyboardMode, type MobileKeyboardMode } from '@/lib/mobileKeyboardMode'; import type { TerminalShell } from '@/lib/api/types'; +import type { ProjectRef } from '@/lib/projectContextApi'; import { useFilesViewTabsStore } from './useFilesViewTabsStore'; import { isWindowsArm64 } from '@/lib/platform'; import { isVSCodeRuntime } from '@/lib/desktop'; @@ -37,6 +38,10 @@ type ContextPanelTab = { panel. Project plans are addressed by id because their markdown is server-owned and has no client-visible path. */ projectPlanId: string | null; + /** The project that owns `projectPlanId`. Persisted with the tab so a + restored plan tab opens against its own project instead of guessing the + owner from whatever directory happens to be current. */ + projectPlanRef: ProjectRef | null; dedupeKey: string; label: string | null; sessionTitleFallback: string | null; @@ -50,6 +55,7 @@ type ContextPanelTabDescriptor = { mode: ContextPanelMode; targetPath?: string | null; projectPlanId?: string | null; + projectPlanRef?: ProjectRef | null; dedupeKey?: string | null; label?: string | null; sessionTitleFallback?: string | null; @@ -191,6 +197,18 @@ const normalizePendingDiffScope = (value: unknown): PendingDiffScope | null => { return value === 'working' || value === 'staged' || value === 'turn' || value === 'branch' ? value : null; }; +/** A plan tab's owner must be a complete project reference or nothing; a + half-valid one is worse than none because it points the editor somewhere. */ +const normalizeContextPanelProjectPlanRef = (value: unknown): ProjectRef | null => { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return null; + } + const candidate = value as { id?: unknown; path?: unknown }; + const id = typeof candidate.id === 'string' ? candidate.id.trim() : ''; + const path = typeof candidate.path === 'string' ? candidate.path.trim() : ''; + return id && path ? { id, path } : null; +}; + const buildDefaultContextPanelTabDedupeKey = (mode: ContextPanelMode, targetPath: string | null): string => { if (mode === 'file') { return targetPath || mode; @@ -240,6 +258,7 @@ const createContextPanelTab = (descriptor: ContextPanelTabDescriptor): ContextPa projectPlanId: typeof descriptor.projectPlanId === 'string' && descriptor.projectPlanId.trim() ? descriptor.projectPlanId.trim() : null, + projectPlanRef: normalizeContextPanelProjectPlanRef(descriptor.projectPlanRef), dedupeKey, label: normalizeContextTabLabel(descriptor.label), sessionTitleFallback: normalizeContextTabLabel(descriptor.sessionTitleFallback), @@ -300,6 +319,7 @@ const sanitizeContextPanelTabs = (tabs: unknown): ContextPanelTab[] => { mode?: unknown; targetPath?: unknown; projectPlanId?: unknown; + projectPlanRef?: unknown; dedupeKey?: unknown; label?: unknown; sessionTitleFallback?: unknown; @@ -323,6 +343,19 @@ const sanitizeContextPanelTabs = (tabs: unknown): ContextPanelTab[] => { } const targetPath = normalizeContextTargetPath(typeof candidate.targetPath === 'string' ? candidate.targetPath : null); + const projectPlanId = typeof candidate.projectPlanId === 'string' && candidate.projectPlanId.trim() + ? candidate.projectPlanId.trim() + : null; + const projectPlanRef = normalizeContextPanelProjectPlanRef(candidate.projectPlanRef); + // `mode: 'plan'` covers two documents: a saved Project knowledge plan + // (needs both the plan id and its owning project) and a plain session + // filesystem plan (has neither). Only the half-identified form — id + // without owner — is unopenable: the editor would have to guess the + // project from the current directory, which is exactly the bug that made + // saved plans open empty. Such tabs are dropped rather than resurrected. + if (candidate.mode === 'plan' && (projectPlanId !== null) !== (projectPlanRef !== null)) { + continue; + } const dedupeKey = normalizeContextPanelTabDedupeKey( candidate.mode, targetPath, @@ -338,9 +371,8 @@ const sanitizeContextPanelTabs = (tabs: unknown): ContextPanelTab[] => { id, mode: candidate.mode, targetPath, - projectPlanId: typeof candidate.projectPlanId === 'string' && candidate.projectPlanId.trim() - ? candidate.projectPlanId.trim() - : null, + projectPlanId, + projectPlanRef, dedupeKey, label: normalizeContextTabLabel(typeof candidate.label === 'string' ? candidate.label : null), sessionTitleFallback: normalizeContextTabLabel(typeof candidate.sessionTitleFallback === 'string' ? candidate.sessionTitleFallback : null), @@ -405,20 +437,22 @@ const upsertContextPanelTab = ( const existingIndex = baseTabs.findIndex((tab) => tab.id === nextTab.id); const tabs = existingIndex === -1 ? [...baseTabs, nextTab] - : baseTabs.map((tab, index) => (index === existingIndex - ? { - ...tab, - mode: nextTab.mode, - targetPath: nextTab.targetPath || tab.targetPath, - dedupeKey: nextTab.dedupeKey, - label: nextTab.label, - sessionTitleFallback: nextTab.sessionTitleFallback || tab.sessionTitleFallback, - stagedDiff: nextTab.stagedDiff, - diffScope: nextTab.diffScope, - readOnly: nextTab.readOnly, - touchedAt: Date.now(), - } - : tab)); + : baseTabs.map((tab, index) => (index === existingIndex + ? { + ...tab, + mode: nextTab.mode, + targetPath: nextTab.targetPath || tab.targetPath, + projectPlanId: nextTab.projectPlanId ?? tab.projectPlanId, + projectPlanRef: nextTab.projectPlanRef ?? tab.projectPlanRef, + dedupeKey: nextTab.dedupeKey, + label: nextTab.label, + sessionTitleFallback: nextTab.sessionTitleFallback || tab.sessionTitleFallback, + stagedDiff: nextTab.stagedDiff, + diffScope: nextTab.diffScope, + readOnly: nextTab.readOnly, + touchedAt: Date.now(), + } + : tab)); // A background upsert (an agent working a page) keeps the panel exactly as // the user left it: closed stays closed, and whatever tab they were on @@ -545,6 +579,10 @@ const sanitizeContextPanelByDirectory = ( let tabs = sanitizeContextPanelTabs(candidate.tabs); let activeTabId = typeof candidate.activeTabId === 'string' ? candidate.activeTabId : null; + // Legacy single-tab state can name a saved project plan, but it carries + // no owner and cannot be migrated into an openable saved-plan tab — that + // combination is dropped by sanitize above. A generic filesystem plan tab + // (no plan id) revives fine from the descriptor alone. if (tabs.length === 0 && (candidate.mode === 'diff' || candidate.mode === 'file' || candidate.mode === 'context' || candidate.mode === 'plan' || candidate.mode === 'chat')) { tabs = [createContextPanelTab({ mode: candidate.mode,