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.
This commit is contained in:
@@ -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', () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user