From a03815e8517f16f4dbd1715269e367e838cda461 Mon Sep 17 00:00:00 2001 From: Zeid Diez Date: Sat, 18 Jul 2026 13:42:44 -0700 Subject: [PATCH 1/5] fix: restore active project folder controls Signed-off-by: Zeid Diez --- AI.md | 4 + Readme.md | 2 + apps/desktop/src/platform/auto-save.ts | 1 + apps/web/src/platform/auto-save.test.tsx | 85 ++++++ apps/web/src/platform/auto-save.ts | 1 + packages/ui/src/theme/global.css | 14 + .../src/views/settings/SettingsView.test.tsx | 135 +++++++++- .../ui/src/views/settings/StorageSettings.tsx | 242 ++++++++++++++++-- 8 files changed, 465 insertions(+), 19 deletions(-) create mode 100644 apps/web/src/platform/auto-save.test.tsx diff --git a/AI.md b/AI.md index 0d617b4..d35a7f5 100644 --- a/AI.md +++ b/AI.md @@ -86,6 +86,10 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - `AppShell` consumes key-scoped adapter `watch()` events for the active project and shows an explicit external-change banner with `Reload from storage` and `Keep my changes`; the Tauri Rust shell polls the active `.pms.json` fingerprint, reports edits/deletes/renames, and updates the expected revision before an intentional overwrite - the shared shell wraps routes in `ToastProvider`; route code should use `useToast()` for short-lived feedback and keep blocking or recoverable errors in `InlineAlert` - the shared shell exposes explicit project actions next to the save-state indicator: `Save now` / `Retry save` routes through the active `ProjectStoreAdapter.save()` plus `markSaving()` / `markSaved()` / `markSaveFailed()`, records a launcher recent from successful browser/folder save metadata, `Switch project` navigates to `/projects`, and `Close project` returns to the launcher immediately only for clean saved projects; dirty projects and unsaved in-memory imports/demos must confirm `Close without saving` before `closeProject()` runs +- `StorageSettings` is the active-project storage-location surface, not a read-only trust report: a browser-local or unsaved web project can `Save to local folder`, while a folder-backed web project can `Change folder` or `Use browser storage`; each successful transition serializes the bundle with the intended trust, saves through the installed `window.__gph_store` adapter, calls `markSaved()` with returned metadata, and replaces the same-key launcher recent +- storage-location transitions deliberately pass a null expected revision because the current revision belongs to the old target rather than the newly selected backend; folder selection first checks `listFolderProjects()` and refuses to overwrite a same-ID `.pms.json` unless it is the already-active folder target +- switching storage locations is copy-first and non-destructive: the old folder file or browser recovery copy remains in place, changing back to browser storage does not clear the shared browser folder handle needed by other folder-backed recents, and an `AbortError` from the native directory picker is a normal dismissal that does not mark the project save as failed +- web and desktop auto-save completions must verify that `latest.storageTrust` still matches the target they wrote before applying returned metadata; this prevents an in-flight save to the previous location from reverting the active trust/path/revision after a storage transition. Folder-to-folder moves are also disabled while the current folder project is dirty, because choosing a new browser folder handle changes the adapter target immediately. - the web shell now surfaces offline status and emits one lightweight due-item/reminder notification per project/day/count combination; local/self-hosted web builds capture `beforeinstallprompt` and show `Install app`, while hosted-demo builds suppress that install prompt and show a `Run locally` GitHub setup CTA instead - desktop storage now only writes to the filesystem when a folder path has actually been attached; otherwise the desktop shell behaves as browser-local storage on purpose instead of pretending to be folder-backed - every save now carries an explicit `browser` or `folder` target; an active browser folder handle or desktop folder path cannot silently promote a different browser-local project, and the PWA launcher exposes `Use browser storage` to clear its selected folder diff --git a/Readme.md b/Readme.md index f7448ba..cb29238 100644 --- a/Readme.md +++ b/Readme.md @@ -82,6 +82,8 @@ The durable project format is a `.pms.json` bundle under `.pm-suite/` when using Important behavior: - New folder-backed projects write their initial `.pm-suite/.pms.json` immediately. +- An open browser-local project can move to a chosen folder from **Settings -> Storage** without being recreated; a folder-backed project can change folders or switch back to browser storage from the same panel. +- Changing storage locations writes and activates the new copy first. The previous browser or folder copy is retained as a recovery point, and Grillo refuses to overwrite a same-ID project already present in a newly selected folder. - Folder-backed browser saves keep a browser-local recovery copy so reloads are recoverable when the browser cannot restore folder access. - Reopening folder-backed recents asks for folder access and refuses to silently open stale browser recovery when the selected folder is missing the recorded project file. - Imported JSON and demo projects are unsaved in-memory sessions until the user explicitly saves them. diff --git a/apps/desktop/src/platform/auto-save.ts b/apps/desktop/src/platform/auto-save.ts index f70ebfe..4871950 100644 --- a/apps/desktop/src/platform/auto-save.ts +++ b/apps/desktop/src/platform/auto-save.ts @@ -32,6 +32,7 @@ export function useAutoSave() { ); const latest = useProjectStore.getState(); if (!latest.bundle || latest.storageKey !== key || latest.bundle.project.id !== projectId) return; + if (latest.storageTrust !== targetTrust) return; if (latest.serialize() === json) { latest.markSaved(key, meta.displayPath, meta.trust, meta.externalRevision); } else { diff --git a/apps/web/src/platform/auto-save.test.tsx b/apps/web/src/platform/auto-save.test.tsx new file mode 100644 index 0000000..42274d2 --- /dev/null +++ b/apps/web/src/platform/auto-save.test.tsx @@ -0,0 +1,85 @@ +import { act, cleanup, render } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { buildProjectFromTemplate, type StorageMetadata } from "@gph/core"; +import { useProjectStore } from "@gph/ui"; +import { useAutoSave } from "./auto-save"; +import { WebStorageAdapter } from "./storage/web-storage"; + +function AutoSaveHarness() { + useAutoSave(); + return null; +} + +describe("web auto-save", () => { + beforeEach(() => { + vi.useFakeTimers(); + const bundle = buildProjectFromTemplate("software-project", "Auto-save target"); + useProjectStore.setState({ + bundle, + storageKey: bundle.project.id, + storagePath: null, + storageTrust: "browser", + externalRevision: 101, + isDirty: true, + saveStatus: "idle", + saveError: null + }); + }); + + afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it("ignores an in-flight result after the active storage target changes", async () => { + let resolveSave!: (metadata: StorageMetadata) => void; + const pendingSave = new Promise((resolve) => { + resolveSave = resolve; + }); + const save = vi.spyOn(WebStorageAdapter.adapter, "save").mockReturnValue(pendingSave); + const projectId = useProjectStore.getState().bundle!.project.id; + + render(); + + await act(async () => { + vi.advanceTimersByTime(251); + await Promise.resolve(); + }); + expect(save).toHaveBeenCalledOnce(); + + const current = useProjectStore.getState().bundle!; + act(() => { + useProjectStore.setState({ + bundle: { + ...current, + projectSettings: { ...current.projectSettings, storageTrust: "folder" } + }, + storagePath: `Client Work/.pm-suite/${projectId}.pms.json`, + storageTrust: "folder", + externalRevision: 909, + isDirty: false, + saveStatus: "saved" + }); + }); + + await act(async () => { + resolveSave({ + key: projectId, + displayPath: null, + externalRevision: 202, + trust: "browser" + }); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(useProjectStore.getState()).toMatchObject({ + storagePath: `Client Work/.pm-suite/${projectId}.pms.json`, + storageTrust: "folder", + externalRevision: 909, + isDirty: false, + saveStatus: "saved" + }); + }); +}); diff --git a/apps/web/src/platform/auto-save.ts b/apps/web/src/platform/auto-save.ts index 27cae23..32d9f0a 100644 --- a/apps/web/src/platform/auto-save.ts +++ b/apps/web/src/platform/auto-save.ts @@ -34,6 +34,7 @@ export function useAutoSave() { ); const latest = useProjectStore.getState(); if (!latest.bundle || latest.storageKey !== key || latest.bundle.project.id !== projectId) return; + if (latest.storageTrust !== targetTrust) return; if (latest.serialize() === json) { latest.markSaved(key, meta.displayPath, meta.trust, meta.externalRevision); } else { diff --git a/packages/ui/src/theme/global.css b/packages/ui/src/theme/global.css index 9200bcb..30bdef0 100644 --- a/packages/ui/src/theme/global.css +++ b/packages/ui/src/theme/global.css @@ -2717,6 +2717,16 @@ a.btn { padding: 10px 0; border-top: 1px solid var(--color-border-subtle); } +.storage-settings-location { + align-items: flex-start; + flex-wrap: wrap; +} +.storage-settings-path { + min-width: 0; + max-width: 100%; + overflow-wrap: anywhere; + text-align: right; +} .settings-truth-callout { display: flex; flex-direction: column; @@ -3483,6 +3493,10 @@ a.btn { .settings-inline-form .select { min-width: 0; } + .storage-settings-path { + width: 100%; + text-align: left; + } .form-grid-2 { grid-template-columns: 1fr; } diff --git a/packages/ui/src/views/settings/SettingsView.test.tsx b/packages/ui/src/views/settings/SettingsView.test.tsx index 9e572e2..bedc8d6 100644 --- a/packages/ui/src/views/settings/SettingsView.test.tsx +++ b/packages/ui/src/views/settings/SettingsView.test.tsx @@ -1,9 +1,16 @@ import { cleanup, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { MemoryRouter } from "react-router-dom"; -import { buildProjectFromTemplate, exportProjectJson } from "@gph/core"; +import { + buildProjectFromTemplate, + exportProjectJson, + importProjectJson, + type PersistedStorageTrust, + type ProjectStoreAdapter +} from "@gph/core"; import { useProjectStore } from "../../store/project-store"; +import { useWorkspaceStore } from "../../store/workspace-store"; import { ThemeProvider } from "../../theme/theme-provider"; import { SettingsView } from "./SettingsView"; @@ -11,13 +18,18 @@ describe("SettingsView", () => { beforeEach(() => { cleanup(); localStorage.clear(); + delete (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store; + useWorkspaceStore.setState({ localMemberId: null, recents: [] }); const bundle = buildProjectFromTemplate("software-project", "Settings"); useProjectStore.setState({ bundle, storageKey: bundle.project.id, storagePath: null, storageTrust: "browser", + externalRevision: 101, isDirty: false, + saveStatus: "saved", + saveError: null, lastSource: null }); useProjectStore.getState().applyCommand({ @@ -190,6 +202,125 @@ describe("SettingsView", () => { expect(within(labelPanel).getByPlaceholderText("Milestone name")).toBeInTheDocument(); }); + it("moves the current browser project to a local folder and back without recreating it", async () => { + const chooseFolder = vi.fn(async () => "Client Work"); + const save = vi.fn(async ( + key: string, + _json: string, + _expectedRevision?: number | null, + targetTrust?: PersistedStorageTrust + ) => ({ + key, + displayPath: targetTrust === "folder" ? `Client Work/.pm-suite/${key}.pms.json` : null, + externalRevision: targetTrust === "folder" ? 202 : 303, + trust: targetTrust ?? "browser" + })); + const adapter: ProjectStoreAdapter = { + capabilities: { folderBacked: true, fileWatch: false, attachments: true }, + list: async () => [], + has: async () => true, + load: async () => null, + save, + delete: async () => undefined, + chooseFolder, + getCurrentFolderDisplay: async () => null, + listFolderProjects: async () => [] + }; + (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store = adapter; + + render( + + + + + + ); + + await userEvent.click(screen.getByRole("tab", { name: "Storage" })); + const panel = screen.getByRole("tabpanel", { name: "Storage" }); + await userEvent.click(within(panel).getByRole("button", { name: "Save to local folder" })); + + await waitFor(() => expect(save).toHaveBeenCalledTimes(1)); + expect(chooseFolder).toHaveBeenCalledOnce(); + const [folderKey, folderJson, folderRevision, folderTrust] = save.mock.calls[0]; + expect(folderKey).toBe(useProjectStore.getState().bundle?.project.id); + expect(folderRevision).toBeNull(); + expect(folderTrust).toBe("folder"); + expect(importProjectJson(folderJson).bundle.projectSettings.storageTrust).toBe("folder"); + expect(useProjectStore.getState()).toMatchObject({ + storagePath: `Client Work/.pm-suite/${folderKey}.pms.json`, + storageTrust: "folder", + externalRevision: 202, + isDirty: false, + saveStatus: "saved" + }); + expect(useWorkspaceStore.getState().recents[0]).toMatchObject({ + key: folderKey, + trust: "folder", + storagePath: `Client Work/.pm-suite/${folderKey}.pms.json` + }); + expect(within(panel).getByText("Folder-backed")).toBeInTheDocument(); + + await userEvent.click(within(panel).getByRole("button", { name: "Use browser storage" })); + + await waitFor(() => expect(save).toHaveBeenCalledTimes(2)); + const [, browserJson, browserRevision, browserTrust] = save.mock.calls[1]; + expect(browserRevision).toBeNull(); + expect(browserTrust).toBe("browser"); + expect(importProjectJson(browserJson).bundle.projectSettings.storageTrust).toBe("browser"); + expect(useProjectStore.getState()).toMatchObject({ + storagePath: null, + storageTrust: "browser", + externalRevision: 303, + isDirty: false, + saveStatus: "saved" + }); + expect(useWorkspaceStore.getState().recents[0]).toMatchObject({ key: folderKey, trust: "browser", storagePath: null }); + expect(within(panel).getByText("Browser-local")).toBeInTheDocument(); + }); + + it("does not overwrite an existing folder project or treat picker cancellation as a save error", async () => { + const bundle = useProjectStore.getState().bundle!; + const save = vi.fn(); + const chooseFolder = vi + .fn<() => Promise>() + .mockResolvedValueOnce("Existing Work") + .mockRejectedValueOnce(new DOMException("Picker dismissed", "AbortError")); + const adapter: ProjectStoreAdapter = { + capabilities: { folderBacked: true, fileWatch: false, attachments: true }, + list: async () => [], + has: async () => true, + load: async () => null, + save, + delete: async () => undefined, + chooseFolder, + listFolderProjects: async () => [`${bundle.project.id}.pms.json`] + }; + (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store = adapter; + + render( + + + + + + ); + + await userEvent.click(screen.getByRole("tab", { name: "Storage" })); + const panel = screen.getByRole("tabpanel", { name: "Storage" }); + const saveToFolder = within(panel).getByRole("button", { name: "Save to local folder" }); + await userEvent.click(saveToFolder); + + expect(await within(panel).findByText(/already contains .*\.pms\.json/i)).toBeInTheDocument(); + expect(save).not.toHaveBeenCalled(); + expect(useProjectStore.getState()).toMatchObject({ storageTrust: "browser", saveStatus: "idle" }); + + await userEvent.click(saveToFolder); + await waitFor(() => expect(chooseFolder).toHaveBeenCalledTimes(2)); + expect(save).not.toHaveBeenCalled(); + expect(useProjectStore.getState()).toMatchObject({ storageTrust: "browser", saveError: null }); + }); + it("supports keyboard navigation across settings sections", async () => { render( diff --git a/packages/ui/src/views/settings/StorageSettings.tsx b/packages/ui/src/views/settings/StorageSettings.tsx index 9f6edf9..a511ff6 100644 --- a/packages/ui/src/views/settings/StorageSettings.tsx +++ b/packages/ui/src/views/settings/StorageSettings.tsx @@ -1,35 +1,243 @@ +import { useState } from "react"; +import { + exportProjectJson, + type PersistedStorageTrust, + type ProjectBundle, + type ProjectStoreAdapter +} from "@gph/core"; +import { InlineAlert, useToast, type InlineAlertTone } from "../../components"; import { useProjectStore } from "../../store/project-store"; -import { SettingsPanelHeader } from "./settings-shared"; +import { useWorkspaceStore } from "../../store/workspace-store"; +import { SettingsPanelHeader, SettingsSectionCard } from "./settings-shared"; + +type StorageOperation = PersistedStorageTrust | null; + +type StorageFeedback = { + message: string; + tone: InlineAlertTone; +}; + +function activeAdapter(): ProjectStoreAdapter | null { + if (typeof window === "undefined") return null; + return ((window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store) ?? null; +} + +function withStorageTrust(bundle: ProjectBundle, storageTrust: PersistedStorageTrust): ProjectBundle { + return { + ...bundle, + projectSettings: { + ...bundle.projectSettings, + storageTrust + } + }; +} + +function isFolderPickerDismissal(error: unknown): boolean { + return error instanceof DOMException && error.name === "AbortError"; +} + +function isCurrentFolderTarget( + storagePath: string | null, + folderName: string, + projectKey: string +): boolean { + if (!storagePath) return false; + const normalized = storagePath.replace(/\\/g, "/"); + const expected = `${folderName}/.pm-suite/${projectKey}.pms.json`; + return normalized === expected || normalized.endsWith(`/${expected}`); +} export function StorageSettings() { const bundle = useProjectStore((state) => state.bundle); + const storagePath = useProjectStore((state) => state.storagePath); + const storageTrust = useProjectStore((state) => state.storageTrust); + const isDirty = useProjectStore((state) => state.isDirty); + const saveStatus = useProjectStore((state) => state.saveStatus); + const markSaving = useProjectStore((state) => state.markSaving); + const markSaved = useProjectStore((state) => state.markSaved); + const markSaveFailed = useProjectStore((state) => state.markSaveFailed); + const recordRecent = useWorkspaceStore((state) => state.recordRecent); + const [operation, setOperation] = useState(null); + const [feedback, setFeedback] = useState(null); + const { notify } = useToast(); if (!bundle) return null; + const adapter = activeAdapter(); + const canChooseFolder = Boolean(adapter?.capabilities.folderBacked && adapter.chooseFolder); + const busy = operation !== null || saveStatus === "saving"; + const mustSaveBeforeChangingFolder = storageTrust === "folder" && isDirty; + const storageCopy = - bundle.projectSettings.storageTrust === "folder" - ? "This project is attached to a local folder and will auto-save there." - : bundle.projectSettings.storageTrust === "browser" - ? "This project is currently browser-local. Reopen it from the workspace recents, or export/import a bundle when moving machines." + storageTrust === "folder" + ? "This project is attached to a local folder and auto-saves there. A browser recovery copy is retained." + : storageTrust === "browser" + ? "This project is saved in this browser profile. You can move the current project into a local folder without recreating it." : "This project has not been saved yet."; + const showFailure = (message: string) => { + setFeedback({ message, tone: "danger" }); + notify({ tone: "danger", message }); + }; + + const persistTo = async (targetTrust: PersistedStorageTrust, folderName?: string): Promise => { + const current = useProjectStore.getState(); + const currentBundle = current.bundle; + if (!adapter || !currentBundle) { + const message = "No storage adapter is available for this project."; + markSaveFailed(message); + showFailure(message); + return false; + } + + markSaving(); + try { + const key = current.storageKey ?? currentBundle.project.id; + const metadata = await adapter.save( + key, + exportProjectJson(withStorageTrust(currentBundle, targetTrust)), + null, + targetTrust + ); + markSaved(metadata.key, metadata.displayPath, metadata.trust, metadata.externalRevision); + if (metadata.trust !== "unsaved") { + recordRecent({ + key: metadata.key, + name: currentBundle.project.name, + storagePath: metadata.displayPath, + trust: metadata.trust, + lastOpenedAt: new Date().toISOString() + }); + } + const destination = targetTrust === "folder" + ? `local folder${folderName ? ` ${folderName}` : ""}` + : "browser storage"; + const message = `Saved ${currentBundle.project.name} to ${destination}.`; + setFeedback({ message, tone: "success" }); + notify({ tone: "success", message }); + return true; + } catch (error) { + const detail = error instanceof Error ? error.message : "The save did not complete."; + const message = `Storage change failed: ${detail}`; + markSaveFailed(detail); + showFailure(message); + return false; + } + }; + + const saveToLocalFolder = async () => { + if (!adapter?.chooseFolder) return; + setOperation("folder"); + setFeedback(null); + try { + const folderName = await adapter.chooseFolder(); + if (!folderName) { + setFeedback({ + message: "Folder access was not granted. The project remains in its current storage location.", + tone: "warning" + }); + return; + } + + const current = useProjectStore.getState(); + const key = current.storageKey ?? current.bundle?.project.id ?? bundle.project.id; + const folderFiles = await adapter.listFolderProjects?.(); + const projectAlreadyExists = folderFiles?.some( + (filename) => filename.toLowerCase() === `${key}.pms.json`.toLowerCase() + ); + const currentTarget = current.storageTrust === "folder" && isCurrentFolderTarget(current.storagePath, folderName, key); + if (projectAlreadyExists && !currentTarget) { + setFeedback({ + message: `That folder already contains ${key}.pms.json. Grillo did not overwrite it; open that project from the workspace launcher or choose a different folder.`, + tone: "warning" + }); + return; + } + + await persistTo("folder", folderName); + } catch (error) { + if (isFolderPickerDismissal(error)) return; + const detail = error instanceof Error ? error.message : "Folder access did not complete."; + showFailure(`Folder access failed: ${detail}`); + } finally { + setOperation(null); + } + }; + + const saveToBrowser = async () => { + setOperation("browser"); + setFeedback(null); + try { + await persistTo("browser"); + } finally { + setOperation(null); + } + }; + return (
-
{storageCopy}
-
- - {" "} - {bundle.projectSettings.storageTrust === "folder" - ? "Folder-backed" - : bundle.projectSettings.storageTrust === "browser" - ? "Browser-local" - : "Unsaved"} - -
+ +
+ + {" "} + {storageTrust === "folder" + ? "Folder-backed" + : storageTrust === "browser" + ? "Browser-local" + : "Unsaved"} + + {storagePath ?? "This browser profile"} +
+ + + + {canChooseFolder ? ( +
+ + {storageTrust === "folder" ? ( + + ) : null} +
+ ) : ( + + Local-folder access is unavailable in this runtime. Browser-local saves and portable JSON export remain available. + + )} +

+ Folder saves use the browser's directory picker and write .pm-suite/{bundle.project.id}.pms.json. Moving back to browser storage does not delete the folder copy. +

+ {mustSaveBeforeChangingFolder ? ( + + Save the current folder copy before choosing a different folder. You can still use browser storage to preserve the current changes. + + ) : null} + {feedback ? {feedback.message} : null} +
); } From b63d3d0cfef2a42ec5b88de0483ff3f2c7c20cba Mon Sep 17 00:00:00 2001 From: Zeid Diez Date: Sat, 18 Jul 2026 14:02:03 -0700 Subject: [PATCH 2/5] fix: ignore stale auto-save failures Signed-off-by: Zeid Diez --- AI.md | 6 +- Readme.md | 1 + apps/desktop/src/platform/auto-save.test.tsx | 87 ++++++++++++++++++++ apps/desktop/src/platform/auto-save.ts | 2 +- apps/web/src/platform/auto-save.test.tsx | 49 +++++++++++ apps/web/src/platform/auto-save.ts | 2 +- 6 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 apps/desktop/src/platform/auto-save.test.tsx diff --git a/AI.md b/AI.md index d35a7f5..2e18a13 100644 --- a/AI.md +++ b/AI.md @@ -89,7 +89,7 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - `StorageSettings` is the active-project storage-location surface, not a read-only trust report: a browser-local or unsaved web project can `Save to local folder`, while a folder-backed web project can `Change folder` or `Use browser storage`; each successful transition serializes the bundle with the intended trust, saves through the installed `window.__gph_store` adapter, calls `markSaved()` with returned metadata, and replaces the same-key launcher recent - storage-location transitions deliberately pass a null expected revision because the current revision belongs to the old target rather than the newly selected backend; folder selection first checks `listFolderProjects()` and refuses to overwrite a same-ID `.pms.json` unless it is the already-active folder target - switching storage locations is copy-first and non-destructive: the old folder file or browser recovery copy remains in place, changing back to browser storage does not clear the shared browser folder handle needed by other folder-backed recents, and an `AbortError` from the native directory picker is a normal dismissal that does not mark the project save as failed -- web and desktop auto-save completions must verify that `latest.storageTrust` still matches the target they wrote before applying returned metadata; this prevents an in-flight save to the previous location from reverting the active trust/path/revision after a storage transition. Folder-to-folder moves are also disabled while the current folder project is dirty, because choosing a new browser folder handle changes the adapter target immediately. +- web and desktop auto-save success and failure completions must verify that `latest.storageTrust` still matches the target they wrote before applying returned metadata or save-error state; this prevents an in-flight save to the previous location from reverting the active trust/path/revision or marking the newly active target as failed after a storage transition. Folder-to-folder moves are also disabled while the current folder project is dirty, because choosing a new browser folder handle changes the adapter target immediately. - the web shell now surfaces offline status and emits one lightweight due-item/reminder notification per project/day/count combination; local/self-hosted web builds capture `beforeinstallprompt` and show `Install app`, while hosted-demo builds suppress that install prompt and show a `Run locally` GitHub setup CTA instead - desktop storage now only writes to the filesystem when a folder path has actually been attached; otherwise the desktop shell behaves as browser-local storage on purpose instead of pretending to be folder-backed - every save now carries an explicit `browser` or `folder` target; an active browser folder handle or desktop folder path cannot silently promote a different browser-local project, and the PWA launcher exposes `Use browser storage` to clear its selected folder @@ -713,6 +713,10 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - mounting a non-modal, keyboard-readable tutorial panel in `AppShell` with 13 steps, progress semantics, Back/Next/Exit controls, responsive bottom-sheet behavior, and real route transitions across Overview, Board, Backlog, Table, Roadmap, Calendar, Docs, Bug Triage, My Work, Search/commands, Trash, and Settings - synchronizing the wizard with manual sidebar navigation so users can skip ahead or revisit a feature without being forced back to the previous route - adding focused launcher/tutorial component coverage plus a full Playwright journey that advances through every tutorial stop and verifies the final handoff back to Overview +- addressed the required Greptile storage-transition follow-up by: + - requiring both the active `storageKey` and `storageTrust` to match the target captured by web and desktop auto-save before a rejected write can call `markSaveFailed()` + - leaving the new target's `saveStatus`, `saveError`, dirty state, path, and external revision untouched when an older browser or folder write rejects after the user changes storage locations + - adding rejection-path auto-save regressions for browser-to-folder web transitions and folder-to-browser desktop transitions, complementing the existing successful-completion race coverage ## Open follow-on planning diff --git a/Readme.md b/Readme.md index cb29238..df890af 100644 --- a/Readme.md +++ b/Readme.md @@ -84,6 +84,7 @@ Important behavior: - New folder-backed projects write their initial `.pm-suite/.pms.json` immediately. - An open browser-local project can move to a chosen folder from **Settings -> Storage** without being recreated; a folder-backed project can change folders or switch back to browser storage from the same panel. - Changing storage locations writes and activates the new copy first. The previous browser or folder copy is retained as a recovery point, and Grillo refuses to overwrite a same-ID project already present in a newly selected folder. +- Auto-save results belong to the storage target that started them. If the user changes targets while a save is in flight, its later success or failure cannot overwrite the new target's saved state. - Folder-backed browser saves keep a browser-local recovery copy so reloads are recoverable when the browser cannot restore folder access. - Reopening folder-backed recents asks for folder access and refuses to silently open stale browser recovery when the selected folder is missing the recorded project file. - Imported JSON and demo projects are unsaved in-memory sessions until the user explicitly saves them. diff --git a/apps/desktop/src/platform/auto-save.test.tsx b/apps/desktop/src/platform/auto-save.test.tsx new file mode 100644 index 0000000..2837a75 --- /dev/null +++ b/apps/desktop/src/platform/auto-save.test.tsx @@ -0,0 +1,87 @@ +import { act, cleanup, render } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { buildProjectFromTemplate, type StorageMetadata } from "@gph/core"; +import { useProjectStore } from "@gph/ui"; +import { useAutoSave } from "./auto-save"; +import { DesktopStorageAdapter } from "./storage/desktop-storage"; + +function AutoSaveHarness() { + useAutoSave(); + return null; +} + +describe("desktop auto-save", () => { + beforeEach(() => { + vi.useFakeTimers(); + const original = buildProjectFromTemplate("software-project", "Auto-save target"); + const bundle = { + ...original, + projectSettings: { ...original.projectSettings, storageTrust: "folder" as const } + }; + useProjectStore.setState({ + bundle, + storageKey: bundle.project.id, + storagePath: `Client Work/.pm-suite/${bundle.project.id}.pms.json`, + storageTrust: "folder", + externalRevision: 101, + isDirty: true, + saveStatus: "idle", + saveError: null + }); + }); + + afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it("ignores an in-flight failure after the active storage target changes", async () => { + let rejectSave!: (reason: Error) => void; + const pendingSave = new Promise((_resolve, reject) => { + rejectSave = reject; + }); + const save = vi.spyOn(DesktopStorageAdapter.adapter, "save").mockReturnValue(pendingSave); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const projectId = useProjectStore.getState().bundle!.project.id; + + render(); + + await act(async () => { + vi.advanceTimersByTime(251); + await Promise.resolve(); + }); + expect(save).toHaveBeenCalledOnce(); + + const current = useProjectStore.getState().bundle!; + act(() => { + useProjectStore.setState({ + bundle: { + ...current, + projectSettings: { ...current.projectSettings, storageTrust: "browser" } + }, + storagePath: null, + storageTrust: "browser", + externalRevision: 909, + isDirty: false, + saveStatus: "saved", + saveError: null + }); + }); + + await act(async () => { + rejectSave(new Error("Old folder write failed")); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(useProjectStore.getState()).toMatchObject({ + storagePath: null, + storageTrust: "browser", + externalRevision: 909, + isDirty: false, + saveStatus: "saved", + saveError: null + }); + }); +}); diff --git a/apps/desktop/src/platform/auto-save.ts b/apps/desktop/src/platform/auto-save.ts index 4871950..c009e16 100644 --- a/apps/desktop/src/platform/auto-save.ts +++ b/apps/desktop/src/platform/auto-save.ts @@ -40,7 +40,7 @@ export function useAutoSave() { } } catch (err) { const latest = useProjectStore.getState(); - if (latest.storageKey === key) { + if (latest.storageKey === key && latest.storageTrust === targetTrust) { latest.markSaveFailed(err instanceof Error ? err.message : "Auto-save failed."); } console.warn("Auto-save failed:", err); diff --git a/apps/web/src/platform/auto-save.test.tsx b/apps/web/src/platform/auto-save.test.tsx index 42274d2..0ef6e21 100644 --- a/apps/web/src/platform/auto-save.test.tsx +++ b/apps/web/src/platform/auto-save.test.tsx @@ -82,4 +82,53 @@ describe("web auto-save", () => { saveStatus: "saved" }); }); + + it("ignores an in-flight failure after the active storage target changes", async () => { + let rejectSave!: (reason: Error) => void; + const pendingSave = new Promise((_resolve, reject) => { + rejectSave = reject; + }); + const save = vi.spyOn(WebStorageAdapter.adapter, "save").mockReturnValue(pendingSave); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const projectId = useProjectStore.getState().bundle!.project.id; + + render(); + + await act(async () => { + vi.advanceTimersByTime(251); + await Promise.resolve(); + }); + expect(save).toHaveBeenCalledOnce(); + + const current = useProjectStore.getState().bundle!; + act(() => { + useProjectStore.setState({ + bundle: { + ...current, + projectSettings: { ...current.projectSettings, storageTrust: "folder" } + }, + storagePath: `Client Work/.pm-suite/${projectId}.pms.json`, + storageTrust: "folder", + externalRevision: 909, + isDirty: false, + saveStatus: "saved", + saveError: null + }); + }); + + await act(async () => { + rejectSave(new Error("Old browser write failed")); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(useProjectStore.getState()).toMatchObject({ + storagePath: `Client Work/.pm-suite/${projectId}.pms.json`, + storageTrust: "folder", + externalRevision: 909, + isDirty: false, + saveStatus: "saved", + saveError: null + }); + }); }); diff --git a/apps/web/src/platform/auto-save.ts b/apps/web/src/platform/auto-save.ts index 32d9f0a..29607c3 100644 --- a/apps/web/src/platform/auto-save.ts +++ b/apps/web/src/platform/auto-save.ts @@ -42,7 +42,7 @@ export function useAutoSave() { } } catch (err) { const latest = useProjectStore.getState(); - if (latest.storageKey === key) { + if (latest.storageKey === key && latest.storageTrust === targetTrust) { latest.markSaveFailed(err instanceof Error ? err.message : "Auto-save failed."); } console.warn("Auto-save failed:", err); From 5d5f8e19b62530177a87fa2f680dcb2f67c47cd9 Mon Sep 17 00:00:00 2001 From: Zeid Diez Date: Sat, 18 Jul 2026 14:15:32 -0700 Subject: [PATCH 3/5] fix: preserve edits during storage moves Signed-off-by: Zeid Diez --- AI.md | 5 ++ Readme.md | 1 + .../src/views/settings/SettingsView.test.tsx | 76 ++++++++++++++++++- .../ui/src/views/settings/StorageSettings.tsx | 37 ++++++--- 4 files changed, 109 insertions(+), 10 deletions(-) diff --git a/AI.md b/AI.md index 2e18a13..8ed4d54 100644 --- a/AI.md +++ b/AI.md @@ -89,6 +89,7 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - `StorageSettings` is the active-project storage-location surface, not a read-only trust report: a browser-local or unsaved web project can `Save to local folder`, while a folder-backed web project can `Change folder` or `Use browser storage`; each successful transition serializes the bundle with the intended trust, saves through the installed `window.__gph_store` adapter, calls `markSaved()` with returned metadata, and replaces the same-key launcher recent - storage-location transitions deliberately pass a null expected revision because the current revision belongs to the old target rather than the newly selected backend; folder selection first checks `listFolderProjects()` and refuses to overwrite a same-ID `.pms.json` unless it is the already-active folder target - switching storage locations is copy-first and non-destructive: the old folder file or browser recovery copy remains in place, changing back to browser storage does not clear the shared browser folder handle needed by other folder-backed recents, and an `AbortError` from the native directory picker is a normal dismissal that does not mark the project save as failed +- manual storage transitions serialize one target-trust snapshot before writing, then compare that snapshot with the latest active bundle normalized to the same target trust; the returned metadata still activates the new location, but a divergent latest bundle is immediately left dirty via `markUnsaved()` so edits made during the in-flight write are not falsely reported as saved and can auto-save next - web and desktop auto-save success and failure completions must verify that `latest.storageTrust` still matches the target they wrote before applying returned metadata or save-error state; this prevents an in-flight save to the previous location from reverting the active trust/path/revision or marking the newly active target as failed after a storage transition. Folder-to-folder moves are also disabled while the current folder project is dirty, because choosing a new browser folder handle changes the adapter target immediately. - the web shell now surfaces offline status and emits one lightweight due-item/reminder notification per project/day/count combination; local/self-hosted web builds capture `beforeinstallprompt` and show `Install app`, while hosted-demo builds suppress that install prompt and show a `Run locally` GitHub setup CTA instead - desktop storage now only writes to the filesystem when a folder path has actually been attached; otherwise the desktop shell behaves as browser-local storage on purpose instead of pretending to be folder-backed @@ -717,6 +718,10 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - requiring both the active `storageKey` and `storageTrust` to match the target captured by web and desktop auto-save before a rejected write can call `markSaveFailed()` - leaving the new target's `saveStatus`, `saveError`, dirty state, path, and external revision untouched when an older browser or folder write rejects after the user changes storage locations - adding rejection-path auto-save regressions for browser-to-folder web transitions and folder-to-browser desktop transitions, complementing the existing successful-completion race coverage +- addressed the follow-up Greptile concurrent-edit finding by: + - capturing the exact target-normalized JSON written by `StorageSettings.persistTo()` and comparing it with the latest active bundle after the adapter resolves + - activating successful folder/browser metadata while retaining `isDirty: true` and `saveStatus: "idle"` when a project edit landed during the write, allowing the normal auto-save bridge to persist the newer state next + - guarding successful and failed completion by the original project id and storage key so a late storage transition cannot mutate or show a save failure on a different project, and adding a focused Settings regression that proves the newer work item remains present but absent from the earlier saved snapshot ## Open follow-on planning diff --git a/Readme.md b/Readme.md index df890af..2d3f12c 100644 --- a/Readme.md +++ b/Readme.md @@ -84,6 +84,7 @@ Important behavior: - New folder-backed projects write their initial `.pm-suite/.pms.json` immediately. - An open browser-local project can move to a chosen folder from **Settings -> Storage** without being recreated; a folder-backed project can change folders or switch back to browser storage from the same panel. - Changing storage locations writes and activates the new copy first. The previous browser or folder copy is retained as a recovery point, and Grillo refuses to overwrite a same-ID project already present in a newly selected folder. +- If an edit arrives while that location change is still writing, Grillo activates the new destination but keeps the newer edit marked unsaved so the next auto-save includes it. - Auto-save results belong to the storage target that started them. If the user changes targets while a save is in flight, its later success or failure cannot overwrite the new target's saved state. - Folder-backed browser saves keep a browser-local recovery copy so reloads are recoverable when the browser cannot restore folder access. - Reopening folder-backed recents asks for folder access and refuses to silently open stale browser recovery when the selected folder is missing the recorded project file. diff --git a/packages/ui/src/views/settings/SettingsView.test.tsx b/packages/ui/src/views/settings/SettingsView.test.tsx index bedc8d6..dc27efe 100644 --- a/packages/ui/src/views/settings/SettingsView.test.tsx +++ b/packages/ui/src/views/settings/SettingsView.test.tsx @@ -1,4 +1,4 @@ -import { cleanup, render, screen, waitFor, within } from "@testing-library/react"; +import { act, cleanup, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { MemoryRouter } from "react-router-dom"; @@ -7,6 +7,7 @@ import { exportProjectJson, importProjectJson, type PersistedStorageTrust, + type StorageMetadata, type ProjectStoreAdapter } from "@gph/core"; import { useProjectStore } from "../../store/project-store"; @@ -279,6 +280,79 @@ describe("SettingsView", () => { expect(within(panel).getByText("Browser-local")).toBeInTheDocument(); }); + it("keeps edits made during a storage move dirty after the earlier snapshot is saved", async () => { + let resolveSave!: (metadata: StorageMetadata) => void; + const pendingSave = new Promise((resolve) => { + resolveSave = resolve; + }); + const save = vi.fn(() => pendingSave); + const adapter: ProjectStoreAdapter = { + capabilities: { folderBacked: true, fileWatch: false, attachments: true }, + list: async () => [], + has: async () => true, + load: async () => null, + save, + delete: async () => undefined, + chooseFolder: async () => "Client Work", + getCurrentFolderDisplay: async () => null, + listFolderProjects: async () => [] + }; + (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store = adapter; + const projectId = useProjectStore.getState().bundle!.project.id; + + render( + + + + + + ); + + await userEvent.click(screen.getByRole("tab", { name: "Storage" })); + const panel = screen.getByRole("tabpanel", { name: "Storage" }); + await userEvent.click(within(panel).getByRole("button", { name: "Save to local folder" })); + await waitFor(() => expect(save).toHaveBeenCalledOnce()); + const savedJson = save.mock.calls[0][1]; + + act(() => { + useProjectStore.getState().applyCommand({ + type: "item.create", + projectId, + typeId: "task", + title: "Edit made during storage move" + }); + }); + + await act(async () => { + resolveSave({ + key: projectId, + displayPath: `Client Work/.pm-suite/${projectId}.pms.json`, + externalRevision: 202, + trust: "folder" + }); + await pendingSave; + await Promise.resolve(); + }); + + await waitFor(() => { + expect(useProjectStore.getState()).toMatchObject({ + storagePath: `Client Work/.pm-suite/${projectId}.pms.json`, + storageTrust: "folder", + externalRevision: 202, + isDirty: true, + saveStatus: "idle", + saveError: null + }); + }); + expect(useProjectStore.getState().bundle?.core.items).toEqual( + expect.arrayContaining([expect.objectContaining({ title: "Edit made during storage move" })]) + ); + expect(importProjectJson(savedJson).bundle.core.items).not.toEqual( + expect.arrayContaining([expect.objectContaining({ title: "Edit made during storage move" })]) + ); + expect(within(panel).getByText(/Newer edits remain unsaved/i)).toBeInTheDocument(); + }); + it("does not overwrite an existing folder project or treat picker cancellation as a save error", async () => { const bundle = useProjectStore.getState().bundle!; const save = vi.fn(); diff --git a/packages/ui/src/views/settings/StorageSettings.tsx b/packages/ui/src/views/settings/StorageSettings.tsx index a511ff6..1fd1796 100644 --- a/packages/ui/src/views/settings/StorageSettings.tsx +++ b/packages/ui/src/views/settings/StorageSettings.tsx @@ -55,6 +55,7 @@ export function StorageSettings() { const saveStatus = useProjectStore((state) => state.saveStatus); const markSaving = useProjectStore((state) => state.markSaving); const markSaved = useProjectStore((state) => state.markSaved); + const markUnsaved = useProjectStore((state) => state.markUnsaved); const markSaveFailed = useProjectStore((state) => state.markSaveFailed); const recordRecent = useWorkspaceStore((state) => state.recordRecent); const [operation, setOperation] = useState(null); @@ -90,20 +91,31 @@ export function StorageSettings() { return false; } + const key = current.storageKey ?? currentBundle.project.id; + const projectId = currentBundle.project.id; + const json = exportProjectJson(withStorageTrust(currentBundle, targetTrust)); markSaving(); try { - const key = current.storageKey ?? currentBundle.project.id; const metadata = await adapter.save( key, - exportProjectJson(withStorageTrust(currentBundle, targetTrust)), + json, null, targetTrust ); - markSaved(metadata.key, metadata.displayPath, metadata.trust, metadata.externalRevision); + const latest = useProjectStore.getState(); + const latestKey = latest.storageKey ?? latest.bundle?.project.id; + const isStillActive = latest.bundle?.project.id === projectId && latestKey === key; + const hasNewerEdits = Boolean( + isStillActive && latest.bundle && exportProjectJson(withStorageTrust(latest.bundle, targetTrust)) !== json + ); + if (isStillActive) { + markSaved(metadata.key, metadata.displayPath, metadata.trust, metadata.externalRevision); + if (hasNewerEdits) markUnsaved(metadata.externalRevision); + } if (metadata.trust !== "unsaved") { recordRecent({ key: metadata.key, - name: currentBundle.project.name, + name: isStillActive && latest.bundle ? latest.bundle.project.name : currentBundle.project.name, storagePath: metadata.displayPath, trust: metadata.trust, lastOpenedAt: new Date().toISOString() @@ -112,15 +124,22 @@ export function StorageSettings() { const destination = targetTrust === "folder" ? `local folder${folderName ? ` ${folderName}` : ""}` : "browser storage"; - const message = `Saved ${currentBundle.project.name} to ${destination}.`; - setFeedback({ message, tone: "success" }); - notify({ tone: "success", message }); + const message = hasNewerEdits + ? `Saved ${currentBundle.project.name} to ${destination}. Newer edits remain unsaved and will auto-save next.` + : `Saved ${currentBundle.project.name} to ${destination}.`; + const tone: InlineAlertTone = hasNewerEdits ? "warning" : "success"; + setFeedback({ message, tone }); + notify({ tone, message }); return true; } catch (error) { const detail = error instanceof Error ? error.message : "The save did not complete."; const message = `Storage change failed: ${detail}`; - markSaveFailed(detail); - showFailure(message); + const latest = useProjectStore.getState(); + const latestKey = latest.storageKey ?? latest.bundle?.project.id; + if (latest.bundle?.project.id === projectId && latestKey === key) { + markSaveFailed(detail); + showFailure(message); + } return false; } }; From 5b273ce4640f85ad9b4ae34313891a9023654380 Mon Sep 17 00:00:00 2001 From: Zeid Diez Date: Sat, 18 Jul 2026 14:31:01 -0700 Subject: [PATCH 4/5] fix: restore rejected folder selections Signed-off-by: Zeid Diez --- AI.md | 6 ++- Readme.md | 2 +- .../src/platform/storage/web-storage.test.ts | 22 ++++++++++ apps/web/src/platform/storage/web-storage.ts | 17 +++++++ packages/core/src/storage/store.ts | 2 + .../src/views/settings/SettingsView.test.tsx | 44 +++++++++++++++++++ .../ui/src/views/settings/StorageSettings.tsx | 13 +++++- 7 files changed, 103 insertions(+), 3 deletions(-) diff --git a/AI.md b/AI.md index 8ed4d54..efa1bad 100644 --- a/AI.md +++ b/AI.md @@ -87,7 +87,7 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - the shared shell wraps routes in `ToastProvider`; route code should use `useToast()` for short-lived feedback and keep blocking or recoverable errors in `InlineAlert` - the shared shell exposes explicit project actions next to the save-state indicator: `Save now` / `Retry save` routes through the active `ProjectStoreAdapter.save()` plus `markSaving()` / `markSaved()` / `markSaveFailed()`, records a launcher recent from successful browser/folder save metadata, `Switch project` navigates to `/projects`, and `Close project` returns to the launcher immediately only for clean saved projects; dirty projects and unsaved in-memory imports/demos must confirm `Close without saving` before `closeProject()` runs - `StorageSettings` is the active-project storage-location surface, not a read-only trust report: a browser-local or unsaved web project can `Save to local folder`, while a folder-backed web project can `Change folder` or `Use browser storage`; each successful transition serializes the bundle with the intended trust, saves through the installed `window.__gph_store` adapter, calls `markSaved()` with returned metadata, and replaces the same-key launcher recent -- storage-location transitions deliberately pass a null expected revision because the current revision belongs to the old target rather than the newly selected backend; folder selection first checks `listFolderProjects()` and refuses to overwrite a same-ID `.pms.json` unless it is the already-active folder target +- storage-location transitions deliberately pass a null expected revision because the current revision belongs to the old target rather than the newly selected backend; folder selection checks `listFolderProjects()` and refuses to overwrite a same-ID `.pms.json` unless it is the already-active folder target, then calls the adapter's optional `restorePreviousFolder()` rollback after collisions, scan errors, failed writes, or an active-project switch so an unsuccessful transition cannot leave future folder operations bound to the unaccepted directory - switching storage locations is copy-first and non-destructive: the old folder file or browser recovery copy remains in place, changing back to browser storage does not clear the shared browser folder handle needed by other folder-backed recents, and an `AbortError` from the native directory picker is a normal dismissal that does not mark the project save as failed - manual storage transitions serialize one target-trust snapshot before writing, then compare that snapshot with the latest active bundle normalized to the same target trust; the returned metadata still activates the new location, but a divergent latest bundle is immediately left dirty via `markUnsaved()` so edits made during the in-flight write are not falsely reported as saved and can auto-save next - web and desktop auto-save success and failure completions must verify that `latest.storageTrust` still matches the target they wrote before applying returned metadata or save-error state; this prevents an in-flight save to the previous location from reverting the active trust/path/revision or marking the newly active target as failed after a storage transition. Folder-to-folder moves are also disabled while the current folder project is dirty, because choosing a new browser folder handle changes the adapter target immediately. @@ -722,6 +722,10 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - capturing the exact target-normalized JSON written by `StorageSettings.persistTo()` and comparing it with the latest active bundle after the adapter resolves - activating successful folder/browser metadata while retaining `isDirty: true` and `saveStatus: "idle"` when a project edit landed during the write, allowing the normal auto-save bridge to persist the newer state next - guarding successful and failed completion by the original project id and storage key so a late storage transition cannot mutate or show a save failure on a different project, and adding a focused Settings regression that proves the newer work item remains present but absent from the earlier saved snapshot +- addressed the follow-up Greptile rejected-folder binding finding by: + - extending `ProjectStoreAdapter` with optional `restorePreviousFolder()` rollback semantics for consumers that validate a directory after the native picker returns + - checkpointing the prior web `FileSystemDirectoryHandle` before each accepted pick and restoring both the active in-memory handle and best-effort IndexedDB binding when Settings rejects a same-ID collision + - invoking rollback before showing the collision warning and after any scan/save failure or active-project switch, with web-adapter coverage proving scans return to the accepted folder plus Settings coverage for collision, picker cancellation, and failed-write behavior ## Open follow-on planning diff --git a/Readme.md b/Readme.md index 2d3f12c..da52954 100644 --- a/Readme.md +++ b/Readme.md @@ -83,7 +83,7 @@ Important behavior: - New folder-backed projects write their initial `.pm-suite/.pms.json` immediately. - An open browser-local project can move to a chosen folder from **Settings -> Storage** without being recreated; a folder-backed project can change folders or switch back to browser storage from the same panel. -- Changing storage locations writes and activates the new copy first. The previous browser or folder copy is retained as a recovery point, and Grillo refuses to overwrite a same-ID project already present in a newly selected folder. +- Changing storage locations writes and activates the new copy first. The previous browser or folder copy is retained as a recovery point, and a collision or failed move restores the previously accepted folder binding instead of leaving later saves pointed at a rejected directory. - If an edit arrives while that location change is still writing, Grillo activates the new destination but keeps the newer edit marked unsaved so the next auto-save includes it. - Auto-save results belong to the storage target that started them. If the user changes targets while a save is in flight, its later success or failure cannot overwrite the new target's saved state. - Folder-backed browser saves keep a browser-local recovery copy so reloads are recoverable when the browser cannot restore folder access. diff --git a/apps/web/src/platform/storage/web-storage.test.ts b/apps/web/src/platform/storage/web-storage.test.ts index 49de8f3..9b0d0e8 100644 --- a/apps/web/src/platform/storage/web-storage.test.ts +++ b/apps/web/src/platform/storage/web-storage.test.ts @@ -125,6 +125,28 @@ describe("WebStorageAdapter folder mode", () => { await expect(adapter.getCurrentFolderDisplay?.()).resolves.toBeNull(); }); + it("restores the previous folder after a newly picked folder is rejected", async () => { + const accepted = createFakeFolder("Accepted Work", { "accepted.pms.json": "{}" }); + const rejected = createFakeFolder("Rejected Work", { "rejected.pms.json": "{}" }); + Object.defineProperty(window, "showDirectoryPicker", { + configurable: true, + value: vi.fn() + .mockResolvedValueOnce(accepted.handle) + .mockResolvedValueOnce(rejected.handle) + }); + const adapter = await getAdapter(); + + await expect(adapter.chooseFolder?.()).resolves.toBe("Accepted Work"); + await expect(adapter.chooseFolder?.()).resolves.toBe("Rejected Work"); + await expect(adapter.getCurrentFolderDisplay?.()).resolves.toBe("Rejected Work"); + await expect(adapter.listFolderProjects?.()).resolves.toEqual(["rejected.pms.json"]); + + await adapter.restorePreviousFolder?.(); + + await expect(adapter.getCurrentFolderDisplay?.()).resolves.toBe("Accepted Work"); + await expect(adapter.listFolderProjects?.()).resolves.toEqual(["accepted.pms.json"]); + }); + it("rejects a stale folder save after the project file changes externally", async () => { const bundle = buildProjectFromTemplate("software-project", "Original"); const originalJson = exportProjectJson(bundle); diff --git a/apps/web/src/platform/storage/web-storage.ts b/apps/web/src/platform/storage/web-storage.ts index 62f48f8..1beee8c 100644 --- a/apps/web/src/platform/storage/web-storage.ts +++ b/apps/web/src/platform/storage/web-storage.ts @@ -180,6 +180,7 @@ function folderDisplayPath(handle: FileSystemDirectoryHandle, key: string): stri class WebLocalStorageAdapter implements ProjectStoreAdapter { capabilities = { folderBacked: supportsFolderAccess(), fileWatch: false, attachments: true }; + private previousFolderHandle: FileSystemDirectoryHandle | null | undefined; async list(): Promise { return readIndex(); @@ -348,9 +349,11 @@ class WebLocalStorageAdapter implements ProjectStoreAdapter { async chooseFolder(): Promise { const picker = supportsFolderAccess() ? window.showDirectoryPicker : undefined; if (!picker) return null; + const previousHandle = await readStoredFolderHandle(); const handle = await picker.call(window, { mode: "readwrite" }); const granted = await ensureFolderPermission(handle, "readwrite", true); if (!granted) return null; + this.previousFolderHandle = previousHandle; activeFolderHandle = handle; try { await writeStoredFolderHandle(handle); @@ -359,11 +362,25 @@ class WebLocalStorageAdapter implements ProjectStoreAdapter { } return handle.name; } + async restorePreviousFolder(): Promise { + if (this.previousFolderHandle === undefined) return; + const previousHandle = this.previousFolderHandle; + this.previousFolderHandle = undefined; + activeFolderHandle = previousHandle; + try { + if (previousHandle) await writeStoredFolderHandle(previousHandle); + else await deleteStoredFolderHandle(); + } catch { + // Restoring the in-memory handle keeps this session on the accepted folder; + // durable handle persistence remains best-effort, matching chooseFolder(). + } + } async getCurrentFolderDisplay(): Promise { const handle = await readStoredFolderHandle(); return handle?.name ?? null; } async clearFolder(): Promise { + this.previousFolderHandle = undefined; activeFolderHandle = null; try { await deleteStoredFolderHandle(); diff --git a/packages/core/src/storage/store.ts b/packages/core/src/storage/store.ts index 01d621b..f65f4a2 100644 --- a/packages/core/src/storage/store.ts +++ b/packages/core/src/storage/store.ts @@ -63,6 +63,8 @@ export type ProjectStoreAdapter = { delete(key: string): Promise; /** Optional folder-picking surface for runtimes that can bind a durable local directory. */ chooseFolder?(): Promise; + /** Optional rollback for the most recent successful folder pick when validation rejects it. */ + restorePreviousFolder?(): Promise; /** Optional human-readable display name for the currently selected folder. */ getCurrentFolderDisplay?(): Promise; /** Optional reset that makes subsequent explicitly browser-local saves ignore a selected folder. */ diff --git a/packages/ui/src/views/settings/SettingsView.test.tsx b/packages/ui/src/views/settings/SettingsView.test.tsx index dc27efe..81a80d9 100644 --- a/packages/ui/src/views/settings/SettingsView.test.tsx +++ b/packages/ui/src/views/settings/SettingsView.test.tsx @@ -356,6 +356,7 @@ describe("SettingsView", () => { it("does not overwrite an existing folder project or treat picker cancellation as a save error", async () => { const bundle = useProjectStore.getState().bundle!; const save = vi.fn(); + const restorePreviousFolder = vi.fn(async () => undefined); const chooseFolder = vi .fn<() => Promise>() .mockResolvedValueOnce("Existing Work") @@ -368,6 +369,7 @@ describe("SettingsView", () => { save, delete: async () => undefined, chooseFolder, + restorePreviousFolder, listFolderProjects: async () => [`${bundle.project.id}.pms.json`] }; (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store = adapter; @@ -387,14 +389,56 @@ describe("SettingsView", () => { expect(await within(panel).findByText(/already contains .*\.pms\.json/i)).toBeInTheDocument(); expect(save).not.toHaveBeenCalled(); + expect(restorePreviousFolder).toHaveBeenCalledOnce(); expect(useProjectStore.getState()).toMatchObject({ storageTrust: "browser", saveStatus: "idle" }); await userEvent.click(saveToFolder); await waitFor(() => expect(chooseFolder).toHaveBeenCalledTimes(2)); expect(save).not.toHaveBeenCalled(); + expect(restorePreviousFolder).toHaveBeenCalledOnce(); expect(useProjectStore.getState()).toMatchObject({ storageTrust: "browser", saveError: null }); }); + it("restores the previous folder when saving to a newly selected folder fails", async () => { + const restorePreviousFolder = vi.fn(async () => undefined); + const adapter: ProjectStoreAdapter = { + capabilities: { folderBacked: true, fileWatch: false, attachments: true }, + list: async () => [], + has: async () => true, + load: async () => null, + save: async () => { + throw new Error("Folder write failed"); + }, + delete: async () => undefined, + chooseFolder: async () => "Unavailable Work", + restorePreviousFolder, + listFolderProjects: async () => [] + }; + (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store = adapter; + + render( + + + + + + ); + + await userEvent.click(screen.getByRole("tab", { name: "Storage" })); + const panel = screen.getByRole("tabpanel", { name: "Storage" }); + await userEvent.click(within(panel).getByRole("button", { name: "Save to local folder" })); + + expect(await within(panel).findByText(/Storage change failed: Folder write failed/i)).toBeInTheDocument(); + expect(restorePreviousFolder).toHaveBeenCalledOnce(); + expect(useProjectStore.getState()).toMatchObject({ + storageTrust: "browser", + storagePath: null, + isDirty: true, + saveStatus: "error", + saveError: "Folder write failed" + }); + }); + it("supports keyboard navigation across settings sections", async () => { render( diff --git a/packages/ui/src/views/settings/StorageSettings.tsx b/packages/ui/src/views/settings/StorageSettings.tsx index 1fd1796..5d394f3 100644 --- a/packages/ui/src/views/settings/StorageSettings.tsx +++ b/packages/ui/src/views/settings/StorageSettings.tsx @@ -121,6 +121,7 @@ export function StorageSettings() { lastOpenedAt: new Date().toISOString() }); } + if (!isStillActive) return false; const destination = targetTrust === "folder" ? `local folder${folderName ? ` ${folderName}` : ""}` : "browser storage"; @@ -148,6 +149,12 @@ export function StorageSettings() { if (!adapter?.chooseFolder) return; setOperation("folder"); setFeedback(null); + let folderWasSelected = false; + const restoreRejectedFolder = async () => { + if (!folderWasSelected) return; + folderWasSelected = false; + await adapter.restorePreviousFolder?.(); + }; try { const folderName = await adapter.chooseFolder(); if (!folderName) { @@ -157,6 +164,7 @@ export function StorageSettings() { }); return; } + folderWasSelected = true; const current = useProjectStore.getState(); const key = current.storageKey ?? current.bundle?.project.id ?? bundle.project.id; @@ -166,6 +174,7 @@ export function StorageSettings() { ); const currentTarget = current.storageTrust === "folder" && isCurrentFolderTarget(current.storagePath, folderName, key); if (projectAlreadyExists && !currentTarget) { + await restoreRejectedFolder(); setFeedback({ message: `That folder already contains ${key}.pms.json. Grillo did not overwrite it; open that project from the workspace launcher or choose a different folder.`, tone: "warning" @@ -173,8 +182,10 @@ export function StorageSettings() { return; } - await persistTo("folder", folderName); + const activated = await persistTo("folder", folderName); + if (!activated) await restoreRejectedFolder(); } catch (error) { + await restoreRejectedFolder(); if (isFolderPickerDismissal(error)) return; const detail = error instanceof Error ? error.message : "Folder access did not complete."; showFailure(`Folder access failed: ${detail}`); From 34c622d8ff3beadb29e2f0deea716526b147200b Mon Sep 17 00:00:00 2001 From: Zeid Diez Date: Sat, 18 Jul 2026 14:52:45 -0700 Subject: [PATCH 5/5] fix: verify selected folder identity Signed-off-by: Zeid Diez --- AI.md | 6 +- Readme.md | 2 +- .../src/platform/storage/web-storage.test.ts | 23 +++++ apps/web/src/platform/storage/web-storage.ts | 18 ++++ packages/core/src/storage/store.ts | 2 + .../src/views/settings/SettingsView.test.tsx | 98 +++++++++++++++++++ .../ui/src/views/settings/StorageSettings.tsx | 26 ++--- 7 files changed, 160 insertions(+), 15 deletions(-) diff --git a/AI.md b/AI.md index efa1bad..4e34972 100644 --- a/AI.md +++ b/AI.md @@ -87,7 +87,7 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - the shared shell wraps routes in `ToastProvider`; route code should use `useToast()` for short-lived feedback and keep blocking or recoverable errors in `InlineAlert` - the shared shell exposes explicit project actions next to the save-state indicator: `Save now` / `Retry save` routes through the active `ProjectStoreAdapter.save()` plus `markSaving()` / `markSaved()` / `markSaveFailed()`, records a launcher recent from successful browser/folder save metadata, `Switch project` navigates to `/projects`, and `Close project` returns to the launcher immediately only for clean saved projects; dirty projects and unsaved in-memory imports/demos must confirm `Close without saving` before `closeProject()` runs - `StorageSettings` is the active-project storage-location surface, not a read-only trust report: a browser-local or unsaved web project can `Save to local folder`, while a folder-backed web project can `Change folder` or `Use browser storage`; each successful transition serializes the bundle with the intended trust, saves through the installed `window.__gph_store` adapter, calls `markSaved()` with returned metadata, and replaces the same-key launcher recent -- storage-location transitions deliberately pass a null expected revision because the current revision belongs to the old target rather than the newly selected backend; folder selection checks `listFolderProjects()` and refuses to overwrite a same-ID `.pms.json` unless it is the already-active folder target, then calls the adapter's optional `restorePreviousFolder()` rollback after collisions, scan errors, failed writes, or an active-project switch so an unsuccessful transition cannot leave future folder operations bound to the unaccepted directory +- storage-location transitions deliberately pass a null expected revision because the current revision belongs to the old target rather than the newly selected backend; folder selection checks `listFolderProjects()` and refuses to overwrite a same-ID `.pms.json` unless the adapter proves the selected directory is the same filesystem entry as the previous active binding through optional `isSelectedFolderSameAsPrevious()` semantics—display names and saved path strings are never folder identities. A confirmed re-selection of the current folder is a no-op rather than a blind rewrite, while collisions, scan errors, failed writes, and active-project switches call `restorePreviousFolder()` so an unsuccessful transition cannot leave future folder operations bound to the unaccepted directory - switching storage locations is copy-first and non-destructive: the old folder file or browser recovery copy remains in place, changing back to browser storage does not clear the shared browser folder handle needed by other folder-backed recents, and an `AbortError` from the native directory picker is a normal dismissal that does not mark the project save as failed - manual storage transitions serialize one target-trust snapshot before writing, then compare that snapshot with the latest active bundle normalized to the same target trust; the returned metadata still activates the new location, but a divergent latest bundle is immediately left dirty via `markUnsaved()` so edits made during the in-flight write are not falsely reported as saved and can auto-save next - web and desktop auto-save success and failure completions must verify that `latest.storageTrust` still matches the target they wrote before applying returned metadata or save-error state; this prevents an in-flight save to the previous location from reverting the active trust/path/revision or marking the newly active target as failed after a storage transition. Folder-to-folder moves are also disabled while the current folder project is dirty, because choosing a new browser folder handle changes the adapter target immediately. @@ -726,6 +726,10 @@ The core domain in `packages/core/src/domain/` covers the entities the plan call - extending `ProjectStoreAdapter` with optional `restorePreviousFolder()` rollback semantics for consumers that validate a directory after the native picker returns - checkpointing the prior web `FileSystemDirectoryHandle` before each accepted pick and restoring both the active in-memory handle and best-effort IndexedDB binding when Settings rejects a same-ID collision - invoking rollback before showing the collision warning and after any scan/save failure or active-project switch, with web-adapter coverage proving scans return to the accepted folder plus Settings coverage for collision, picker cancellation, and failed-write behavior +- addressed the follow-up Greptile same-name folder identity finding by: + - removing the `StorageSettings` path-suffix/display-name heuristic that could mistake two distinct directories with the same basename for one folder target + - adding optional `ProjectStoreAdapter.isSelectedFolderSameAsPrevious()` semantics and implementing the web comparison with `FileSystemHandle.isSameEntry()`; unavailable or failed identity checks conservatively remain collisions + - covering same-name distinct handle comparisons in the web adapter plus both Settings outcomes: distinct directories retain the collision warning and restore the accepted binding, while a confirmed re-selection of the current directory performs no write ## Open follow-on planning diff --git a/Readme.md b/Readme.md index da52954..8affd4c 100644 --- a/Readme.md +++ b/Readme.md @@ -83,7 +83,7 @@ Important behavior: - New folder-backed projects write their initial `.pm-suite/.pms.json` immediately. - An open browser-local project can move to a chosen folder from **Settings -> Storage** without being recreated; a folder-backed project can change folders or switch back to browser storage from the same panel. -- Changing storage locations writes and activates the new copy first. The previous browser or folder copy is retained as a recovery point, and a collision or failed move restores the previously accepted folder binding instead of leaving later saves pointed at a rejected directory. +- Changing storage locations writes and activates the new copy first. The previous browser or folder copy is retained as a recovery point, and a collision or failed move restores the previously accepted folder binding instead of leaving later saves pointed at a rejected directory. Folder identity is checked by the browser's filesystem handles, so two different folders with the same name cannot bypass the same-project collision guard; reselecting the actual current folder is recognized without rewriting its file. - If an edit arrives while that location change is still writing, Grillo activates the new destination but keeps the newer edit marked unsaved so the next auto-save includes it. - Auto-save results belong to the storage target that started them. If the user changes targets while a save is in flight, its later success or failure cannot overwrite the new target's saved state. - Folder-backed browser saves keep a browser-local recovery copy so reloads are recoverable when the browser cannot restore folder access. diff --git a/apps/web/src/platform/storage/web-storage.test.ts b/apps/web/src/platform/storage/web-storage.test.ts index 9b0d0e8..e889a6f 100644 --- a/apps/web/src/platform/storage/web-storage.test.ts +++ b/apps/web/src/platform/storage/web-storage.test.ts @@ -45,6 +45,7 @@ function createFakeFolder(name: string, seed: Record = {}): Fake name, queryPermission: async () => "granted", requestPermission: async () => "granted", + isSameEntry: async (other: FileSystemHandle) => other === handle, getDirectoryHandle: async (dirname: string, options?: FileSystemGetDirectoryOptions) => { if (dirname === ".pm-suite") return projectDir; if (options?.create) throw new Error("Only .pm-suite is supported by this test handle"); @@ -147,6 +148,28 @@ describe("WebStorageAdapter folder mode", () => { await expect(adapter.listFolderProjects?.()).resolves.toEqual(["accepted.pms.json"]); }); + it("compares folder picks by handle identity instead of their display names", async () => { + const accepted = createFakeFolder("Shared Name"); + const different = createFakeFolder("Shared Name"); + Object.defineProperty(window, "showDirectoryPicker", { + configurable: true, + value: vi.fn() + .mockResolvedValueOnce(accepted.handle) + .mockResolvedValueOnce(accepted.handle) + .mockResolvedValueOnce(different.handle) + }); + const adapter = await getAdapter(); + + await adapter.chooseFolder?.(); + await expect(adapter.isSelectedFolderSameAsPrevious?.()).resolves.toBe(false); + + await adapter.chooseFolder?.(); + await expect(adapter.isSelectedFolderSameAsPrevious?.()).resolves.toBe(true); + + await adapter.chooseFolder?.(); + await expect(adapter.isSelectedFolderSameAsPrevious?.()).resolves.toBe(false); + }); + it("rejects a stale folder save after the project file changes externally", async () => { const bundle = buildProjectFromTemplate("software-project", "Original"); const originalJson = exportProjectJson(bundle); diff --git a/apps/web/src/platform/storage/web-storage.ts b/apps/web/src/platform/storage/web-storage.ts index 1beee8c..8444d37 100644 --- a/apps/web/src/platform/storage/web-storage.ts +++ b/apps/web/src/platform/storage/web-storage.ts @@ -375,6 +375,24 @@ class WebLocalStorageAdapter implements ProjectStoreAdapter { // durable handle persistence remains best-effort, matching chooseFolder(). } } + async isSelectedFolderSameAsPrevious(): Promise { + const selectedHandle = activeFolderHandle; + const previousHandle = this.previousFolderHandle; + if (!selectedHandle || !previousHandle) return false; + const isSameEntry = ( + selectedHandle as FileSystemDirectoryHandle & { + isSameEntry?: (other: FileSystemHandle) => Promise; + } + ).isSameEntry; + if (!isSameEntry) return false; + try { + return await isSameEntry.call(selectedHandle, previousHandle); + } catch { + // Display names are not identities. If the browser cannot compare handles, + // callers must conservatively treat an existing same-ID file as a collision. + return false; + } + } async getCurrentFolderDisplay(): Promise { const handle = await readStoredFolderHandle(); return handle?.name ?? null; diff --git a/packages/core/src/storage/store.ts b/packages/core/src/storage/store.ts index f65f4a2..fce1735 100644 --- a/packages/core/src/storage/store.ts +++ b/packages/core/src/storage/store.ts @@ -65,6 +65,8 @@ export type ProjectStoreAdapter = { chooseFolder?(): Promise; /** Optional rollback for the most recent successful folder pick when validation rejects it. */ restorePreviousFolder?(): Promise; + /** Optional identity check for the latest folder pick against the binding it replaced. */ + isSelectedFolderSameAsPrevious?(): Promise; /** Optional human-readable display name for the currently selected folder. */ getCurrentFolderDisplay?(): Promise; /** Optional reset that makes subsequent explicitly browser-local saves ignore a selected folder. */ diff --git a/packages/ui/src/views/settings/SettingsView.test.tsx b/packages/ui/src/views/settings/SettingsView.test.tsx index 81a80d9..ab7c30b 100644 --- a/packages/ui/src/views/settings/SettingsView.test.tsx +++ b/packages/ui/src/views/settings/SettingsView.test.tsx @@ -399,6 +399,104 @@ describe("SettingsView", () => { expect(useProjectStore.getState()).toMatchObject({ storageTrust: "browser", saveError: null }); }); + it("does not trust a matching folder name when a different folder contains the same project id", async () => { + const bundle = useProjectStore.getState().bundle!; + const storagePath = `Shared Name/.pm-suite/${bundle.project.id}.pms.json`; + useProjectStore.setState({ + storagePath, + storageTrust: "folder", + isDirty: false, + saveStatus: "saved" + }); + const save = vi.fn(); + const restorePreviousFolder = vi.fn(async () => undefined); + const isSelectedFolderSameAsPrevious = vi.fn(async () => false); + const adapter: ProjectStoreAdapter = { + capabilities: { folderBacked: true, fileWatch: false, attachments: true }, + list: async () => [], + has: async () => true, + load: async () => null, + save, + delete: async () => undefined, + chooseFolder: async () => "Shared Name", + restorePreviousFolder, + isSelectedFolderSameAsPrevious, + listFolderProjects: async () => [`${bundle.project.id}.pms.json`] + }; + (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store = adapter; + + render( + + + + + + ); + + await userEvent.click(screen.getByRole("tab", { name: "Storage" })); + const panel = screen.getByRole("tabpanel", { name: "Storage" }); + await userEvent.click(within(panel).getByRole("button", { name: "Change folder" })); + + expect(await within(panel).findByText(/already contains .*\.pms\.json/i)).toBeInTheDocument(); + expect(isSelectedFolderSameAsPrevious).toHaveBeenCalledOnce(); + expect(save).not.toHaveBeenCalled(); + expect(restorePreviousFolder).toHaveBeenCalledOnce(); + expect(useProjectStore.getState()).toMatchObject({ + storagePath, + storageTrust: "folder", + saveStatus: "saved" + }); + }); + + it("treats a confirmed re-selection of the active folder as a no-op", async () => { + const bundle = useProjectStore.getState().bundle!; + const storagePath = `Client Work/.pm-suite/${bundle.project.id}.pms.json`; + useProjectStore.setState({ + storagePath, + storageTrust: "folder", + isDirty: false, + saveStatus: "saved" + }); + const save = vi.fn(); + const restorePreviousFolder = vi.fn(async () => undefined); + const isSelectedFolderSameAsPrevious = vi.fn(async () => true); + const adapter: ProjectStoreAdapter = { + capabilities: { folderBacked: true, fileWatch: false, attachments: true }, + list: async () => [], + has: async () => true, + load: async () => null, + save, + delete: async () => undefined, + chooseFolder: async () => "Client Work", + restorePreviousFolder, + isSelectedFolderSameAsPrevious, + listFolderProjects: async () => [`${bundle.project.id}.pms.json`] + }; + (window as typeof window & { __gph_store?: ProjectStoreAdapter }).__gph_store = adapter; + + render( + + + + + + ); + + await userEvent.click(screen.getByRole("tab", { name: "Storage" })); + const panel = screen.getByRole("tabpanel", { name: "Storage" }); + await userEvent.click(within(panel).getByRole("button", { name: "Change folder" })); + + expect(await within(panel).findByText(/already saved in Client Work\. No files were changed/i)).toBeInTheDocument(); + expect(isSelectedFolderSameAsPrevious).toHaveBeenCalledOnce(); + expect(save).not.toHaveBeenCalled(); + expect(restorePreviousFolder).not.toHaveBeenCalled(); + expect(useProjectStore.getState()).toMatchObject({ + storagePath, + storageTrust: "folder", + saveStatus: "saved" + }); + }); + it("restores the previous folder when saving to a newly selected folder fails", async () => { const restorePreviousFolder = vi.fn(async () => undefined); const adapter: ProjectStoreAdapter = { diff --git a/packages/ui/src/views/settings/StorageSettings.tsx b/packages/ui/src/views/settings/StorageSettings.tsx index 5d394f3..d28d735 100644 --- a/packages/ui/src/views/settings/StorageSettings.tsx +++ b/packages/ui/src/views/settings/StorageSettings.tsx @@ -36,17 +36,6 @@ function isFolderPickerDismissal(error: unknown): boolean { return error instanceof DOMException && error.name === "AbortError"; } -function isCurrentFolderTarget( - storagePath: string | null, - folderName: string, - projectKey: string -): boolean { - if (!storagePath) return false; - const normalized = storagePath.replace(/\\/g, "/"); - const expected = `${folderName}/.pm-suite/${projectKey}.pms.json`; - return normalized === expected || normalized.endsWith(`/${expected}`); -} - export function StorageSettings() { const bundle = useProjectStore((state) => state.bundle); const storagePath = useProjectStore((state) => state.storagePath); @@ -172,8 +161,19 @@ export function StorageSettings() { const projectAlreadyExists = folderFiles?.some( (filename) => filename.toLowerCase() === `${key}.pms.json`.toLowerCase() ); - const currentTarget = current.storageTrust === "folder" && isCurrentFolderTarget(current.storagePath, folderName, key); - if (projectAlreadyExists && !currentTarget) { + const currentTarget = Boolean( + projectAlreadyExists + && current.storageTrust === "folder" + && await adapter.isSelectedFolderSameAsPrevious?.() + ); + if (projectAlreadyExists && currentTarget) { + setFeedback({ + message: `This project is already saved in ${folderName}. No files were changed.`, + tone: "info" + }); + return; + } + if (projectAlreadyExists) { await restoreRejectedFolder(); setFeedback({ message: `That folder already contains ${key}.pms.json. Grillo did not overwrite it; open that project from the workspace launcher or choose a different folder.`,