From 6faafdb01e26d86a4b6c68eeda76199f330f32b9 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 12:28:55 -0500 Subject: [PATCH 01/12] Fix(vault): per-vault asset sort, board root and board statuses survive a settings save The main process rewrites vault.json through a normalizer that lists the view keys it keeps, and three keys added later were never put on that list: assetSortOrder, kanbanFolderRoot and kanbanStatuses. Any settings save from the desktop (a new favorite, a folder icon) dropped them from the file, so the renderer's per-vault choice silently never stuck. They are now carried through, kanbanStatuses reduced to its string entries. The comment on the normalizer says that every VaultViewSettings key must be listed, since the type cannot enforce it. Found while writing the settings merge for #816, which saves through the same path; the round-trip test gains a case for the three keys. Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 Co-authored-by: Amp --- apps/desktop/src/main/vault.test.ts | 22 ++++++++++++++++++++++ apps/desktop/src/main/vault.ts | 11 ++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/main/vault.test.ts b/apps/desktop/src/main/vault.test.ts index 84ca5e2b..a64e11a5 100644 --- a/apps/desktop/src/main/vault.test.ts +++ b/apps/desktop/src/main/vault.test.ts @@ -1317,6 +1317,28 @@ describe('per-vault view settings round-trip (#292)', () => { expect((saved.view as Record | undefined)?.bogus).toBeUndefined() }) + // The renderer persists these three per vault too (#292, #730), but the + // main normalizer used to drop them on every save, so the choice never + // stuck. They also feed the Cloud settings comparison (#816), where a key + // dropped on one side shows up as a difference that is not there. + it('carries asset sort, kanban statuses and the kanban folder root', async () => { + const root = await makeTempDir('zennotes-vault-view-kanban-') + await ensureVaultLayout(root) + const base = await getVaultSettings(root) + await setVaultSettings(root, { + ...base, + view: { + assetSortOrder: 'modified-desc', + kanbanStatuses: ['todo', 42, 'done'], + kanbanFolderRoot: 'Projects' + } + } as Awaited>) + const saved = await getVaultSettings(root) + expect(saved.view?.assetSortOrder).toBe('modified-desc') + expect(saved.view?.kanbanStatuses).toEqual(['todo', 'done']) + expect(saved.view?.kanbanFolderRoot).toBe('Projects') + }) + it('omits the view block when there are no overrides', async () => { const root = await makeTempDir('zennotes-vault-noview-') await ensureVaultLayout(root) diff --git a/apps/desktop/src/main/vault.ts b/apps/desktop/src/main/vault.ts index 2809d28f..9c3cb375 100644 --- a/apps/desktop/src/main/vault.ts +++ b/apps/desktop/src/main/vault.ts @@ -1182,15 +1182,24 @@ function normalizeTasksSettings(raw: unknown): VaultSettings['tasks'] | undefine /** Carry the per-vault view overrides (#292) through the round-trip, keeping * only known keys. The renderer validates the values strictly; here we just - * preserve a clean object (or undefined when there are no overrides). */ + * preserve a clean object (or undefined when there are no overrides). Every + * key of VaultViewSettings must be listed: a key missing here is dropped on + * every save, so the renderer's per-vault choice silently never sticks. */ function normalizeVaultViewSettings(raw: unknown): VaultViewSettings | undefined { if (!raw || typeof raw !== 'object') return undefined const c = raw as Record const view: VaultViewSettings = {} if (typeof c.noteSortOrder === 'string') view.noteSortOrder = c.noteSortOrder + if (typeof c.assetSortOrder === 'string') view.assetSortOrder = c.assetSortOrder if (typeof c.groupByKind === 'boolean') view.groupByKind = c.groupByKind if (typeof c.tasksViewMode === 'string') view.tasksViewMode = c.tasksViewMode if (typeof c.kanbanGroupBy === 'string') view.kanbanGroupBy = c.kanbanGroupBy + if (typeof c.kanbanFolderRoot === 'string') view.kanbanFolderRoot = c.kanbanFolderRoot + if (Array.isArray(c.kanbanStatuses)) { + view.kanbanStatuses = c.kanbanStatuses.filter( + (status): status is string => typeof status === 'string' + ) + } if (c.kanbanColumnTitles && typeof c.kanbanColumnTitles === 'object') { view.kanbanColumnTitles = c.kanbanColumnTitles as Record } From 92fc3b70345bc87941e815e909186c3793e80be0 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 12:29:21 -0500 Subject: [PATCH 02/12] Feat(cloud): the vault settings question is asked where you are, per setting (#816) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When this device and another one both changed a vault's settings since the last sync, sync parks the cloud's copy as .zennotes/vault.cloud-conflict.json and keeps this device's in use. Until now the only place that said so was a card in Settings > Cloud with two whole-file buttons, so the question sat there until the user happened to look, and answering it meant choosing between "my favorites" and "their folder icons" when they wanted both. @uNyanda hit exactly that (#816). The question now opens on its own right after the sync that found it. It lists each settings section that differs, this device's value beside the cloud's, down to the field that changed, and offers This device or Cloud per section, plus one button for all. Keys the cloud's file carries that this build has no setting for are listed under "Not used on this device" and never applied. Decide later applies nothing: the status bar reads "Settings need review" until answered, and Space r, the Review Cloud conflicts palette command or the status bar's Review brings the question back, the file queue first when both are pending. The card in Settings > Cloud stays, with a "Compare and choose…" button into the same dialog; its two whole-file answers behave as before. The pure core lives in shared-domain (vault-settings-conflict.ts): diffVaultSettings, mergeVaultSettings and unknownVaultSettingsKeys, with SECTIONS typed as Record so a new settings key fails to typecheck until it is added to the prompt or knowingly left out. The desktop's settingsConflict() now includes the parsed parked file as cloud_settings, optional in the bridge contract: a host without it keeps the whole-file question, and a parked file that does not parse is still reported. store.setVaultSettings resolves to a boolean so callers can tell a refused save from a completed one. The Cloud demo fixture's /demo/arm also bumps the cloud's vault.json so the flow can be reproduced locally. Verified in the built app over CDP against the fixture, both stores isolated: the prompt opened on its own, Decide later left both files untouched, Space r reopened it, and Cloud for two sections plus Apply produced exactly that mix in vault.json with the parked copy gone. Not exercised live: the web client and the mobile shells. Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 Co-authored-by: Amp --- .../src/main/cloud-sync-service.test.ts | 13 +- apps/desktop/src/main/cloud-sync-service.ts | 39 +- packages/app-core/src/App.tsx | 2 + .../src/components/CloudSettings.test.ts | 47 +++ .../app-core/src/components/CloudSettings.tsx | 78 ++-- .../CloudSettingsConflictDialog.test.ts | 171 +++++++++ .../CloudSettingsConflictDialog.tsx | 348 ++++++++++++++++++ .../components/CloudSettingsConflictHost.tsx | 49 +++ .../src/components/CommandPalette.tsx | 5 +- .../src/components/OnboardingWizard.tsx | 2 +- .../app-core/src/components/StatusBar.tsx | 13 +- packages/app-core/src/components/VimNav.tsx | 14 +- .../app-core/src/lib/cloud-auto-sync.test.ts | 150 ++++++++ packages/app-core/src/lib/cloud-auto-sync.ts | 190 +++++++++- packages/app-core/src/lib/commands.ts | 15 +- packages/app-core/src/lib/help.ts | 2 +- packages/app-core/src/lib/keymaps.ts | 2 +- .../src/lib/vault-settings-conflict.test.ts | 147 ++++++++ .../src/lib/vault-settings-conflict.ts | 191 ++++++++++ packages/app-core/src/store.ts | 14 +- packages/bridge-contract/src/cloud-sync.ts | 7 + .../src/vault-settings-conflict.test.ts | 178 +++++++++ .../src/vault-settings-conflict.ts | 184 +++++++++ .../scripts/cloud-conflict-demo-fixture.mjs | 49 +++ 24 files changed, 1843 insertions(+), 67 deletions(-) create mode 100644 packages/app-core/src/components/CloudSettingsConflictDialog.test.ts create mode 100644 packages/app-core/src/components/CloudSettingsConflictDialog.tsx create mode 100644 packages/app-core/src/components/CloudSettingsConflictHost.tsx create mode 100644 packages/app-core/src/lib/vault-settings-conflict.test.ts create mode 100644 packages/app-core/src/lib/vault-settings-conflict.ts create mode 100644 packages/shared-domain/src/vault-settings-conflict.test.ts create mode 100644 packages/shared-domain/src/vault-settings-conflict.ts diff --git a/apps/desktop/src/main/cloud-sync-service.test.ts b/apps/desktop/src/main/cloud-sync-service.test.ts index 0aee4f6b..8ab2faf7 100644 --- a/apps/desktop/src/main/cloud-sync-service.test.ts +++ b/apps/desktop/src/main/cloud-sync-service.test.ts @@ -257,9 +257,12 @@ describe('DesktopCloudSyncService', () => { expect(await service.settingsConflict(localRoot)).toBeNull() await writeFile(parkedPath, JSON.stringify({ favorites: ['cloud.md'] })) + // The cloud's copy travels with the question, so the app can show what + // differs and let the user answer one section at a time. expect(await service.settingsConflict(localRoot)).toEqual({ path: '.zennotes/vault.json', - cloud_path: '.zennotes/vault.cloud-conflict.json' + cloud_path: '.zennotes/vault.cloud-conflict.json', + cloud_settings: { favorites: ['cloud.md'] } }) // Keeping this device's settings drops the pending copy and changes nothing. @@ -283,8 +286,12 @@ describe('DesktopCloudSyncService', () => { await expect(service.resolveSettingsConflict(localRoot, 'cloud')).rejects.toThrow( 'could not be read' ) - // The question stays open rather than resolving itself badly. - expect(await service.settingsConflict(localRoot)).not.toBeNull() + // The question stays open rather than resolving itself badly, and is + // asked whole-file: there are no contents to compare. + expect(await service.settingsConflict(localRoot)).toEqual({ + path: '.zennotes/vault.json', + cloud_path: '.zennotes/vault.cloud-conflict.json' + }) }) it('links only a vault owned by the connected account', async () => { diff --git a/apps/desktop/src/main/cloud-sync-service.ts b/apps/desktop/src/main/cloud-sync-service.ts index eb9c9f6e..fc02a1b4 100644 --- a/apps/desktop/src/main/cloud-sync-service.ts +++ b/apps/desktop/src/main/cloud-sync-service.ts @@ -507,14 +507,21 @@ export class DesktopCloudSyncService { * it by accident. */ async settingsConflict(localRoot: string): Promise { const parked = path.join(localRoot, ...CLOUD_SYNC_SETTINGS_CONFLICT_PATH.split('/')) + let raw: string try { - await fs.access(parked) + raw = await fs.readFile(parked, 'utf8') } catch { return null } + // The parsed copy lets the app show what differs and offer a per-section + // answer. A copy that does not parse is still a pending question (the + // file is there, and sync will not touch vault.json until it is gone), so + // it is reported without the contents and the app asks whole-file. + const cloudSettings = parseParkedSettings(raw) return { path: CLOUD_SYNC_VAULT_SETTINGS_PATH, - cloud_path: CLOUD_SYNC_SETTINGS_CONFLICT_PATH + cloud_path: CLOUD_SYNC_SETTINGS_CONFLICT_PATH, + ...(cloudSettings ? { cloud_settings: cloudSettings } : {}) } } @@ -528,17 +535,14 @@ export class DesktopCloudSyncService { ): Promise { const parked = path.join(localRoot, ...CLOUD_SYNC_SETTINGS_CONFLICT_PATH.split('/')) if (choice === 'cloud') { - const raw = await fs.readFile(parked, 'utf8') - let parsed: unknown - try { - parsed = JSON.parse(raw) - } catch { - throw new Error('The settings from the cloud could not be read, so nothing was changed.') - } - if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + const parsed = parseParkedSettings(await fs.readFile(parked, 'utf8')) + if (!parsed) { throw new Error('The settings from the cloud could not be read, so nothing was changed.') } - await setVaultSettings(localRoot, parsed as Parameters[1]) + await setVaultSettings( + localRoot, + parsed as unknown as Parameters[1] + ) } await fs.rm(parked, { force: true }) } @@ -630,6 +634,19 @@ function rootFingerprint(localRoot: string): string { return fingerprint(path.resolve(localRoot)) } +/** The parked cloud vault.json as an object, or null when the bytes are not + * one (invalid JSON, a bare list, a hand-edited scalar). */ +function parseParkedSettings(raw: string): Record | null { + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + return null + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return null + return parsed as Record +} + function isCloudVaultLink(value: unknown): value is CloudVaultLink { if (!value || typeof value !== 'object') return false const link = value as Partial diff --git a/packages/app-core/src/App.tsx b/packages/app-core/src/App.tsx index 7edb77d3..d4385bce 100644 --- a/packages/app-core/src/App.tsx +++ b/packages/app-core/src/App.tsx @@ -24,6 +24,7 @@ import { ConfirmHost } from './components/ConfirmHost' import { DatePickerHost } from './components/DatePickerHost' import { PublishNoteHost } from './components/PublishNoteHost' import { CloudConflictReviewHost } from './components/CloudConflictReviewHost' +import { CloudSettingsConflictHost } from './components/CloudSettingsConflictHost' import { ServerDirectoryPickerHost } from './components/ServerDirectoryPickerHost' import { IconButton, ToastHost } from './components/ui' import { CloseIcon } from './components/icons' @@ -1260,6 +1261,7 @@ function App(): JSX.Element { + diff --git a/packages/app-core/src/components/CloudSettings.test.ts b/packages/app-core/src/components/CloudSettings.test.ts index e9192e30..fa5a7f12 100644 --- a/packages/app-core/src/components/CloudSettings.test.ts +++ b/packages/app-core/src/components/CloudSettings.test.ts @@ -1081,6 +1081,53 @@ describe("CloudSettings", () => { ); }); + // With the cloud's copy in hand the card says what differs and hands the + // per-setting choice to the shared prompt (#816). + it("names the settings that differ and opens the comparison prompt", async () => { + mocks.getCloudAccountStatus.mockResolvedValue(connected); + mocks.getCloudServiceAccount.mockResolvedValue(serviceAccount); + mocks.getCloudVaultLink.mockResolvedValue({ + base_url: "https://zennotes.org", + vault_id: "vault-1", + vault_name: "Cloud Notes", + linked_at: "2026-08-10T12:00:00.000Z", + }); + mocks.listCloudVaults.mockResolvedValue([]); + const local = useStore.getState().vaultSettings; + mocks.getCloudSettingsConflict.mockResolvedValue({ + path: ".zennotes/vault.json", + cloud_path: ".zennotes/vault.cloud-conflict.json", + cloud_settings: { + ...JSON.parse(JSON.stringify(local)), + favorites: [...local.favorites, "inbox:Reading"], + folderColors: { ...local.folderColors, "inbox:Reading": "amber" }, + experimentalSpellcheck: { enabled: true }, + }, + }); + + await act(async () => + root.render( + createElement(CloudSettings, { + localVaultAvailable: true, + localVaultName: "Notes", + }), + ), + ); + + expect(host.textContent).toContain("What differs: Folder colors, Favorites."); + expect(host.textContent).toContain("settings this device does not use (experimentalSpellcheck)"); + + // Sync surfaced the question and opened the prompt; the card reopens it + // after a "Decide later". + useCloudSyncStatusStore.setState({ settingsConflictPromptOpen: false }); + const compare = [...host.querySelectorAll("button")].find( + (button) => button.textContent?.trim() === "Compare and choose…", + ); + expect(compare).toBeTruthy(); + await act(async () => compare!.click()); + expect(useCloudSyncStatusStore.getState().settingsConflictPromptOpen).toBe(true); + }); + it("does not request vault data when sync is not included", async () => { mocks.getCloudAccountStatus.mockResolvedValue(connected); mocks.getCloudServiceAccount.mockResolvedValue({ diff --git a/packages/app-core/src/components/CloudSettings.tsx b/packages/app-core/src/components/CloudSettings.tsx index 8735772a..ed85aa4a 100644 --- a/packages/app-core/src/components/CloudSettings.tsx +++ b/packages/app-core/src/components/CloudSettings.tsx @@ -20,10 +20,17 @@ import { confirmApp } from "../lib/confirm-requests"; import { cloudSyncAttentionItems, cloudSyncAttentionMessage, + openCloudSettingsConflictPrompt, + refreshCloudSettingsConflict, + resolveCloudSettingsConflictWithStatus, useCloudSyncStatusStore, requestCloudAutoSync, syncCloudVaultWithStatus, } from "../lib/cloud-auto-sync"; +import { + describeVaultSettingsConflict, + VAULT_SETTINGS_SECTION_LABELS, +} from "../lib/vault-settings-conflict"; import { useToastStore } from "../lib/toast"; import { notifyPublishedNoteChanged } from "../lib/published-note-events"; import { requestPublishNote } from "../lib/publish-note-requests"; @@ -70,8 +77,9 @@ export function CloudSettings({ const [selectedVaultId, setSelectedVaultId] = useState(""); const [newVaultName, setNewVaultName] = useState(localVaultName); const [summary, setSummary] = useState(null); - const [settingsConflict, setSettingsConflict] = - useState(null); + // The settings question is the runtime's, not this panel's: sync raises it + // and the prompt answers it from anywhere, so this panel only shows it. + const settingsConflict = useCloudSyncStatusStore((state) => state.settingsConflict); const [backups, setBackups] = useState([]); const [backupSchedule, setBackupSchedule] = useState(null); @@ -105,7 +113,6 @@ export function CloudSettings({ setSelectedVaultId((selected) => remainingVaults.some((vault) => vault.id === selected) ? selected : (remainingVaults[0]?.id ?? "")); setSummary(null); - setSettingsConflict(null); setBackups([]); setBackupSchedule(null); setExpandedBackupId(null); @@ -383,24 +390,17 @@ export function CloudSettings({ }); }; - const loadSettingsConflict = useCallback(async (): Promise => { - try { - setSettingsConflict(await bridge.getCloudSettingsConflict()); - } catch { - // A host without the question (the web client) simply has none to ask. - setSettingsConflict(null); - } - }, [bridge]); - + // Opening Settings is a chance the runtime did not have: a question parked + // before this window existed (or while sync was off) shows up here too. useEffect(() => { - void loadSettingsConflict(); - }, [loadSettingsConflict]); + void refreshCloudSettingsConflict(bridge); + }, [bridge]); const syncVault = (): Promise => { setSummary(null); return runAction("sync", async () => { setSummary(await syncCloudVaultWithStatus(bridge, link?.vault_name)); - await loadSettingsConflict(); + await refreshCloudSettingsConflict(bridge); await refreshServiceAccount(); }); }; @@ -408,12 +408,8 @@ export function CloudSettings({ const resolveSettingsConflict = ( choice: CloudSyncSettingsChoice, ): Promise => - runAction( - choice === "cloud" ? "settings-cloud" : "settings-local", - async () => { - await bridge.resolveCloudSettingsConflict(choice); - await loadSettingsConflict(); - }, + runAction(choice === "cloud" ? "settings-cloud" : "settings-local", () => + resolveCloudSettingsConflictWithStatus(choice, bridge), ); const createBackup = (): Promise => @@ -1215,7 +1211,9 @@ function CloudVaultPanel({ {settingsConflict && ( )} {syncing && ( @@ -1924,15 +1922,28 @@ function numericLimit( * conflict copy to compare side by side, but settings are a single answer, and * a copy of them inside a hidden folder is not something anyone can act on. * This device's settings stay in use until the question is answered, so doing - * nothing keeps what is already working. + * nothing keeps what is already working. The card names the sections that + * differ and hands the per-section choice to the shared prompt; the two + * whole-file answers stay here for the common "just keep mine" case. */ function CloudSettingsConflictCard({ action, + conflict, onResolve, + onCompare, }: { action: CloudAction; + conflict: CloudSyncSettingsConflict; onResolve: (choice: CloudSyncSettingsChoice) => void; + onCompare: () => void; }): JSX.Element { + const localSettings = useStore((state) => state.vaultSettings); + const described = conflict.cloud_settings + ? describeVaultSettingsConflict(localSettings, conflict.cloud_settings) + : null; + const sections = described?.differences.map( + (difference) => VAULT_SETTINGS_SECTION_LABELS[difference.section], + ); return (
Vault settings differ from the cloud
- Another device saved different settings for this vault: favorites, - folder icons and colors, and where the built-in folders live. This - device’s settings are the ones in use. + {sections === undefined + ? "Another device saved different settings for this vault: favorites, folder icons and colors, and where the built-in folders live." + : sections.length === 0 + ? "Another device saved settings for this vault that this device reads the same way as its own." + : `Another device saved different settings for this vault. What differs: ${sections.join(", ")}.`}{" "} + This device’s settings are the ones in use. + {described !== null && described.unknownKeys.length > 0 && ( + <> + {" "} + The cloud’s copy also carries settings this device does not use ( + {described.unknownKeys.join(", ")}). + + )}
+ {sections !== undefined && sections.length > 0 && ( + + )} + +
+
+
+ {differences.map((difference) => ( + + setChoices((current) => ({ ...current, [difference.section]: side })) + } + /> + ))} +
+ + )} + {described !== null && described.unknownKeys.length > 0 && ( +
0 ? "mt-3" : ""}`} + > +
Not used on this device
+
{described.unknownKeys.join(", ")}
+
+ The cloud’s file also carries these settings from another app or a + newer version. This device has no such settings, so it cannot compare or + choose them. +
+
+ )} + {error && ( +
+ {error} +
+ )} + + +
+ This device’s settings stay in use until you decide. +
+ + {described === null && ( + // Without a readable copy there is nothing to pick from, so the + // whole-file alternative gets its own button. + + )} + +
+ + ); +} + +function describeQuestion( + vaultName: string, + described: ReturnType | null, + differences: VaultSettingsSectionDifference[], +): string { + if (described === null) { + return `Another device saved different settings for ${vaultName}, and the cloud's copy could not be read on this device, so the two cannot be compared here. Keep this device's settings or take the cloud's as a whole.`; + } + if (differences.length === 0) { + return `Another device saved settings for ${vaultName} that this device reads the same way as its own. Keep this device's settings to finish syncing.`; + } + const count = differences.length; + return `Another device saved different settings for ${vaultName}. ${count === 1 ? "One setting differs" : `${count} settings differ`}; both versions are shown so you can keep this device's or take the cloud's for each.`; +} + +function SectionRow({ + difference, + choice, + disabled, + onChoose, +}: { + difference: VaultSettingsSectionDifference; + choice: VaultSettingsSide; + disabled: boolean; + onChoose: (side: VaultSettingsSide) => void; +}): JSX.Element { + const label = VAULT_SETTINGS_SECTION_LABELS[difference.section]; + const shown = difference.fields.slice(0, FIELD_ROW_LIMIT); + const hidden = difference.fields.length - shown.length; + return ( +
+
+
{label}
+
+ onChoose("local")} + > + This device + + onChoose("cloud")} + > + Cloud + +
+
+ + + + + + + + + + {shown.map((field) => ( + + + + + + ))} + +
+ Setting + + This device + + Cloud +
+ {vaultSettingsFieldLabel(difference.section, field.path)} + + {formatVaultSettingsValue(difference.section, field.path, field.local)} + + {formatVaultSettingsValue(difference.section, field.path, field.cloud)} +
+ {hidden > 0 && ( +
+ and {hidden} more {hidden === 1 ? "difference" : "differences"} in this section +
+ )} +
+ ); +} + +function ChoiceButton({ + active, + disabled, + onClick, + children, +}: { + active: boolean; + disabled: boolean; + onClick: () => void; + children: React.ReactNode; +}): JSX.Element { + return ( + + ); +} diff --git a/packages/app-core/src/components/CloudSettingsConflictHost.tsx b/packages/app-core/src/components/CloudSettingsConflictHost.tsx new file mode 100644 index 00000000..7b542c91 --- /dev/null +++ b/packages/app-core/src/components/CloudSettingsConflictHost.tsx @@ -0,0 +1,49 @@ +import type { VaultSettings } from "@shared/ipc"; +import { + closeCloudSettingsConflictPrompt, + resolveCloudSettingsConflictWithStatus, + useCloudSyncStatusStore, +} from "../lib/cloud-auto-sync"; +import { useStore } from "../store"; +import { CloudSettingsConflictDialog } from "./CloudSettingsConflictDialog"; + +/** + * Mounts the vault settings prompt for the whole app, beside the file + * conflict queue and for the same reason: sync raises the question from the + * runtime, and the status bar, palette and leader binding reopen it, so it + * cannot live inside Settings. The file queue takes the screen first when + * both are pending; the settings prompt waits for it to close. + */ +export function CloudSettingsConflictHost(): JSX.Element | null { + const conflict = useCloudSyncStatusStore((state) => state.settingsConflict); + const open = useCloudSyncStatusStore((state) => state.settingsConflictPromptOpen); + const fileReviewOpen = useCloudSyncStatusStore((state) => state.conflictReviewOpen); + const vaultName = useCloudSyncStatusStore((state) => state.vaultName); + const localSettings = useStore((state) => state.vaultSettings); + if (conflict === null || !open || fileReviewOpen) return null; + return ( + resolveCloudSettingsConflictWithStatus(choice)} + onApply={applyMergedVaultSettings} + /> + ); +} + +/** + * A per-section answer is this device's settings with some sections taken + * from the cloud. Saving them through the store gives them the same + * validation and pattern history as an edit in Settings; only once that save + * is on disk is the parked copy retired, as "keep this device's" (the merged + * file is now this device's, and the next run uploads it). + */ +async function applyMergedVaultSettings(settings: VaultSettings): Promise { + const saved = await useStore.getState().setVaultSettings(settings); + if (!saved) { + throw new Error("The settings could not be saved. The cloud's copy is still waiting."); + } + await resolveCloudSettingsConflictWithStatus("local"); +} diff --git a/packages/app-core/src/components/CommandPalette.tsx b/packages/app-core/src/components/CommandPalette.tsx index 60c8ed91..333a6c92 100644 --- a/packages/app-core/src/components/CommandPalette.tsx +++ b/packages/app-core/src/components/CommandPalette.tsx @@ -310,12 +310,13 @@ export function CommandPalette(): JSX.Element { // closePalette's focus restore; the retry wins that race. Skipped when the // command opened Settings or another palette (search, vault text search, // outline, …) so we don't pull focus behind it, and likewise for the Cloud - // conflict queue and the Publish Note dialog, which claim focus themselves - // and are tracked outside the store. + // conflict queue, the Cloud settings prompt and the Publish Note dialog, + // which claim focus themselves and are tracked outside the store. if ( shouldRefocusEditorAfterCommand( useStore.getState(), useCloudSyncStatusStore.getState().conflictReviewOpen || + useCloudSyncStatusStore.getState().settingsConflictPromptOpen || getPublishNoteRequest() !== null ) ) diff --git a/packages/app-core/src/components/OnboardingWizard.tsx b/packages/app-core/src/components/OnboardingWizard.tsx index 97ee438b..41d779c7 100644 --- a/packages/app-core/src/components/OnboardingWizard.tsx +++ b/packages/app-core/src/components/OnboardingWizard.tsx @@ -782,7 +782,7 @@ function LayoutStep({ primaryLocation: PrimaryNotesLocation dailyEnabled: boolean dailyDirectory: string - setVaultSettings: (next: VaultSettings) => Promise + setVaultSettings: (next: VaultSettings) => Promise hasVault: boolean onBack: () => void onNext: () => void diff --git a/packages/app-core/src/components/StatusBar.tsx b/packages/app-core/src/components/StatusBar.tsx index da776872..7f94dcc8 100644 --- a/packages/app-core/src/components/StatusBar.tsx +++ b/packages/app-core/src/components/StatusBar.tsx @@ -5,9 +5,11 @@ import { backlinksForNote } from "../lib/wikilinks"; import { countWords } from "../lib/word-count"; import { useHoveredLinkStore } from "../lib/hovered-link"; import { + cloudSyncAttentionIsSettingsOnly, connectCloudAccountFromStatusBar, formatRelativeSyncTime, openCloudConflictReview, + openCloudSettingsConflictPrompt, resolvableCloudConflictCount, syncCloudVaultWithStatus, type CloudSyncPhase, @@ -100,6 +102,7 @@ function CloudSyncStatus({ const lastSyncedAt = useCloudSyncStatusStore((state) => state.lastSyncedAt); const error = useCloudSyncStatusStore((state) => state.error); const lastSummary = useCloudSyncStatusStore((state) => state.lastSummary); + const settingsOnly = useCloudSyncStatusStore(cloudSyncAttentionIsSettingsOnly); const setSettingsOpen = useStore((state) => state.setSettingsOpen); const [now, setNow] = useState(() => Date.now()); const resolvableConflictCount = resolvableCloudConflictCount(lastSummary); @@ -127,7 +130,9 @@ function CloudSyncStatus({ : phase === "attention" ? hasResolvableConflict ? `${resolvableConflictCount} ${resolvableConflictCount === 1 ? "file needs" : "files need"} review` - : "Sync incomplete" + : settingsOnly + ? "Settings need review" + : "Sync incomplete" : phase === "error" ? "Sync failed" : lastSyncedAt === null @@ -195,6 +200,12 @@ function CloudSyncStatus({ openCloudConflictReview(); return; } + if (settingsOnly) { + // The prompt is where the settings decision is made; Settings only + // repeats the question. + openCloudSettingsConflictPrompt(); + return; + } if (phase === "attention") { openCloudSettings(); return; diff --git a/packages/app-core/src/components/VimNav.tsx b/packages/app-core/src/components/VimNav.tsx index 0c397e3b..ba70b078 100644 --- a/packages/app-core/src/components/VimNav.tsx +++ b/packages/app-core/src/components/VimNav.tsx @@ -48,8 +48,8 @@ import { getBufferNavigationTarget } from '../lib/buffer-navigation' import { focusEditorNormalMode } from '../lib/editor-focus' import { atlasHoldsKeyboard } from '../lib/atlas' import { - hasResolvableCloudConflicts, - openCloudConflictReview, + hasPendingCloudReview, + openPendingCloudReview, resolvableCloudConflictCount, useCloudSyncStatusStore } from '../lib/cloud-auto-sync' @@ -208,7 +208,7 @@ export function VimNav(): JSX.Element | null { isCalendarToggleAvailable(s.vaultSettings, s.activeNote) ) const cloudConflictsWaiting = useCloudSyncStatusStore( - (s) => resolvableCloudConflictCount(s.lastSummary) > 0 + (s) => resolvableCloudConflictCount(s.lastSummary) > 0 || s.settingsConflict !== null ) const whichKeyHintsPref = useStore((s) => s.whichKeyHints) const whichKeyHintMode = useStore((s) => s.whichKeyHintMode) @@ -285,7 +285,7 @@ export function VimNav(): JSX.Element | null { { keyLabel: getKeymapDisplay(keymapOverrides, 'vim.leaderCloudConflicts'), label: 'Review Cloud conflicts', - detail: 'Open the queue of files waiting on a sync decision.' + detail: 'Open the files waiting on a sync decision, or the vault settings question.' } ] : []), @@ -939,16 +939,16 @@ export function VimNav(): JSX.Element | null { void state.openWorkflowsView() return } - // Skipped with an empty queue so the key falls through as an unbound + // Skipped while nothing waits so the key falls through as an unbound // leader press rather than opening an empty dialog. if ( - hasResolvableCloudConflicts() && + hasPendingCloudReview() && matchesSequenceToken(e, overrides, 'vim.leaderCloudConflicts') ) { e.preventDefault() e.stopImmediatePropagation() resetLeader() - openCloudConflictReview() + openPendingCloudReview() return } if (matchesSequenceToken(e, overrides, 'vim.hintMode')) { diff --git a/packages/app-core/src/lib/cloud-auto-sync.test.ts b/packages/app-core/src/lib/cloud-auto-sync.test.ts index c05d1126..ddc831e0 100644 --- a/packages/app-core/src/lib/cloud-auto-sync.test.ts +++ b/packages/app-core/src/lib/cloud-auto-sync.test.ts @@ -8,12 +8,17 @@ import type { VaultChangeEvent } from "@shared/ipc"; import { acknowledgeCloudConflictResolution, clearCloudSyncStatus, + cloudSyncAttentionIsSettingsOnly, cloudSyncAttentionItems, cloudSyncAttentionMessage, closeCloudConflictReview, + closeCloudSettingsConflictPrompt, connectCloudAccountFromStatusBar, + hasPendingCloudReview, openCloudConflictReview, + openPendingCloudReview, registerCloudConflictDraftFlusher, + resolveCloudSettingsConflictWithStatus, startCloudAutoSync, syncCloudVaultWithStatus, type CloudAutoSyncBridge, @@ -820,3 +825,148 @@ describe("cloudSyncAttentionItems (Discord: name the file, not the count)", () = closeCloudConflictReview(); }); }); + +describe("the vault settings question (#816)", () => { + const base: CloudSyncRunSummary = { + cursor: 9, + pulled: 0, + pushed: 0, + conflicts: [], + bootstrap_conflicts: [], + local_conflicts: [], + }; + const question = { + path: ".zennotes/vault.json", + cloud_path: ".zennotes/vault.cloud-conflict.json", + cloud_settings: { favorites: ["inbox:Projects"] }, + }; + + function bridgeWith(parked: () => typeof question | null) { + return { + syncCloudVault: async () => base, + getCloudSettingsConflict: async () => parked(), + resolveCloudSettingsConflict: vi.fn(async () => undefined), + }; + } + + beforeEach(() => clearCloudSyncStatus()); + afterEach(() => clearCloudSyncStatus()); + + it("surfaces the question right after the run that parked it, and opens the prompt once", async () => { + let parked: typeof question | null = question; + const bridge = bridgeWith(() => parked); + + await syncCloudVaultWithStatus(bridge, "Notes"); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + phase: "attention", + settingsConflict: question, + settingsConflictPromptOpen: true, + }); + expect(cloudSyncAttentionIsSettingsOnly()).toBe(true); + expect(hasPendingCloudReview()).toBe(true); + + // "Decide later" applies nothing: the question stays, the prompt closes, + // and the next run with the same parked copy does not reopen it. + closeCloudSettingsConflictPrompt(); + await syncCloudVaultWithStatus(bridge, "Notes"); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + phase: "attention", + settingsConflict: question, + settingsConflictPromptOpen: false, + }); + + // The status bar and the leader binding reopen the same prompt. + openPendingCloudReview(); + expect(useCloudSyncStatusStore.getState().settingsConflictPromptOpen).toBe(true); + closeCloudSettingsConflictPrompt(); + + // A newer cloud copy is a new question, so it is asked again. + parked = { ...question, cloud_settings: { favorites: ["inbox:Reading"] } }; + await syncCloudVaultWithStatus(bridge, "Notes"); + expect(useCloudSyncStatusStore.getState().settingsConflictPromptOpen).toBe(true); + }); + + it("clears the question and the attention once it is answered", async () => { + let parked: typeof question | null = question; + const bridge = bridgeWith(() => parked); + await syncCloudVaultWithStatus(bridge, "Notes"); + + parked = null; + await resolveCloudSettingsConflictWithStatus("cloud", bridge); + expect(bridge.resolveCloudSettingsConflict).toHaveBeenCalledWith("cloud"); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + phase: "ready", + error: null, + settingsConflict: null, + settingsConflictPromptOpen: false, + }); + expect(hasPendingCloudReview()).toBe(false); + // Nothing to open: the guard keeps an empty prompt off the screen. + openPendingCloudReview(); + expect(useCloudSyncStatusStore.getState().settingsConflictPromptOpen).toBe(false); + }); + + it("keeps the generic wording when files also need attention, and lets the file queue go first", async () => { + const pending = { + id: "item-1", + item_id: "item-1", + path: "Plans/Trip.md", + cloud_path: "Plans/Trip.md", + kind: "content" as const, + can_merge: true, + has_base: true, + }; + await syncCloudVaultWithStatus( + { + syncCloudVault: async () => ({ ...base, pending_conflicts: [pending] }), + getCloudSettingsConflict: async () => question, + }, + "Notes", + ); + const state = useCloudSyncStatusStore.getState(); + expect(state.phase).toBe("attention"); + expect(state.error).toContain("1 file differs"); + expect(cloudSyncAttentionIsSettingsOnly()).toBe(false); + expect(state.settingsConflict).toEqual(question); + + openPendingCloudReview(); + expect(useCloudSyncStatusStore.getState().conflictReviewOpen).toBe(true); + }); + + it("is a question of the host: a bridge without one, or one answering nothing, asks nothing", async () => { + await syncCloudVaultWithStatus({ syncCloudVault: async () => base }, "Notes"); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + phase: "ready", + settingsConflict: null, + settingsConflictPromptOpen: false, + }); + await syncCloudVaultWithStatus( + { + syncCloudVault: async () => base, + getCloudSettingsConflict: (async () => undefined) as unknown as () => Promise, + }, + "Notes", + ); + expect(useCloudSyncStatusStore.getState().settingsConflict).toBeNull(); + }); + + it("surfaces a question left by an earlier run even when this run fails", async () => { + await expect( + syncCloudVaultWithStatus( + { + syncCloudVault: async () => { + throw new Error("Offline"); + }, + getCloudSettingsConflict: async () => question, + }, + "Notes", + ), + ).rejects.toThrow("Offline"); + await flushPromises(); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + phase: "error", + settingsConflict: question, + settingsConflictPromptOpen: true, + }); + }); +}); diff --git a/packages/app-core/src/lib/cloud-auto-sync.ts b/packages/app-core/src/lib/cloud-auto-sync.ts index 5f06c626..8163b44a 100644 --- a/packages/app-core/src/lib/cloud-auto-sync.ts +++ b/packages/app-core/src/lib/cloud-auto-sync.ts @@ -1,7 +1,11 @@ import { humanIpcError } from "./ipc-error"; import type { ZenBridge } from "@zennotes/bridge-contract/bridge"; import { getZenBridge } from "@zennotes/bridge-contract/bridge"; -import type { CloudSyncRunSummary } from "@zennotes/bridge-contract/cloud-sync"; +import type { + CloudSyncRunSummary, + CloudSyncSettingsChoice, + CloudSyncSettingsConflict, +} from "@zennotes/bridge-contract/cloud-sync"; import type { VaultChangeEvent } from "@shared/ipc"; import { create } from "zustand"; import { @@ -10,6 +14,13 @@ import { type CloudAutoSyncReason, } from "@zennotes/shared-domain/cloud-auto-sync"; import { shouldSyncVaultPath } from "@zennotes/shared-domain/cloud-sync"; +import { vaultSettingsValueEqual } from "@zennotes/shared-domain/vault-settings-conflict"; + +/** A host without the settings question (the web client's bridge answers it + * with null; a test bridge may leave it out) simply never asks it. */ +type CloudSettingsConflictBridge = Partial< + Pick +>; export type CloudAutoSyncBridge = Pick< ZenBridge, @@ -22,7 +33,8 @@ export type CloudAutoSyncBridge = Pick< | "onCloudSyncWindow" | "onVaultChange" | "onCloudAccountChange" ->; +> & + CloudSettingsConflictBridge; export interface CloudAutoSyncEnvironment { online(): boolean; @@ -62,6 +74,16 @@ interface CloudSyncStatusStore { syncWindowLocked: boolean; /** A note was saved, but the following whole-vault sync has not completed. */ resolutionSaved: boolean; + /** The vault settings question, while one is pending: sync parked the + * cloud's vault.json beside this device's and waits for an answer. It is + * read from the parked copy after every run, not from the run summary, + * because only the run that parked it reports it and the question stays + * open long after that summary is gone. */ + settingsConflict: CloudSyncSettingsConflict | null; + /** Whether the settings prompt is on screen. Lives here for the same reason + * as conflictReviewOpen: the status bar, the palette, the leader binding + * and the sync runtime itself all open the one prompt. */ + settingsConflictPromptOpen: boolean; } const emptyCloudSyncStatus: CloudSyncStatusStore = { @@ -73,8 +95,13 @@ const emptyCloudSyncStatus: CloudSyncStatusStore = { conflictReviewOpen: false, syncWindowLocked: false, resolutionSaved: false, + settingsConflict: null, + settingsConflictPromptOpen: false, }; +const SETTINGS_ATTENTION_MESSAGE = + "Vault settings differ on this device and in Cloud. Choose which settings to use."; + export const useCloudSyncStatusStore = create(() => ({ ...emptyCloudSyncStatus, })); @@ -188,6 +215,9 @@ export function startCloudAutoSync( void refreshRemovedCloudLink(bridge, error); } useCloudSyncStatusStore.setState({ syncWindowLocked: false }); + // The other window's run may have parked, replaced or (after an answer + // there) removed the settings question; this window's status follows. + void refreshCloudSettingsConflict(bridge); }, }); const unsubscribeAccount = bridge.onCloudAccountChange((status) => { @@ -238,7 +268,8 @@ export async function connectCloudAccountFromStatusBar( } export async function syncCloudVaultWithStatus( - bridge: Pick & Partial> = getZenBridge(), + bridge: Pick & + Partial> = getZenBridge(), vaultName?: string | null, ): Promise { const current = useCloudSyncStatusStore.getState(); @@ -254,6 +285,10 @@ export async function syncCloudVaultWithStatus( await Promise.all([...conflictDraftFlushers].map((flush) => flush())); } const summary = await bridge.syncCloudVault(); + // Read the settings question before the run's status is drawn from the + // summary, so a still-open question is part of that status rather than a + // correction to it a moment later. + await refreshCloudSettingsConflict(bridge); applyCloudSyncSummary(summary, nextVaultName); return summary; } catch (error) { @@ -264,10 +299,142 @@ export async function syncCloudVaultWithStatus( error: syncFailureMessage(error), }); } + // The parked copy is local, so a failed run can still surface a question + // an earlier run left behind (the first run after a restart, offline). + void refreshCloudSettingsConflict(bridge); throw error; } } +/** + * Re-read the pending vault settings question from the host and fold it into + * the status. Safe with a bridge that cannot answer it (the web client, a + * remote vault): such a host has no question to ask. + */ +export async function refreshCloudSettingsConflict( + bridge: CloudSettingsConflictBridge = getZenBridge(), +): Promise { + if (!bridge.getCloudSettingsConflict) return; + let next: CloudSyncSettingsConflict | null; + try { + next = (await bridge.getCloudSettingsConflict()) ?? null; + } catch { + next = null; + } + applyCloudSettingsConflict(next); +} + +function applyCloudSettingsConflict(next: CloudSyncSettingsConflict | null): void { + const current = useCloudSyncStatusStore.getState(); + const previous = current.settingsConflict; + if (next === null) { + if (previous === null) return; + // Answered, here or in another window. A status that only spoke of the + // question goes back to what the last run reported; the summary of the + // run that parked it still lists it, and that entry is now stale. + const settledAttention = + current.phase === "attention" && current.error === SETTINGS_ATTENTION_MESSAGE + ? attentionMessageWithoutSettings(current.lastSummary) + : undefined; + useCloudSyncStatusStore.setState({ + settingsConflict: null, + settingsConflictPromptOpen: false, + ...(settledAttention === undefined + ? {} + : { + phase: settledAttention === null ? "ready" : "attention", + error: settledAttention, + }), + }); + return; + } + // The prompt opens itself for a new question, and again when the cloud's + // copy changed underneath a postponed one (sync replaces the parked copy + // with the newest cloud version). It does not reopen the same postponed + // question on every run: "Decide later" means that. + const newQuestion = + previous === null || + !vaultSettingsValueEqual(previous.cloud_settings, next.cloud_settings); + useCloudSyncStatusStore.setState({ + settingsConflict: next, + settingsConflictPromptOpen: current.settingsConflictPromptOpen || newQuestion, + ...(current.phase === "ready" + ? { phase: "attention", error: SETTINGS_ATTENTION_MESSAGE } + : {}), + }); +} + +function attentionMessageWithoutSettings( + summary: CloudSyncRunSummary | null, +): string | null { + if (summary === null) return null; + const attention = cloudSyncAttentionMessage(summary); + return attention === SETTINGS_ATTENTION_MESSAGE ? null : attention; +} + +/** True while sync waits for an answer about the vault settings. */ +export function hasPendingCloudSettingsConflict(): boolean { + return useCloudSyncStatusStore.getState().settingsConflict !== null; +} + +/** + * True when the settings question is the only thing keeping the status at + * attention, so a status surface can say "settings" instead of the generic + * "incomplete" and open the prompt directly. Capacity trouble or rejected + * changes alongside it keep the generic wording: those are read in Settings. + */ +export function cloudSyncAttentionIsSettingsOnly( + state: Pick = + useCloudSyncStatusStore.getState(), +): boolean { + return ( + state.phase === "attention" && + state.settingsConflict !== null && + state.error === SETTINGS_ATTENTION_MESSAGE + ); +} + +export function openCloudSettingsConflictPrompt(): void { + if (!hasPendingCloudSettingsConflict()) return; + useCloudSyncStatusStore.setState({ settingsConflictPromptOpen: true }); +} + +/** Postpones the question. Nothing is applied: this device's settings stay in + * use and the cloud's copy stays parked until the prompt is answered. */ +export function closeCloudSettingsConflictPrompt(): void { + useCloudSyncStatusStore.setState({ settingsConflictPromptOpen: false }); +} + +/** + * Answer the question on the host and sync the answer. Keeping this device's + * settings only drops the parked copy, which is not itself a synced file, so + * the run that pushes the local settings up has to be asked for here. + */ +export async function resolveCloudSettingsConflictWithStatus( + choice: CloudSyncSettingsChoice, + bridge: Pick & + CloudSettingsConflictBridge = getZenBridge(), +): Promise { + await bridge.resolveCloudSettingsConflict(choice); + await refreshCloudSettingsConflict(bridge); + requestCloudAutoSync("local-change"); +} + +/** + * Whatever Cloud is waiting on the user for, most urgent first: the file + * queue outranks the settings question, because its files cannot sync at all + * until answered. One entry point so the status bar, the palette entry and + * the leader binding agree on what "review" opens. + */ +export function hasPendingCloudReview(): boolean { + return hasResolvableCloudConflicts() || hasPendingCloudSettingsConflict(); +} + +export function openPendingCloudReview(): void { + if (hasResolvableCloudConflicts()) openCloudConflictReview(); + else openCloudSettingsConflictPrompt(); +} + async function refreshRemovedCloudLink( bridge: Partial>, error: unknown, @@ -309,7 +476,11 @@ export function acknowledgeCloudConflictResolution( function applyCloudSyncSummary(summary: CloudSyncRunSummary, vaultName?: string | null): void { const current = useCloudSyncStatusStore.getState(); - const attention = cloudSyncAttentionMessage(summary); + // A run that reports nothing new has still not synced the vault settings + // while the question from an earlier run is open. + const attention = + cloudSyncAttentionMessage(summary) ?? + (current.settingsConflict !== null ? SETTINGS_ATTENTION_MESSAGE : null); useCloudSyncStatusStore.setState({ phase: attention === null ? "ready" : "attention", vaultName: vaultName ?? current.vaultName, @@ -391,6 +562,8 @@ function markCloudSyncDisconnected(error: string | null = null): void { lastSyncedAt: null, resolutionSaved: false, error, + settingsConflict: null, + settingsConflictPromptOpen: false, }); } @@ -413,6 +586,10 @@ function markCloudSyncUnlinked(error: string | null = null): void { error, lastSummary: null, conflictReviewOpen: false, + // Unlinking leaves the parked copy on disk, but there is no cloud to + // answer to; linking again asks afresh. + settingsConflict: null, + settingsConflictPromptOpen: false, }); } @@ -504,7 +681,7 @@ export function cloudSyncAttentionMessage( if (summary.local_conflicts.length > 0) { const count = summary.local_conflicts.length; if (summary.local_conflicts.every((conflict) => conflict.code === "SETTINGS_CONFLICT")) { - return "Vault settings differ on this device and in Cloud. Choose which settings to use."; + return SETTINGS_ATTENTION_MESSAGE; } return `Cloud sync kept both versions of ${count} changed ${count === 1 ? "file" : "files"}. Review the conflict copies.`; } @@ -596,7 +773,8 @@ export function cloudSyncAttentionItems( items.push({ kind: "settings", path: conflict.path, - detail: "Vault settings differ from the cloud. Choose which to keep above.", + detail: + "Vault settings differ from the cloud. Compare them and choose which to keep from the card above.", conflictCopyPath: null, }); } else if (conflict.conflict_copy_path) { diff --git a/packages/app-core/src/lib/commands.ts b/packages/app-core/src/lib/commands.ts index fc3e2389..c457ff69 100644 --- a/packages/app-core/src/lib/commands.ts +++ b/packages/app-core/src/lib/commands.ts @@ -38,8 +38,8 @@ import { isCalendarToggleAvailable, noteFolderSubpath } from './vault-layout' import { runWorkflowById } from './workflow-trigger' import { requestPublishNote } from './publish-note-requests' import { - hasResolvableCloudConflicts, - openCloudConflictReview + hasPendingCloudReview, + openPendingCloudReview } from './cloud-auto-sync' import { DEMO_TOUR_START_PATH } from '@shared/demo-tour' @@ -1813,12 +1813,13 @@ export function buildCommands(options?: { includeUnavailable?: boolean }): Comma id: 'app.cloud.reviewConflicts', title: 'Review Cloud Sync Conflicts', category: 'Vault', - keywords: 'cloud sync conflict merge review resolve queue two devices differ', + keywords: 'cloud sync conflict merge review resolve queue two devices differ settings', shortcut: leaderShortcut('vim.leaderCloudConflicts'), - // Hidden with an empty queue: the same dialog the status bar's Review - // now opens, and there is nothing to review without it. - when: () => hasResolvableCloudConflicts(), - run: () => openCloudConflictReview() + // Hidden while nothing waits: the same dialogs the status bar's Review + // opens (the file queue first, then the vault settings question), and + // there is nothing to review without one of them. + when: () => hasPendingCloudReview(), + run: () => openPendingCloudReview() }, { id: 'app.vault.switch', diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index a727f9d9..d252aa67 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -546,7 +546,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ { keys: 'Space p', action: 'Note outline', detail: 'Jump to any heading in the active note via a searchable overlay.' }, { keys: 'Space v', action: 'Switch vault', detail: 'Open the command palette directly to the local vault switcher.' }, { keys: 'Space a', action: 'Open workflows', detail: 'Open the Workflows view, where saved pipelines over your notes are built and run. Workflows are off by default; turn them on under Settings → Workflows first.' }, - { keys: 'Space r', action: 'Review Cloud conflicts', detail: 'Open the Cloud sync conflict queue: the files two devices changed at once, one decision at a time. The binding and the command palette entry appear only while files are waiting, and open the same queue as Review now in the status bar.' }, + { keys: 'Space r', action: 'Review Cloud conflicts', detail: 'Open whatever Cloud sync is waiting on: the queue of files two devices changed at once, one decision at a time, or the vault settings question, which lists each setting that differs with this device’s value beside the cloud’s and lets you pick a side per setting. The binding and the command palette entry appear only while something is waiting, and open the same dialog as Review in the status bar.' }, { keys: 'Space g', action: 'Open atlas', detail: 'Open the Atlas view: the whole vault drawn as a map of notes and links.' }, { keys: 'Space q', action: 'Quick capture window', detail: 'Open the floating, always-on-top capture window, same as the global hotkey.' }, { keys: 'Space i', action: 'Insert template into note', detail: 'Pick a template and insert it at the cursor of the active note, instead of creating a new note from it.' }, diff --git a/packages/app-core/src/lib/keymaps.ts b/packages/app-core/src/lib/keymaps.ts index d8701276..f281368a 100644 --- a/packages/app-core/src/lib/keymaps.ts +++ b/packages/app-core/src/lib/keymaps.ts @@ -526,7 +526,7 @@ const KEYMAP_DEFINITIONS: KeymapDefinition[] = [ title: "Leader: review Cloud conflicts", // `c` is the calendar and `s` the search group, so review takes `r`. description: - "Open the Cloud sync conflict queue. Available while files are waiting on a decision.", + "Open the Cloud sync conflict queue, or the vault settings question. Available while files or settings are waiting on a decision.", defaultBinding: "r", vimOnly: true, maxTokens: 1, diff --git a/packages/app-core/src/lib/vault-settings-conflict.test.ts b/packages/app-core/src/lib/vault-settings-conflict.test.ts new file mode 100644 index 00000000..5270bce4 --- /dev/null +++ b/packages/app-core/src/lib/vault-settings-conflict.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, it } from "vitest"; +import { DEFAULT_VAULT_SETTINGS, type VaultSettings } from "@shared/ipc"; +import { + describeVaultSettingsConflict, + formatVaultSettingsValue, + vaultSettingsFieldLabel, +} from "./vault-settings-conflict"; +import { normalizeVaultSettings } from "./vault-layout"; + +function localSettings(overrides: Partial = {}): VaultSettings { + return normalizeVaultSettings({ + ...DEFAULT_VAULT_SETTINGS, + primaryNotesLocation: "root", + favorites: ["Ideas.md", "inbox:Projects"], + folderIcons: { "inbox:Projects": "bolt" }, + ...overrides, + }); +} + +describe("describeVaultSettingsConflict", () => { + it("reports only the sections whose values differ, with the leaf that differs", () => { + const local = localSettings(); + const cloudRaw = { + ...JSON.parse(JSON.stringify(local)), + favorites: ["inbox:Projects", "Ideas.md"], + folderIcons: { "inbox:Projects": "bolt", "inbox:Archive": "archive" }, + }; + const described = describeVaultSettingsConflict(local, cloudRaw); + expect(described.differences.map((difference) => difference.section)).toEqual([ + "folderIcons", + "favorites", + ]); + expect(described.differences[0].fields).toEqual([ + { path: ["folderIcons", "inbox:Archive"], local: undefined, cloud: "archive" }, + ]); + expect(described.unknownKeys).toEqual([]); + }); + + it("finds nothing to ask about when the cloud's file is this device's file", () => { + const local = localSettings({ + dailyNotes: { ...DEFAULT_VAULT_SETTINGS.dailyNotes, enabled: true, directory: "Journal" }, + }); + const described = describeVaultSettingsConflict(local, JSON.parse(JSON.stringify(local))); + expect(described.differences).toEqual([]); + }); + + it("does not report a primary location the cloud's file never states", () => { + const local = localSettings(); + const cloudRaw = JSON.parse(JSON.stringify(local)) as Record; + delete cloudRaw.primaryNotesLocation; + const described = describeVaultSettingsConflict(local, cloudRaw); + expect(described.cloud.primaryNotesLocation).toBe("root"); + expect(described.differences).toEqual([]); + }); + + it("names the keys another client wrote that this one has no setting for", () => { + const local = localSettings(); + const cloudRaw = { + ...JSON.parse(JSON.stringify(local)), + widgets: { weather: true }, + accentColor: "teal", + }; + const described = describeVaultSettingsConflict(local, cloudRaw); + expect(described.unknownKeys).toEqual(["accentColor", "widgets"]); + expect(described.differences).toEqual([]); + }); + + it("compares the cloud's raw values after normalizing them, not before", () => { + const local = localSettings(); + const cloudRaw = { + ...JSON.parse(JSON.stringify(local)), + // An icon id this client does not know is dropped by the normalizer, + // exactly as it would be when the file is read from disk. + folderIcons: { "inbox:Projects": "bolt", "inbox:Later": "not-an-icon" }, + }; + expect(describeVaultSettingsConflict(local, cloudRaw).differences).toEqual([]); + }); +}); + +describe("vaultSettingsFieldLabel", () => { + it("uses the Settings window's words for known fields", () => { + expect(vaultSettingsFieldLabel("dailyNotes", ["dailyNotes", "tasksDueOnNoteDate"])).toBe( + "Tasks are due on the note's date", + ); + expect(vaultSettingsFieldLabel("primaryNotesLocation", ["primaryNotesLocation"])).toBe( + "Primary notes location", + ); + }); + + it("reads folder keys as folder paths", () => { + expect(vaultSettingsFieldLabel("folderIcons", ["folderIcons", "inbox:Projects/Ideas"])).toBe( + "Inbox/Projects/Ideas", + ); + expect(vaultSettingsFieldLabel("folderColors", ["folderColors", "quick:"])).toBe("Quick Notes"); + expect(vaultSettingsFieldLabel("systemFolderPaths", ["systemFolderPaths", "archive"])).toBe( + "Archive folder", + ); + expect(vaultSettingsFieldLabel("view", ["view", "systemFolderLabels", "trash"])).toBe( + "Label: Trash", + ); + }); + + it("spells out a key it has no words for instead of hiding it", () => { + expect(vaultSettingsFieldLabel("view", ["view", "showBreadcrumbs"])).toBe("Show breadcrumbs"); + expect(vaultSettingsFieldLabel("view", ["view", "kanbanColumnOrder", "status"])).toBe( + "Column order: status", + ); + }); +}); + +describe("formatVaultSettingsValue", () => { + it("translates enumerations and booleans", () => { + expect(formatVaultSettingsValue("primaryNotesLocation", ["primaryNotesLocation"], "root")).toBe( + "Vault root", + ); + expect( + formatVaultSettingsValue("drawingsLocation", ["drawingsLocation", "mode"], "active-note"), + ).toBe("Active note's folder"); + expect(formatVaultSettingsValue("dailyNotes", ["dailyNotes", "enabled"], true)).toBe("On"); + expect(formatVaultSettingsValue("dailyNotes", ["dailyNotes", "locale"], "system")).toBe("System"); + }); + + it("says when a side has no value at all", () => { + expect(formatVaultSettingsValue("dailyNotes", ["dailyNotes", "templateId"], undefined)).toBe( + "Not set", + ); + expect(formatVaultSettingsValue("view", ["view", "kanbanFolderRoot"], "")).toBe("Not set"); + expect(formatVaultSettingsValue("favorites", ["favorites"], [])).toBe("None"); + }); + + it("previews lists and counts ignored suggestions instead of printing hashes", () => { + expect( + formatVaultSettingsValue("favorites", ["favorites"], [ + "A.md", + "inbox:Projects", + "C.md", + "D.md", + "E.md", + "F.md", + ]), + ).toBe("A.md, Inbox/Projects, C.md, D.md and 2 more"); + expect( + formatVaultSettingsValue("harper", ["harper", "ignoredLints"], ["1", "2", "3"]), + ).toBe("3 suggestions"); + expect(formatVaultSettingsValue("harper", ["harper", "words"], ["zennotes"])).toBe("zennotes"); + }); +}); diff --git a/packages/app-core/src/lib/vault-settings-conflict.ts b/packages/app-core/src/lib/vault-settings-conflict.ts new file mode 100644 index 00000000..0b922606 --- /dev/null +++ b/packages/app-core/src/lib/vault-settings-conflict.ts @@ -0,0 +1,191 @@ +import type { NoteFolder, VaultSettings } from "@shared/ipc"; +import { + diffVaultSettings, + unknownVaultSettingsKeys, + type VaultSettingsSection, + type VaultSettingsSectionDifference, +} from "@zennotes/shared-domain/vault-settings-conflict"; +import { DEFAULT_SYSTEM_FOLDER_LABELS } from "./system-folder-labels"; +import { normalizeVaultSettings } from "./vault-layout"; + +/** + * Words for the vault settings comparison the Cloud settings prompt shows. + * The comparison itself is shared-domain's; this file only says what each + * section, field and value is called on screen, in the vocabulary the + * Settings window already uses for the same controls. + */ + +export const VAULT_SETTINGS_SECTION_LABELS: Record = { + primaryNotesLocation: "Primary notes location", + dailyNotes: "Daily notes", + weeklyNotes: "Weekly notes", + monthlyNotes: "Monthly notes", + drawingsLocation: "New drawings location", + databasesLocation: "New databases location", + tasksLocation: "New tasks location", + view: "View settings", + folderIcons: "Folder icons", + folderColors: "Folder colors", + favorites: "Favorites", + systemFolderPaths: "Built-in folder locations", + tasks: "Tasks", + typstPreambles: "Typst preambles", + harper: "Harper dictionary", +}; + +export interface VaultSettingsConflictDescription { + /** The cloud's copy read the way this device would read its own file. */ + cloud: VaultSettings; + differences: VaultSettingsSectionDifference[]; + /** Top-level keys of the cloud's file this client has no setting for. */ + unknownKeys: string[]; +} + +/** + * Compare this device's settings with the parked cloud copy. The copy is + * normalized first, so only real differences are reported, not the defaults a + * normalizer fills in. A copy silent on the primary notes location takes this + * device's answer, which is what the desktop host does with such a file too + * (it infers the location from the vault's layout, and this vault's layout is + * what the local answer already describes). + */ +export function describeVaultSettingsConflict( + local: VaultSettings, + cloudRaw: Record, +): VaultSettingsConflictDescription { + const cloud = normalizeVaultSettings({ + ...(cloudRaw as Partial), + primaryNotesLocation: + (cloudRaw.primaryNotesLocation as VaultSettings["primaryNotesLocation"] | undefined) ?? + local.primaryNotesLocation, + } as VaultSettings); + return { + cloud, + differences: diffVaultSettings(local, cloud), + unknownKeys: unknownVaultSettingsKeys(cloudRaw), + }; +} + +const FIELD_LABELS: Record = { + enabled: "Enabled", + directory: "Directory", + titlePattern: "Title pattern", + locale: "Locale", + templateId: "Template", + tasksDueOnNoteDate: "Tasks are due on the note's date", + rolloverUnfinishedTasks: "Roll unfinished tasks over", + mode: "Where", + folder: "Folder", + noteSortOrder: "Note sort order", + assetSortOrder: "Asset sort order", + groupByKind: "Group by kind", + tasksViewMode: "Tasks view", + kanbanGroupBy: "Board grouping", + kanbanFolderRoot: "Board folder root", + kanbanColumnTitles: "Column title", + kanbanColumnOrder: "Column order", + kanbanCardOrder: "Card order", + kanbanStatuses: "Board statuses", + autoReveal: "Reveal the active note", + systemFolderLabels: "Label", + unifiedSidebar: "Unified sidebar", + excludedFolders: "Folders left out of Tasks", + words: "Words", + ignoredLints: "Ignored suggestions", +}; + +/** + * What to call a differing leaf, given its path from the section root. The + * section itself is not repeated: the caller shows it as the heading. Keys the + * user typed (a folder path, a board name) are shown as they are. + */ +export function vaultSettingsFieldLabel( + section: VaultSettingsSection, + path: string[], +): string { + const rest = path.slice(1); + if (rest.length === 0) return VAULT_SETTINGS_SECTION_LABELS[section]; + if (section === "folderIcons" || section === "folderColors") { + return formatFolderKey(rest.join(".")); + } + if (section === "systemFolderPaths") { + return `${folderLabel(rest[0])} folder`; + } + const [head, ...tail] = rest; + const label = FIELD_LABELS[head] ?? humanize(head); + if (tail.length === 0) return label; + const detail = + head === "systemFolderLabels" ? folderLabel(tail.join(".")) : tail.join("."); + return `${label}: ${detail}`; +} + +/** + * A setting's value in words. Enumerations use the Settings window's labels; + * lists name their first items; a missing value is "Not set" rather than a + * blank cell, so the side without it still reads as an answer. + */ +export function formatVaultSettingsValue( + section: VaultSettingsSection, + path: string[], + value: unknown, +): string { + if (value === undefined || value === null || value === "") return "Not set"; + if (typeof value === "boolean") return value ? "On" : "Off"; + if (typeof value === "number") return String(value); + const leaf = path[path.length - 1]; + if (Array.isArray(value)) { + if (value.length === 0) return "None"; + if (section === "harper" && leaf === "ignoredLints") { + return value.length === 1 ? "1 suggestion" : `${value.length} suggestions`; + } + const items = value.map((item) => + section === "favorites" ? formatFavorite(item) : String(item), + ); + return listPreview(items); + } + if (typeof value !== "string") return JSON.stringify(value); + if (section === "primaryNotesLocation") { + return value === "root" ? "Vault root" : value === "inbox" ? "Inbox" : value; + } + if (leaf === "mode") return FILE_LOCATION_LABELS[value] ?? value; + if (leaf === "locale" && value === "system") return "System"; + return value; +} + +const FILE_LOCATION_LABELS: Record = { + primary: "Primary location", + "active-note": "Active note's folder", + folder: "Specific folder", +}; + +const LIST_PREVIEW_LENGTH = 4; + +function listPreview(items: string[]): string { + if (items.length <= LIST_PREVIEW_LENGTH) return items.join(", "); + const rest = items.length - LIST_PREVIEW_LENGTH; + return `${items.slice(0, LIST_PREVIEW_LENGTH).join(", ")} and ${rest} more`; +} + +/** A favorite is a note path or a `folder:subpath` key (see VaultSettings). */ +function formatFavorite(entry: unknown): string { + if (typeof entry !== "string") return String(entry); + return entry.includes(":") ? formatFolderKey(entry) : entry; +} + +/** `inbox:Projects/Ideas` reads as `Inbox/Projects/Ideas`; a bare `inbox:` is the folder itself. */ +function formatFolderKey(key: string): string { + const separator = key.indexOf(":"); + if (separator === -1) return key; + const folder = folderLabel(key.slice(0, separator)); + const subpath = key.slice(separator + 1); + return subpath ? `${folder}/${subpath}` : folder; +} + +function folderLabel(folder: string): string { + return DEFAULT_SYSTEM_FOLDER_LABELS[folder as NoteFolder] ?? folder; +} + +function humanize(key: string): string { + const spaced = key.replace(/([a-z0-9])([A-Z])/g, "$1 $2").replace(/[_-]+/g, " "); + return spaced.charAt(0).toUpperCase() + spaced.slice(1).toLowerCase(); +} diff --git a/packages/app-core/src/store.ts b/packages/app-core/src/store.ts index e0a28673..70157cb2 100644 --- a/packages/app-core/src/store.ts +++ b/packages/app-core/src/store.ts @@ -3184,7 +3184,11 @@ interface Store { closedTabStack: ClosedTabEntry[] setVault: (v: VaultInfo | null) => void - setVaultSettings: (next: VaultSettings) => Promise + /** Resolves true once the settings are on disk. A failed write is logged, + * not thrown, because most callers fire and forget; the Cloud settings + * prompt reads the flag so it does not discard the cloud's copy after a + * save that never happened. */ + setVaultSettings: (next: VaultSettings) => Promise /** * Toggle a favorite (a note path or a `folder:subpath` key) and persist it. * Favorites pin to the top of the sidebar. @@ -5739,11 +5743,19 @@ export const useStore = create((set, get) => { set({ vaultSettings: settings }) + } catch (err) { + console.error('setVaultSettings failed', err) + return false + } + // The settings are saved at this point; a failed listing refresh is not a + // failed save. + try { await get().refreshNotes() await get().refreshRootContentHidden() } catch (err) { console.error('setVaultSettings failed', err) } + return true }, applyFavorites: async (nextFavorites) => { const isCurrent = captureFolderActionContext(get) diff --git a/packages/bridge-contract/src/cloud-sync.ts b/packages/bridge-contract/src/cloud-sync.ts index f868096f..be2f45bd 100644 --- a/packages/bridge-contract/src/cloud-sync.ts +++ b/packages/bridge-contract/src/cloud-sync.ts @@ -550,6 +550,13 @@ export interface CloudSyncPendingConflictResolution { export interface CloudSyncSettingsConflict { path: string; cloud_path: string; + /** + * The parked cloud vault.json, parsed, so the app can show which settings + * differ instead of asking for an all-or-nothing answer. Optional: hosts + * that predate it, or a parked copy that is not valid JSON, leave it out and + * the app falls back to the whole-file question. + */ + cloud_settings?: Record; } export type CloudSyncSettingsChoice = "local" | "cloud"; diff --git a/packages/shared-domain/src/vault-settings-conflict.test.ts b/packages/shared-domain/src/vault-settings-conflict.test.ts new file mode 100644 index 00000000..edc96474 --- /dev/null +++ b/packages/shared-domain/src/vault-settings-conflict.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, it } from 'vitest' +import { DEFAULT_VAULT_SETTINGS, type VaultSettings } from '@zennotes/bridge-contract/ipc' +import { + VAULT_SETTINGS_SECTIONS, + diffVaultSettings, + mergeVaultSettings, + unknownVaultSettingsKeys, + vaultSettingsValueEqual +} from './vault-settings-conflict' + +function settings(overrides: Partial = {}): VaultSettings { + return { + ...DEFAULT_VAULT_SETTINGS, + dailyNotes: { ...DEFAULT_VAULT_SETTINGS.dailyNotes }, + weeklyNotes: { ...DEFAULT_VAULT_SETTINGS.weeklyNotes }, + monthlyNotes: { ...DEFAULT_VAULT_SETTINGS.monthlyNotes }, + folderIcons: {}, + folderColors: {}, + favorites: [], + systemFolderPaths: {}, + ...overrides + } +} + +describe('diffVaultSettings', () => { + it('reports nothing for two files that differ only in key order', () => { + const local = settings({ + favorites: ['inbox/A.md', 'inbox/B.md'], + folderIcons: { 'inbox:Work': 'briefcase', 'inbox:Home': 'home' } + }) + const cloud = settings({ + folderIcons: { 'inbox:Home': 'home', 'inbox:Work': 'briefcase' }, + favorites: ['inbox/A.md', 'inbox/B.md'] + }) + expect(diffVaultSettings(local, cloud)).toEqual([]) + }) + + it('names the leaf that differs inside a section, and only the sections that differ', () => { + const local = settings({ + dailyNotes: { ...DEFAULT_VAULT_SETTINGS.dailyNotes, enabled: true, directory: 'Journal' } + }) + const cloud = settings({ + dailyNotes: { ...DEFAULT_VAULT_SETTINGS.dailyNotes, enabled: true, directory: 'Daily Notes' }, + favorites: ['inbox/Plan.md'] + }) + const differences = diffVaultSettings(local, cloud) + expect(differences.map((difference) => difference.section)).toEqual(['dailyNotes', 'favorites']) + expect(differences[0].fields).toEqual([ + { path: ['dailyNotes', 'directory'], local: 'Journal', cloud: 'Daily Notes' } + ]) + // A list is one setting: the whole favorites order is the leaf. + expect(differences[1].fields).toEqual([ + { path: ['favorites'], local: [], cloud: ['inbox/Plan.md'] } + ]) + }) + + it('treats a reordered list as a difference', () => { + const local = settings({ favorites: ['inbox/A.md', 'inbox/B.md'] }) + const cloud = settings({ favorites: ['inbox/B.md', 'inbox/A.md'] }) + expect(diffVaultSettings(local, cloud)).toHaveLength(1) + }) + + it('ignores the date-note pattern history the app maintains on its own', () => { + const local = settings({ + dailyNotes: { + ...DEFAULT_VAULT_SETTINGS.dailyNotes, + legacyPatterns: [{ directory: 'Old', titlePattern: 'yyyy-MM-dd', locale: 'system' }] + } + }) + const cloud = settings() + expect(diffVaultSettings(local, cloud)).toEqual([]) + }) + + it('compares a section one side lacks field by field, not as one opaque value', () => { + const local = settings({ view: { noteSortOrder: 'title-asc', autoReveal: true } }) + const cloud = settings() + const [difference] = diffVaultSettings(local, cloud) + expect(difference.section).toBe('view') + expect(difference.fields).toEqual([ + { path: ['view', 'autoReveal'], local: true, cloud: undefined }, + { path: ['view', 'noteSortOrder'], local: 'title-asc', cloud: undefined } + ]) + // An empty object and an absent section mean the same thing. + expect(diffVaultSettings(settings({ view: {} }), settings())).toEqual([]) + }) + + it('reports each remapped built-in folder on its own', () => { + const local = settings({ systemFolderPaths: { inbox: '01 - Entry' } }) + const cloud = settings({ systemFolderPaths: { inbox: 'Inbox', archive: 'Old' } }) + const [difference] = diffVaultSettings(local, cloud) + expect(difference.fields).toEqual([ + { path: ['systemFolderPaths', 'archive'], local: undefined, cloud: 'Old' }, + { path: ['systemFolderPaths', 'inbox'], local: '01 - Entry', cloud: 'Inbox' } + ]) + }) + + it('covers every vault.json section', () => { + const declared: Record = { + primaryNotesLocation: true, + dailyNotes: true, + weeklyNotes: true, + monthlyNotes: true, + drawingsLocation: true, + databasesLocation: true, + tasksLocation: true, + view: true, + folderIcons: true, + folderColors: true, + favorites: true, + systemFolderPaths: true, + tasks: true, + typstPreambles: true, + harper: true + } + expect([...VAULT_SETTINGS_SECTIONS].sort()).toEqual(Object.keys(declared).sort()) + }) +}) + +describe('mergeVaultSettings', () => { + const local = settings({ + favorites: ['inbox/Mine.md'], + folderIcons: { 'inbox:Work': 'briefcase' }, + view: { noteSortOrder: 'title-asc' } + }) + const cloud = settings({ + favorites: ['inbox/Theirs.md'], + folderIcons: { 'inbox:Work': 'star' }, + harper: { words: ['zennotes'], ignoredLints: [] } + }) + + it('takes the cloud value only for the sections answered cloud', () => { + const merged = mergeVaultSettings(local, cloud, { favorites: 'cloud', folderIcons: 'local' }) + expect(merged.favorites).toEqual(['inbox/Theirs.md']) + expect(merged.folderIcons).toEqual({ 'inbox:Work': 'briefcase' }) + // Unanswered sections are this device's, whichever side has a value. + expect(merged.view).toEqual({ noteSortOrder: 'title-asc' }) + expect(merged.harper).toBeUndefined() + }) + + it('drops a section the cloud does not have when the cloud is chosen for it', () => { + const merged = mergeVaultSettings(local, cloud, { view: 'cloud', harper: 'cloud' }) + expect('view' in merged).toBe(false) + expect(merged.harper).toEqual({ words: ['zennotes'], ignoredLints: [] }) + }) + + it('leaves both inputs untouched', () => { + const localBefore = JSON.stringify(local) + const cloudBefore = JSON.stringify(cloud) + mergeVaultSettings(local, cloud, { favorites: 'cloud', view: 'cloud' }) + expect(JSON.stringify(local)).toBe(localBefore) + expect(JSON.stringify(cloud)).toBe(cloudBefore) + }) +}) + +describe('unknownVaultSettingsKeys', () => { + it('lists, sorted, the top-level keys this runtime has no section for', () => { + expect( + unknownVaultSettingsKeys({ favorites: [], zeta: 1, alpha: { nested: true }, view: {} }) + ).toEqual(['alpha', 'zeta']) + }) + + it('has nothing to say about a file that is not an object', () => { + expect(unknownVaultSettingsKeys(null)).toEqual([]) + expect(unknownVaultSettingsKeys(['favorites'])).toEqual([]) + expect(unknownVaultSettingsKeys('{}')).toEqual([]) + }) +}) + +describe('vaultSettingsValueEqual', () => { + it('ignores undefined properties but not null ones', () => { + expect(vaultSettingsValueEqual({ a: 1, b: undefined }, { a: 1 })).toBe(true) + expect(vaultSettingsValueEqual({ a: 1, b: null }, { a: 1 })).toBe(false) + }) + + it('does not confuse a list with an object of the same keys', () => { + expect(vaultSettingsValueEqual(['a'], { 0: 'a' })).toBe(false) + }) +}) diff --git a/packages/shared-domain/src/vault-settings-conflict.ts b/packages/shared-domain/src/vault-settings-conflict.ts new file mode 100644 index 00000000..f1a1fafd --- /dev/null +++ b/packages/shared-domain/src/vault-settings-conflict.ts @@ -0,0 +1,184 @@ +/** + * Comparing two vault.json files section by section. + * + * Cloud sync parks the cloud's vault.json beside this device's when both + * changed, and the app asks which to keep. A whole-file answer forces a + * choice between "my favorites" and "their folder icons" when the user wants + * both, so the question is asked per top-level section instead: this module + * says which sections differ, down to the leaf that differs, and builds the + * settings that result from a per-section answer. It is pure and shared so + * every runtime asks the same question of the same bytes. + * + * Both sides are expected already normalized by the runtime's own vault + * settings normalizer. Comparing a raw file against a normalized one reports + * every default the normalizer filled in as a difference. + */ + +import type { VaultSettings } from '@zennotes/bridge-contract/ipc' + +export type VaultSettingsSection = keyof VaultSettings + +// The type error a new VaultSettings key raises here is deliberate: the +// section list is what the settings conflict prompt offers, so a new section +// must be added (or knowingly left out) rather than silently never shown. +const SECTIONS: Record = { + primaryNotesLocation: true, + dailyNotes: true, + weeklyNotes: true, + monthlyNotes: true, + drawingsLocation: true, + databasesLocation: true, + tasksLocation: true, + view: true, + folderIcons: true, + folderColors: true, + favorites: true, + systemFolderPaths: true, + tasks: true, + typstPreambles: true, + harper: true +} + +export const VAULT_SETTINGS_SECTIONS = Object.keys(SECTIONS) as VaultSettingsSection[] + +export function isVaultSettingsSection(key: string): key is VaultSettingsSection { + return Object.prototype.hasOwnProperty.call(SECTIONS, key) +} + +export type VaultSettingsSide = 'local' | 'cloud' + +/** + * One value that differs, addressed by its path from the section root + * (`['dailyNotes', 'directory']`, `['folderIcons', 'inbox:Projects']`). A list + * is one leaf, order included: favorites and board columns are ordered by the + * user, so a reordered list is a different setting, not the same set. + */ +export interface VaultSettingsFieldDifference { + path: string[] + local: unknown + cloud: unknown +} + +export interface VaultSettingsSectionDifference { + section: VaultSettingsSection + local: unknown + cloud: unknown + fields: VaultSettingsFieldDifference[] +} + +/** + * Bookkeeping the app maintains on the user's behalf rather than a choice the + * user made. Two devices almost always disagree on it once one of them changed + * the pattern it records, and asking about it would only bury the pattern + * change that matters. The chosen side's value still travels with its section + * when the answer is applied. + */ +const IGNORED_PATHS = new Set([ + 'dailyNotes.legacyPatterns', + 'weeklyNotes.legacyPatterns', + 'monthlyNotes.legacyPatterns' +]) + +/** Sections that differ between the two sides, each with the leaves that differ. */ +export function diffVaultSettings( + local: VaultSettings, + cloud: VaultSettings +): VaultSettingsSectionDifference[] { + const localRecord = local as unknown as Record + const cloudRecord = cloud as unknown as Record + const differences: VaultSettingsSectionDifference[] = [] + for (const section of VAULT_SETTINGS_SECTIONS) { + const fields: VaultSettingsFieldDifference[] = [] + collectDifferences([section], localRecord[section], cloudRecord[section], fields) + if (fields.length === 0) continue + differences.push({ + section, + local: localRecord[section], + cloud: cloudRecord[section], + fields + }) + } + return differences +} + +/** + * The settings that result from answering per section. Sections without an + * answer, or answered `local`, keep this device's value; a section answered + * `cloud` takes the cloud's value, or disappears when the cloud has none, so + * the runtime's normalizer fills in its default exactly as it would for the + * cloud's own file. + */ +export function mergeVaultSettings( + local: VaultSettings, + cloud: VaultSettings, + choices: Partial> +): VaultSettings { + const merged: Record = { ...(local as unknown as Record) } + const cloudRecord = cloud as unknown as Record + for (const section of VAULT_SETTINGS_SECTIONS) { + if (choices[section] !== 'cloud') continue + if (cloudRecord[section] === undefined) delete merged[section] + else merged[section] = cloudRecord[section] + } + return merged as unknown as VaultSettings +} + +/** + * Top-level keys of a raw vault.json this runtime has no section for: settings + * a newer or different client wrote. They cannot be compared or chosen here, + * and the prompt says so rather than pretending the files are otherwise equal. + */ +export function unknownVaultSettingsKeys(raw: unknown): string[] { + if (!isPlainObject(raw)) return [] + return Object.keys(raw) + .filter((key) => !isVaultSettingsSection(key)) + .sort() +} + +/** Structural equality: key order is not a difference, absent and undefined are the same. */ +export function vaultSettingsValueEqual(a: unknown, b: unknown): boolean { + if (a === b) return true + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false + return a.every((item, index) => vaultSettingsValueEqual(item, b[index])) + } + if (isPlainObject(a) && isPlainObject(b)) { + const keys = new Set([...definedKeys(a), ...definedKeys(b)]) + for (const key of keys) { + if (!vaultSettingsValueEqual(a[key], b[key])) return false + } + return true + } + return false +} + +function collectDifferences( + path: string[], + local: unknown, + cloud: unknown, + out: VaultSettingsFieldDifference[] +): void { + if (IGNORED_PATHS.has(path.join('.'))) return + // An object on one side and nothing on the other is compared field by + // field against an empty object, so the prompt can name the fields that + // would appear or disappear rather than one opaque "view" leaf. + const localObject = isPlainObject(local) ? local : local === undefined ? {} : null + const cloudObject = isPlainObject(cloud) ? cloud : cloud === undefined ? {} : null + if (localObject && cloudObject && (isPlainObject(local) || isPlainObject(cloud))) { + const keys = [...new Set([...definedKeys(localObject), ...definedKeys(cloudObject)])].sort() + for (const key of keys) { + collectDifferences([...path, key], localObject[key], cloudObject[key], out) + } + return + } + if (vaultSettingsValueEqual(local, cloud)) return + out.push({ path, local, cloud }) +} + +function definedKeys(value: Record): string[] { + return Object.keys(value).filter((key) => value[key] !== undefined) +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} diff --git a/tooling/scripts/cloud-conflict-demo-fixture.mjs b/tooling/scripts/cloud-conflict-demo-fixture.mjs index 96c02372..0de8a97d 100644 --- a/tooling/scripts/cloud-conflict-demo-fixture.mjs +++ b/tooling/scripts/cloud-conflict-demo-fixture.mjs @@ -102,6 +102,7 @@ const server = createServer(async (request, response) => { items.set(noteId, next) revisions.get(noteId)?.push(revisionRecord(next, 'upsert')) changes.push(changeRecord(next, 'upsert', sequence, notePath)) + armOtherDeviceSettings() } return json(response, 200, { armed, cursor: sequence }) } @@ -302,6 +303,54 @@ function vaultSummary() { } } +/** + * The other device also changed the vault's settings: it turned daily notes + * on, gave the Plans folder an icon, pinned the trip note, and runs a build + * with a setting this one does not know. The cloud only holds a vault.json + * once the desktop's first sync has pushed it, so arming before that link is + * a no-op for settings. With a local settings change since the last sync, + * the next sync parks this copy and the app asks per section (#816). + */ +function armOtherDeviceSettings() { + const settingsPath = '.zennotes/vault.json' + const current = [...items.values()].find( + (item) => !item.deleted && item.path.toLowerCase() === settingsPath + ) + if (!current || current.kind !== 'text') return + let settings + try { + settings = JSON.parse(current.content.data) + } catch { + return + } + // Pinned on the other device: the first note this desktop pushed, so the + // favorites lists disagree even when this device pins the trip note. + const pinned = + [...items.values()].find( + (item) => !item.deleted && item.kind === 'text' && item.path.endsWith('.md') && item.path !== notePath + )?.path ?? notePath + const otherDevice = { + ...settings, + dailyNotes: { ...(settings.dailyNotes ?? {}), enabled: true, directory: 'Journal' }, + folderIcons: { ...(settings.folderIcons ?? {}), 'inbox:Plans': 'map' }, + favorites: [pinned], + experimentalSpellcheck: { enabled: true } + } + const next = { + ...current, + revision: current.revision + 1, + content: { + ...textContent(`${JSON.stringify(otherDevice, null, 2)}\n`), + media_type: 'application/json' + }, + deleted: false + } + sequence += 1 + items.set(current.item_id, next) + revisions.get(current.item_id)?.push(revisionRecord(next, 'upsert')) + changes.push(changeRecord(next, 'upsert', sequence, settingsPath)) +} + function applyMutation(current, mutation, revision) { if (mutation.type === 'delete') { return { From c5c3933d414bdb67fd26370be42afb04aa589e74 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 12:29:44 -0500 Subject: [PATCH 03/12] Fix(updater): a check that finds no network waits for it and tries again by itself (#812) The startup update check runs once, 8 seconds after launch, and never rescheduled itself. Launch before the network is up and that one check failed, Settings > About read "Update error", the "update needs attention" toast fired, and nothing tried again until the user pressed Check for Updates. On the package-manager path the message was just "fetch failed": undici keeps the real cause (ENOTFOUND, ENETUNREACH) in error.cause. @uNyanda reported it from an AUR install (#812). A check that cannot reach GitHub at all is no longer an error. The updater enters a new offline phase ("Waiting for network" in About, the cause named in the message, no toast) and waits: net.isOnline() is read every 15 s without making a request, and the check runs again on a down-to-up transition. When the link is up but GitHub still cannot be reached (a captive portal, a router with no WAN) it retries 30 s after the failure, doubling up to every 15 minutes, and it makes a real attempt at least every 15 minutes whatever isOnline() says, so a notifier that is wrong about this machine cannot silence the check. Any phase other than offline or checking ends the wait, so a manual check owns its result. electron-updater's own error event is ignored for network-class errors so the state never flashes error before the check's catch decides. Errors that are not the network (a 404, a bad signature) still show as errors, now with the most specific line of the cause chain instead of "fetch failed". isNetworkUnreachableError walks message, code and cause five levels deep for the Node, undici and Chromium net::ERR_* spellings, including undici's bare "fetch failed". The bridge contract gains the offline phase; a host that does not know it shows no badge, notice or action. Verified with fake timers and a mocked net.isOnline (link down: no request until the link is back; link up: retries at 30, 60 and 120 s; the 15-minute attempt; a 404 does not wait; a manual check while waiting), and live in the built app over CDP with the feed on a closed local port: "Waiting for network (connect ECONNREFUSED ...)" and no toast, then, with the feed up and nobody touching the app, "Update available" 25 s later. Not exercised live: the electron-updater path and a real link transition. Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 Co-authored-by: Amp --- apps/desktop/src/main/index.ts | 8 +- apps/desktop/src/main/updater.test.ts | 225 ++++++++++++++++-- apps/desktop/src/main/updater.ts | 151 +++++++++++- .../app-core/src/components/SettingsModal.tsx | 10 +- .../app-core/src/lib/app-update-state.test.ts | 12 + packages/app-core/src/lib/help.ts | 2 +- packages/bridge-contract/src/ipc.ts | 6 + 7 files changed, 380 insertions(+), 34 deletions(-) diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 2fd2d629..127fad70 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -5235,9 +5235,11 @@ async function runMenuUpdateCheck(): Promise { ? "ZenNotes is up to date." : state.phase === "unsupported" ? "Update checks are unavailable." - : state.phase === "error" - ? "Could not check for updates." - : "ZenNotes Updates", + : state.phase === "offline" + ? "ZenNotes can't reach GitHub right now." + : state.phase === "error" + ? "Could not check for updates." + : "ZenNotes Updates", detail: state.message, }); } diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts index 448ea078..285cd1bf 100644 --- a/apps/desktop/src/main/updater.test.ts +++ b/apps/desktop/src/main/updater.test.ts @@ -1,11 +1,16 @@ import { afterEach, describe, expect, it, vi } from 'vitest' +// What Chromium's network change notifier would answer; tests flip it to +// play a link going down and coming back. +const network = vi.hoisted(() => ({ online: true })) + // updater.ts imports electron and electron-updater at module load. Stub both so // we can unit-test the pure Linux-install helpers without an Electron runtime. vi.mock('electron', () => ({ app: { getVersion: () => '2.0.2' }, BrowserWindow: { getAllWindows: () => [] }, Notification: { isSupported: () => false }, + net: { isOnline: () => network.online }, shell: {} })) vi.mock('electron-updater', () => ({ @@ -23,6 +28,7 @@ import electronUpdater from 'electron-updater' import { elevatedInstallScript, installedLinuxFormat, + isNetworkUnreachableError, isOfficialLinuxSystemPackage, linuxFormatFromOsRelease, linuxInstallMismatch, @@ -31,7 +37,11 @@ import { linuxUpdaterForFormat, linuxUpdaterFormat, manualInstallHint, - mismatchedUpdateMessage + mismatchedUpdateMessage, + offlineRetryDelayMs, + OFFLINE_POLL_MS, + OFFLINE_RETRY_BASE_MS, + OFFLINE_RETRY_MAX_MS } from './updater' describe('linuxPackageFormat', () => { @@ -251,34 +261,51 @@ describe('Linux updater build support', () => { }) }) +const FEED = (version: string) => + `version: ${version}\nfiles: []\nreleaseDate: '2026-09-02T15:28:11.000Z'\n` + +/** What undici's fetch throws with no network: the readable part is in `cause`. */ +function fetchFailed(code: string, detail: string): TypeError { + return new TypeError('fetch failed', { + cause: Object.assign(new Error(detail), { code }) + }) +} + +/** + * A fresh updater module on the package-manager (notify-only) path, its feed + * served by `feed`: a fixed body, a fixed error, or a function answering per + * call so a test can bring the network back partway through. + */ +async function loadManagedUpdater(feed: string | Error | (() => string | Error)) { + vi.resetModules() + process.env.ZENNOTES_UPDATER_FORMAT = 'managed' + process.env.ZENNOTES_UPDATE_FEED_URL = 'http://127.0.0.1:1/latest-linux.yml' + const fetchMock = vi.fn(async () => { + const body = typeof feed === 'function' ? feed() : feed + if (body instanceof Error) throw body + return { ok: true, status: 200, text: async () => body } + }) + vi.stubGlobal('fetch', fetchMock) + return { mod: await import('./updater'), fetchMock } +} + +function restoreUpdaterEnv(original: { format?: string; feed?: string }): void { + vi.unstubAllGlobals() + if (original.format === undefined) delete process.env.ZENNOTES_UPDATER_FORMAT + else process.env.ZENNOTES_UPDATER_FORMAT = original.format + if (original.feed === undefined) delete process.env.ZENNOTES_UPDATE_FEED_URL + else process.env.ZENNOTES_UPDATE_FEED_URL = original.feed +} + describe('checkForAppUpdates on a package-manager install', () => { - const FEED = (version: string) => `version: ${version}\nfiles: []\nreleaseDate: '2026-09-02T15:28:11.000Z'\n` const original = { format: process.env.ZENNOTES_UPDATER_FORMAT, feed: process.env.ZENNOTES_UPDATE_FEED_URL } - async function loadManagedUpdater(feedBody: string | Error) { - vi.resetModules() - process.env.ZENNOTES_UPDATER_FORMAT = 'managed' - process.env.ZENNOTES_UPDATE_FEED_URL = 'http://127.0.0.1:1/latest-linux.yml' - vi.stubGlobal( - 'fetch', - vi.fn(async () => { - if (feedBody instanceof Error) throw feedBody - return { ok: true, status: 200, text: async () => feedBody } - }) - ) - return await import('./updater') - } - afterEach(() => { - vi.unstubAllGlobals() - if (original.format === undefined) delete process.env.ZENNOTES_UPDATER_FORMAT - else process.env.ZENNOTES_UPDATER_FORMAT = original.format - if (original.feed === undefined) delete process.env.ZENNOTES_UPDATE_FEED_URL - else process.env.ZENNOTES_UPDATE_FEED_URL = original.feed + restoreUpdaterEnv(original) }) it('reports a newer version without offering to install it', async () => { - const mod = await loadManagedUpdater(FEED('2.0.3')) + const { mod } = await loadManagedUpdater(FEED('2.0.3')) const state = await mod.checkForAppUpdates() expect(state.phase).toBe('available') expect(state.availableVersion).toBe('2.0.3') @@ -289,7 +316,7 @@ describe('checkForAppUpdates on a package-manager install', () => { }) it('says so when the running version is the newest', async () => { - const mod = await loadManagedUpdater(FEED('2.0.2')) + const { mod } = await loadManagedUpdater(FEED('2.0.2')) const state = await mod.checkForAppUpdates() expect(state.phase).toBe('not-available') expect(state.message).toBe("You're already on ZenNotes 2.0.2.") @@ -297,10 +324,158 @@ describe('checkForAppUpdates on a package-manager install', () => { }) it('surfaces a feed failure instead of staying on checking', async () => { - const mod = await loadManagedUpdater(new Error('getaddrinfo EAI_FAIL github.com')) + const { mod } = await loadManagedUpdater(new Error('GitHub answered 404 for the release feed.')) const state = await mod.checkForAppUpdates() expect(state.phase).toBe('error') - expect(state.message).toMatch(/EAI_FAIL/) + expect(state.message).toMatch(/404/) + }) +}) + +describe('isNetworkUnreachableError', () => { + it('recognizes no-network failures from both HTTP stacks, cause chain included', () => { + // undici (the package-manager feed check): the code is two levels down. + expect(isNetworkUnreachableError(fetchFailed('ENOTFOUND', 'getaddrinfo ENOTFOUND github.com'))).toBe(true) + expect(isNetworkUnreachableError(fetchFailed('ENETUNREACH', 'connect ENETUNREACH 140.82.121.4:443'))).toBe(true) + // Electron's net module (electron-updater): Chromium's name in the message. + expect(isNetworkUnreachableError(new Error('net::ERR_INTERNET_DISCONNECTED'))).toBe(true) + expect(isNetworkUnreachableError(new Error('net::ERR_NAME_NOT_RESOLVED'))).toBe(true) + // Plain Node errors, as a message or as a code. + expect(isNetworkUnreachableError(new Error('getaddrinfo EAI_AGAIN github.com'))).toBe(true) + expect(isNetworkUnreachableError(Object.assign(new Error('connect failed'), { code: 'ECONNREFUSED' }))).toBe(true) + }) + + it('leaves answers from GitHub, and everything else, to the error path', () => { + expect(isNetworkUnreachableError(new Error('GitHub answered 404 for the release feed.'))).toBe(false) + expect(isNetworkUnreachableError(new Error('HttpError: 503 Service Unavailable'))).toBe(false) + expect(isNetworkUnreachableError(new Error('The release feed carried no version.'))).toBe(false) + // "connection" in prose is not a connection error code. + expect(isNetworkUnreachableError(new Error('Could not verify the connection to the signing service'))).toBe(false) + expect(isNetworkUnreachableError(undefined)).toBe(false) + }) +}) + +describe('offlineRetryDelayMs', () => { + it('doubles from the base up to the cap', () => { + expect(offlineRetryDelayMs(1)).toBe(OFFLINE_RETRY_BASE_MS) + expect(offlineRetryDelayMs(2)).toBe(OFFLINE_RETRY_BASE_MS * 2) + expect(offlineRetryDelayMs(3)).toBe(OFFLINE_RETRY_BASE_MS * 4) + expect(offlineRetryDelayMs(6)).toBe(OFFLINE_RETRY_MAX_MS) + // Far past the cap, and past where 2 ** n stops being a safe integer. + expect(offlineRetryDelayMs(60)).toBe(OFFLINE_RETRY_MAX_MS) + expect(offlineRetryDelayMs(0)).toBe(OFFLINE_RETRY_BASE_MS) + }) +}) + +describe('checkForAppUpdates with no network (issue #812)', () => { + const original = { format: process.env.ZENNOTES_UPDATER_FORMAT, feed: process.env.ZENNOTES_UPDATE_FEED_URL } + const offline = fetchFailed('ENOTFOUND', 'getaddrinfo ENOTFOUND github.com') + + afterEach(() => { + vi.useRealTimers() + network.online = true + restoreUpdaterEnv(original) + }) + + it('waits instead of erroring, and checks again by itself when the link comes back', async () => { + vi.useFakeTimers() + network.online = false + let reachable = false + const { mod, fetchMock } = await loadManagedUpdater(() => (reachable ? FEED('2.0.3') : offline)) + + const state = await mod.checkForAppUpdates() + expect(state.phase).toBe('offline') + // The readable cause, not undici's "fetch failed". + expect(state.message).toContain('getaddrinfo ENOTFOUND github.com') + expect(state.message).toMatch(/check for updates again on its own/) + expect(fetchMock).toHaveBeenCalledTimes(1) + + // Link down for a while: no requests are wasted on it. + await vi.advanceTimersByTimeAsync(5 * 60_000) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(mod.getAppUpdateState().phase).toBe('offline') + + // The link returns: one poll later the check runs and gets its answer. + network.online = true + reachable = true + await vi.advanceTimersByTimeAsync(OFFLINE_POLL_MS) + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(mod.getAppUpdateState().phase).toBe('available') + expect(mod.getAppUpdateState().availableVersion).toBe('2.0.3') + + // Answered: nothing keeps polling. + await vi.advanceTimersByTimeAsync(60 * 60_000) + expect(fetchMock).toHaveBeenCalledTimes(2) + }) + + it('backs off while the link is up but GitHub stays out of reach, then stops once answered', async () => { + vi.useFakeTimers() + network.online = true + let reachable = false + const { mod, fetchMock } = await loadManagedUpdater(() => (reachable ? FEED('2.0.2') : offline)) + + expect((await mod.checkForAppUpdates()).phase).toBe('offline') + expect(fetchMock).toHaveBeenCalledTimes(1) + + // First retry after the base delay (polls land on 15 s marks). + await vi.advanceTimersByTimeAsync(OFFLINE_RETRY_BASE_MS - OFFLINE_POLL_MS) + expect(fetchMock).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(OFFLINE_POLL_MS) + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(mod.getAppUpdateState().phase).toBe('offline') + + // Second retry waits twice as long: nothing at the base delay again. + await vi.advanceTimersByTimeAsync(OFFLINE_RETRY_BASE_MS) + expect(fetchMock).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(OFFLINE_RETRY_BASE_MS) + expect(fetchMock).toHaveBeenCalledTimes(3) + + // GitHub is back; the third retry (four times the base) succeeds and ends the wait. + reachable = true + await vi.advanceTimersByTimeAsync(OFFLINE_RETRY_BASE_MS * 4) + expect(fetchMock).toHaveBeenCalledTimes(4) + expect(mod.getAppUpdateState().phase).toBe('not-available') + await vi.advanceTimersByTimeAsync(60 * 60_000) + expect(fetchMock).toHaveBeenCalledTimes(4) + }) + + it('still makes a real attempt now and then if the notifier keeps saying offline', async () => { + vi.useFakeTimers() + network.online = false + const { mod, fetchMock } = await loadManagedUpdater(offline) + + expect((await mod.checkForAppUpdates()).phase).toBe('offline') + await vi.advanceTimersByTimeAsync(OFFLINE_RETRY_MAX_MS - OFFLINE_POLL_MS) + expect(fetchMock).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(OFFLINE_POLL_MS) + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(mod.getAppUpdateState().phase).toBe('offline') + }) + + it('does not wait on errors that are not the network', async () => { + vi.useFakeTimers() + const { mod, fetchMock } = await loadManagedUpdater(new Error('GitHub answered 404 for the release feed.')) + + expect((await mod.checkForAppUpdates()).phase).toBe('error') + await vi.advanceTimersByTimeAsync(60 * 60_000) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('lets a manual check while waiting run at once, and keeps waiting if it fails the same way', async () => { + vi.useFakeTimers() + network.online = true + let reachable = false + const { mod, fetchMock } = await loadManagedUpdater(() => (reachable ? FEED('2.0.2') : offline)) + + expect((await mod.checkForAppUpdates()).phase).toBe('offline') + // The user presses Check for Updates before any retry is due. + expect((await mod.checkForAppUpdates()).phase).toBe('offline') + expect(fetchMock).toHaveBeenCalledTimes(2) + + // The wait survived the manual check and still recovers on its own. + reachable = true + await vi.advanceTimersByTimeAsync(OFFLINE_RETRY_MAX_MS) + expect(mod.getAppUpdateState().phase).toBe('not-available') + expect(fetchMock).toHaveBeenCalledTimes(3) }) }) diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index c44d0334..7bb30bdf 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -1,4 +1,4 @@ -import { app, BrowserWindow, Notification, shell } from 'electron' +import { app, BrowserWindow, net, Notification, shell } from 'electron' import { execFile } from 'node:child_process' import { existsSync, readFileSync } from 'node:fs' import { fetchLatestRelease, isNewerVersion } from './update-feed' @@ -16,6 +16,12 @@ const execFileAsync = promisify(execFile) const UPDATE_CHECK_MAX_ATTEMPTS = 3 const UPDATE_CHECK_RETRY_DELAY_MS = 1500 const BACKGROUND_UPDATE_CHECK_DELAY_MS = 8000 +/** How often a check that found no network looks at the link again. */ +export const OFFLINE_POLL_MS = 15_000 +/** First wait before re-checking when the link is up but GitHub still could + * not be reached; doubles per failure up to the cap. */ +export const OFFLINE_RETRY_BASE_MS = 30_000 +export const OFFLINE_RETRY_MAX_MS = 15 * 60_000 let initialized = false let updater: AppUpdater | null = null @@ -86,10 +92,54 @@ function nextStateFromInfo( }) } +/** + * Every message and code along an error's `cause` chain, innermost last. + * undici's fetch reports the network as `TypeError: fetch failed` and keeps + * the part worth reading (`getaddrinfo ENOTFOUND github.com`) in `cause`; + * Electron's net module puts it in the message (`net::ERR_INTERNET_DISCONNECTED`). + */ +function errorChainText(error: unknown): string[] { + const parts: string[] = [] + let current: unknown = error + for (let depth = 0; depth < 5 && current != null; depth += 1) { + if (current instanceof Error) { + parts.push(current.message) + const { code, cause } = current as { code?: unknown; cause?: unknown } + if (typeof code === 'string') parts.push(code) + current = cause + } else { + parts.push(String(current)) + break + } + } + return parts.map((part) => part.trim()).filter(Boolean) +} + +const NETWORK_UNREACHABLE_PATTERN = + /\b(ENOTFOUND|EAI_AGAIN|EAI_FAIL|EAI_NONAME|ENETUNREACH|ENETDOWN|EHOSTUNREACH|ECONNREFUSED|ECONNRESET|ETIMEDOUT|UND_ERR_CONNECT_TIMEOUT|UND_ERR_SOCKET)\b|net::ERR_(INTERNET_DISCONNECTED|NAME_NOT_RESOLVED|NAME_RESOLUTION_FAILED|DNS_TIMED_OUT|ADDRESS_UNREACHABLE|NETWORK_CHANGED|NETWORK_IO_SUSPENDED|NETWORK_ACCESS_DENIED|PROXY_CONNECTION_FAILED|TIMED_OUT|CONNECTION_(REFUSED|RESET|CLOSED|ABORTED|FAILED|TIMED_OUT))\b|^fetch failed$/i + +/** + * True when the check never reached GitHub: no route, no DNS, nothing + * listening. GitHub answering badly (a 5xx, a 404) is not this; those are + * errors to show, not a connection to wait for. + */ +export function isNetworkUnreachableError(error: unknown): boolean { + return errorChainText(error).some((part) => NETWORK_UNREACHABLE_PATTERN.test(part)) +} + +/** The most specific line of an error chain, for the message the user reads: + * `getaddrinfo ENOTFOUND github.com` rather than `fetch failed`. */ +function describeError(error: unknown): string { + const parts = errorChainText(error).filter((part) => !/^fetch failed$/i.test(part)) + const messages = parts.filter((part) => /\s|::/.test(part)) + return messages.at(-1) ?? parts.at(-1) ?? 'Unknown updater error.' +} + function humanizeUpdateError(error: unknown): string { - const base = - error instanceof Error ? error.message.trim() : String(error).trim() - const message = base.length > 0 ? base : 'Unknown updater error.' + const message = describeError(error) + if (isNetworkUnreachableError(error)) { + return `${message} ZenNotes could not reach GitHub. Check the connection and try again.` + } if (/5\d\d|gateway time-?out|timed out|econnreset|eai_again|socket hang up/i.test(message)) { return `${message} GitHub returned a temporary network or server error while checking for updates. Try again in a moment, or open the latest release directly.` } @@ -122,9 +172,89 @@ function broadcastUpdateState(): void { function setUpdateState(next: AppUpdateState): void { updateState = next + // The wait for the network lives exactly as long as the phase it explains. + // A check in flight keeps it (its failure count carries into the next + // wait); any other outcome, from either a timer or the user, ends it. + if (next.phase !== 'offline' && next.phase !== 'checking') stopWaitingForNetwork() broadcastUpdateState() } +/** + * A check that could not reach GitHub at all (issue #812: the app launched + * offline, the startup check failed, and nothing ever tried again until the + * user pressed Check for Updates). Instead of reporting an error, wait for + * the network and check again on our own. + * + * Two triggers, both cheap: the link coming back (Chromium's own network + * change notifier, read through `net.isOnline()` every OFFLINE_POLL_MS; no + * request is made), and a retry timer for the case the link is up but GitHub + * still cannot be reached (captive portal, the router's WAN side down). The + * retry delay doubles per failure up to OFFLINE_RETRY_MAX_MS. + * + * `isOnline()` answering false is trusted enough to skip retries while the + * link is down, but never for good: a real attempt happens at least every + * OFFLINE_RETRY_MAX_MS, so a notifier that is wrong about this machine + * cannot silence the check forever. + */ +interface NetworkWait { + timer: NodeJS.Timeout + failures: number + lastAttemptAt: number + linkWasUp: boolean +} + +let networkWait: NetworkWait | null = null + +export function offlineRetryDelayMs(failures: number): number { + const doublings = Math.max(0, Math.min(failures - 1, 30)) + return Math.min(OFFLINE_RETRY_BASE_MS * 2 ** doublings, OFFLINE_RETRY_MAX_MS) +} + +function stopWaitingForNetwork(): void { + if (!networkWait) return + clearInterval(networkWait.timer) + networkWait = null +} + +function waitForNetwork(error: unknown): void { + const failures = (networkWait?.failures ?? 0) + 1 + stopWaitingForNetwork() + networkWait = { + failures, + lastAttemptAt: Date.now(), + linkWasUp: net.isOnline(), + timer: setInterval(pollForNetwork, OFFLINE_POLL_MS) + } + setUpdateState( + nextStateFromInfo( + 'offline', + lastInfo, + `ZenNotes can't reach GitHub right now (${describeError(error)}). It will check for updates again on its own once the connection is back.`, + { availableVersion: lastInfo?.version ?? updateState.availableVersion } + ) + ) +} + +function pollForNetwork(): void { + const wait = networkWait + if (!wait) return + if (updateState.phase !== 'offline') { + // A check the user started is running, or something else owns the state. + if (updateState.phase !== 'checking') stopWaitingForNetwork() + return + } + const linkUp = net.isOnline() + const linkRestored = linkUp && !wait.linkWasUp + wait.linkWasUp = linkUp + const sinceAttempt = Date.now() - wait.lastAttemptAt + const retryDue = sinceAttempt >= offlineRetryDelayMs(wait.failures) + const overdue = sinceAttempt >= OFFLINE_RETRY_MAX_MS + if (linkRestored || (retryDue && linkUp) || overdue) { + wait.lastAttemptAt = Date.now() + void checkForAppUpdates() + } +} + function focusAppAndOpenSettings(): void { const windows = BrowserWindow.getAllWindows().filter((win) => !win.isDestroyed()) const target = BrowserWindow.getFocusedWindow() ?? windows[0] ?? null @@ -270,6 +400,11 @@ export function initAppUpdater(): void { } }) updater.on('error', (error) => { + // electron-updater emits this and rejects the same promise. Let the check + // or download that owns the promise decide what an unreachable network + // means (a wait, not an error); reporting it here first would flash the + // error state through the renderer on the way. + if (isNetworkUnreachableError(error)) return setUpdateState( nextStateFromInfo('error', lastInfo, humanizeUpdateError(error)) ) @@ -307,6 +442,10 @@ export async function checkForAppUpdates(): Promise { continue } + if (isNetworkUnreachableError(error)) { + waitForNetwork(error) + break + } setUpdateState( nextStateFromInfo('error', lastInfo, humanizeUpdateError(error)) ) @@ -371,6 +510,10 @@ async function checkManagedInstallForUpdates(): Promise { await sleep(UPDATE_CHECK_RETRY_DELAY_MS) continue } + if (isNetworkUnreachableError(error)) { + waitForNetwork(error) + return getAppUpdateState() + } setUpdateState(makeState({ phase: 'error', message: humanizeUpdateError(error) })) return getAppUpdateState() } diff --git a/packages/app-core/src/components/SettingsModal.tsx b/packages/app-core/src/components/SettingsModal.tsx index 80b1e33b..630a3161 100644 --- a/packages/app-core/src/components/SettingsModal.tsx +++ b/packages/app-core/src/components/SettingsModal.tsx @@ -363,6 +363,8 @@ function formatUpdatePhaseLabel(phase: AppUpdateState["phase"]): string { return "Ready to install"; case "installing": return "Installing"; + case "offline": + return "Waiting for network"; case "error": return "Update error"; case "idle": @@ -382,6 +384,8 @@ function updatePhaseBadgeClass(phase: AppUpdateState["phase"]): string { return "border-paper-300/70 bg-paper-100/85 text-ink-700"; case "error": return "border-red-400/25 bg-red-500/10 text-red-700"; + case "offline": + return "border-amber-500/30 bg-amber-500/10 text-amber-700"; case "not-available": return "border-emerald-400/25 bg-emerald-500/10 text-emerald-700"; case "unsupported": @@ -808,7 +812,11 @@ export function SettingsModal(): JSX.Element { window.alert(state.message); return; } - if (state.phase === "unsupported" || state.phase === "error") { + if ( + state.phase === "unsupported" || + state.phase === "offline" || + state.phase === "error" + ) { window.alert(state.message); } }, diff --git a/packages/app-core/src/lib/app-update-state.test.ts b/packages/app-core/src/lib/app-update-state.test.ts index 6d2fb49a..f5e3e1d0 100644 --- a/packages/app-core/src/lib/app-update-state.test.ts +++ b/packages/app-core/src/lib/app-update-state.test.ts @@ -81,4 +81,16 @@ describe('app update state labels', () => { expect(appUpdateNoticeLabel(state)).toBeNull() expect(appUpdatePrimaryActionLabel(state)).toBeNull() }) + + it('stays quiet while waiting for the network: the host retries by itself (#812)', () => { + // Launching offline used to surface as "update needs attention" with a + // Details button, for a check that was going to be retried anyway. + const state = updateState('offline', { + message: "ZenNotes can't reach GitHub right now (net::ERR_INTERNET_DISCONNECTED)." + }) + + expect(appUpdateBadgeLabel(state)).toBeNull() + expect(appUpdateNoticeLabel(state)).toBeNull() + expect(appUpdatePrimaryActionLabel(state)).toBeNull() + }) }) diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index d252aa67..a44c1924 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -159,7 +159,7 @@ export const HELP_HOW_TO_GUIDES: HelpCard[] = [ { title: 'Check for updates and install them', body: - 'Use Check for Updates from the app menu, the command palette, or Settings → About. When a release is available, ZenNotes can download it in the background and then prompt you to install and relaunch. A copy installed by a package manager (the AUR package, or a tarball unpacked by hand) is only told that a newer version exists; install it the way you installed ZenNotes, since the package manager owns those files. On Arch, a `.pacman` build installs through a graphical polkit prompt; dismissing it keeps the download ready to retry, and if no graphical agent can run, Details in Settings → About shows the manual install command.' + 'Use Check for Updates from the app menu, the command palette, or Settings → About. ZenNotes also checks on its own shortly after launch. If that check finds no network (Settings → About then says "Waiting for network"), nothing needs doing: it checks again by itself once the connection is back, and keeps trying at growing intervals if GitHub stays out of reach. When a release is available, ZenNotes can download it in the background and then prompt you to install and relaunch. A copy installed by a package manager (the AUR package, or a tarball unpacked by hand) is only told that a newer version exists; install it the way you installed ZenNotes, since the package manager owns those files. On Arch, a `.pacman` build installs through a graphical polkit prompt; dismissing it keeps the download ready to retry, and if no graphical agent can run, Details in Settings → About shows the manual install command.' }, { title: 'Run the self-hosted web version with Docker', diff --git a/packages/bridge-contract/src/ipc.ts b/packages/bridge-contract/src/ipc.ts index 91716746..69c6749a 100644 --- a/packages/bridge-contract/src/ipc.ts +++ b/packages/bridge-contract/src/ipc.ts @@ -227,6 +227,12 @@ export type AppUpdatePhase = | 'downloading' | 'downloaded' | 'installing' + /** + * The last check could not reach GitHub at all (no network, DNS down, + * connection refused). Not an error the user has to act on: the host keeps + * watching and checks again on its own once the connection is back. + */ + | 'offline' | 'error' export interface CliInstallRequest { From af645e1e11e13d9019775e1f68661efbda041814 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 14:46:01 -0500 Subject: [PATCH 04/12] Fix(cloud): a sync that fails midway keeps what already landed instead of replaying it (#813) Delete Untitled.md on the desktop while the Trash already holds an older Untitled.md and the note is trashed as trash/Untitled 2.md, a move that changes folder and name in one step. Trash is synced, so the phone had the older trash/Untitled.md too, and its Storage Access Framework plugin did such a move as "move the document, then rename it". The move step keeps the source name, so Android refused the intermediate trash/Untitled.md with "rename failed: Already exists". @uNyanda reported the sync stuck there for good, the deleted note coming back on the phone with every Sync now (#813). The plugin's ordering is fixed in the Android repo (rename first, in the source folder, then move). What this commit fixes is why the failure kept coming back. pullChanges saved its cursor only after a whole batch had been applied, but the files a change touched stayed changed when a later change threw. Every retry then replayed the earlier upsert of Untitled.md from the old cursor, re-creating the note the user had just deleted, and failed again on the same move. The loop now keeps a second state, the newest one that can be persisted on its own, and saves it before letting the error escape, so a retry resumes at the change that failed. That state only advances when no coalesced upsert is still waiting for the change that lands it: a skipped revision is reduced into the live state but never written (#661), and persisting past it would make this device vouch for bytes it does not have. The success path still saves the final state once; the conflict branches are untouched. For the reporter this alone turns the loop into a clean retry: delete the note on the phone once more, Sync now, and the move and delete become no-ops without a source. Verified with two coordinator tests: the #813 sequence (an upsert then a move that throws; the cursor lands on 2, not 1, and after the local delete the retry succeeds with cursor 4 and re-creates nothing), and a guard that progress is never persisted past a coalesced revision whose landing change has not run, which fails against a naive save-after-every-change implementation. Not driven against a live phone. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 --- .../src/cloud-sync-coordinator.test.ts | 126 ++++++++++++++++++ .../src/cloud-sync-coordinator.ts | 85 +++++++----- 2 files changed, 176 insertions(+), 35 deletions(-) diff --git a/packages/shared-domain/src/cloud-sync-coordinator.test.ts b/packages/shared-domain/src/cloud-sync-coordinator.test.ts index c833709d..48563c94 100644 --- a/packages/shared-domain/src/cloud-sync-coordinator.test.ts +++ b/packages/shared-domain/src/cloud-sync-coordinator.test.ts @@ -1906,6 +1906,132 @@ describe('CloudSyncCoordinator: catching up on a file this device never touched' expect(server.mutations).toEqual([]) }) + it('keeps the changes that landed before a failing one so a retry does not replay them (#813)', async () => { + // The desktop saved Untitled.md, trashed it as "Untitled 2.md" because an + // older Untitled.md already sat in the trash, then emptied it. On the + // phone the move fails in storage every time. + const fs = memoryFileSystem({ 'Untitled.md': 'v1', 'trash/Untitled.md': 'older' }) + const rename = fs.rename + fs.rename = async (from, to) => { + throw new Error(`rename failed: Already exists /storage/emulated/0/Mind/trash/Untitled.md (${from} -> ${to})`) + } + const states = memoryState({ + version: 1, + vault_id: 'vault-1', + cursor: 1, + items: { + note: tracked('note', 'Untitled.md', 1, 'v1'), + older: tracked('older', 'trash/Untitled.md', 1, 'older') + } + }) + const server = remote({ + changes: [ + upsert(2, 'note', 'Untitled.md', 'v2'), + { + sequence: 3, + item_id: 'note', + type: 'move', + path: 'trash/Untitled 2.md', + previous_path: 'Untitled.md', + revision: 3 + }, + { + sequence: 4, + item_id: 'note', + type: 'delete', + path: 'trash/Untitled 2.md', + previous_path: null, + revision: 4 + } + ] + }) + const coordinator = new CloudSyncCoordinator( + 'vault-1', + server, + new PortableCloudSyncRepository(fs), + states, + ids() + ) + + await expect(coordinator.sync()).rejects.toThrow('Already exists') + // The saved revision is on disk, so the cursor moves past it: only the + // move is still owed. + expect(fs.files.get('Untitled.md')).toBe('v2') + expect(states.current?.cursor).toBe(2) + expect(states.current?.items.note?.sha256).toBe(realContent('v2').sha256) + + // The user deletes the note on the phone to get unstuck. A retry must not + // bring it back by replaying the upsert; without a source the move and + // the delete have nothing left to do. + fs.files.delete('Untitled.md') + fs.rename = rename + const retry = await coordinator.sync() + + expect(retry.localConflicts).toEqual([]) + expect([...fs.files.keys()]).toEqual(['trash/Untitled.md']) + expect(states.current?.cursor).toBe(4) + expect(states.current?.items.note).toBeUndefined() + expect(server.mutations).toEqual([]) + }) + + it('does not persist progress past a coalesced revision whose landing change has not run', async () => { + // Plan.md v2 is skipped in favour of v3, which comes after the failing + // move of another note. Saving the state after v2 would describe the + // server's history rather than this device's file, and the retry would + // park v3 as a conflict copy. + const fs = memoryFileSystem({ [path]: 'v1', 'Other.md': 'other' }) + const rename = fs.rename + let refuse = true + fs.rename = async (from, to) => { + if (refuse) throw new Error('rename failed: Already exists') + return rename(from, to) + } + const states = memoryState({ + version: 1, + vault_id: 'vault-1', + cursor: 1, + items: { + plan: tracked('plan', path, 1, 'v1'), + other: tracked('other', 'Other.md', 1, 'other') + } + }) + const server = remote({ + changes: [ + upsert(2, 'plan', path, 'v2'), + { + sequence: 3, + item_id: 'other', + type: 'move', + path: 'archive/Other.md', + previous_path: 'Other.md', + revision: 3 + }, + upsert(4, 'plan', path, 'v3') + ] + }) + const coordinator = new CloudSyncCoordinator( + 'vault-1', + server, + new PortableCloudSyncRepository(fs), + states, + ids() + ) + + await expect(coordinator.sync()).rejects.toThrow('Already exists') + expect(states.current?.cursor).toBe(1) + expect(fs.files.get(path)).toBe('v1') + + refuse = false + const retry = await coordinator.sync() + + expect(retry.localConflicts).toEqual([]) + expect([...fs.files.keys()].sort()).toEqual(['archive/Other.md', path]) + expect(fs.files.get(path)).toBe('v3') + expect(states.current?.cursor).toBe(4) + expect(states.current?.pending_conflicts ?? {}).toEqual({}) + expect(server.mutations).toEqual([]) + }) + it('queues a real local edit without creating a note beside it', async () => { const fs = memoryFileSystem({ [path]: 'edited here while offline' }) const states = memoryState({ diff --git a/packages/shared-domain/src/cloud-sync-coordinator.ts b/packages/shared-domain/src/cloud-sync-coordinator.ts index 5bb6ac06..39f81d33 100644 --- a/packages/shared-domain/src/cloud-sync-coordinator.ts +++ b/packages/shared-domain/src/cloud-sync-coordinator.ts @@ -567,48 +567,63 @@ export class CloudSyncCoordinator { // just received. Remember what was on disk before the first skipped // revision and give the change that finally lands that instead. const onDisk = new Map() - for (const change of changes) { - const acknowledged = acknowledgedSequences.has(change.sequence) - if (acknowledged) { - // This device's own push: the file already holds these bytes. - onDisk.delete(change.item_id) - } else if (supersededUpserts.has(change.sequence)) { - if (!onDisk.has(change.item_id)) onDisk.set(change.item_id, state.items[change.item_id]) - pulled++ - } else { - const previous = onDisk.has(change.item_id) - ? onDisk.get(change.item_id) - : state.items[change.item_id] - onDisk.delete(change.item_id) - const existingConflict = state.pending_conflicts?.[change.item_id] - if (existingConflict) { - state = { - ...state, - pending_conflicts: { - ...state.pending_conflicts, - [change.item_id]: advancePendingConflict(existingConflict, change) - } - } + // The files a change touched stay changed when a later change in the same + // batch fails, but the cursor used to move only once the whole batch was + // through. Every retry then replayed the applied changes from the old + // cursor: an upsert re-created a note the user had since deleted on this + // device, and the batch failed again on the same change (#813). Remember + // the newest state that can be persisted on its own (no coalesced + // revision still waiting for the change that lands it) and save that + // before the error escapes, so a retry resumes at the failing change. + let landed = initialState + try { + for (const change of changes) { + const acknowledged = acknowledgedSequences.has(change.sequence) + if (acknowledged) { + // This device's own push: the file already holds these bytes. + onDisk.delete(change.item_id) + } else if (supersededUpserts.has(change.sequence)) { + if (!onDisk.has(change.item_id)) onDisk.set(change.item_id, state.items[change.item_id]) + pulled++ } else { - const conflict = await this.repository.apply(change, previous) - if (conflict?.code === 'LOCAL_EDIT_CONFLICT') { - const pending = await this.storedConflict(change, previous, conflict.local ?? null) - if (!(await this.applyAutomaticMerge(pending))) { - state = { - ...state, - pending_conflicts: { - ...state.pending_conflicts, - [pending.id]: pending + const previous = onDisk.has(change.item_id) + ? onDisk.get(change.item_id) + : state.items[change.item_id] + onDisk.delete(change.item_id) + const existingConflict = state.pending_conflicts?.[change.item_id] + if (existingConflict) { + state = { + ...state, + pending_conflicts: { + ...state.pending_conflicts, + [change.item_id]: advancePendingConflict(existingConflict, change) + } + } + } else { + const conflict = await this.repository.apply(change, previous) + if (conflict?.code === 'LOCAL_EDIT_CONFLICT') { + const pending = await this.storedConflict(change, previous, conflict.local ?? null) + if (!(await this.applyAutomaticMerge(pending))) { + state = { + ...state, + pending_conflicts: { + ...state.pending_conflicts, + [pending.id]: pending + } } } + } else if (conflict) { + localConflicts.push(publicLocalConflict(conflict)) } - } else if (conflict) { - localConflicts.push(publicLocalConflict(conflict)) } + pulled++ } - pulled++ + state = reduceCloudSyncChange(state, change) + if (onDisk.size === 0) landed = state } - state = reduceCloudSyncChange(state, change) + } catch (error) { + if (landed !== initialState) await this.states.save(landed) + throw error } if (changes.length > 0) await this.states.save(state) From ab719eab0e26cf3cec7311286a4fdcf503a00c11 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 15:14:11 -0500 Subject: [PATCH 05/12] Fix(daily): a task added to a past daily note after today's was opened still rolls over (#817) Open today's daily note with "Roll over unfinished tasks to today" on and nothing to roll yet, then type a task into yesterday's note and open today's again: the task stayed where it was until the next day. @naingyeminn reported it as the setting doing nothing at all (#817), which is what it looks like when today's note is always opened first. The rollover kept a once-per-day marker in localStorage, written after every run including the ones that found nothing, so the first open of the day locked the feature for the rest of it. The marker existed to avoid re-reading years of daily notes on every open, and that cost is real, so it is not simply dropped. Its replacement is a per-vault record of the past daily notes already read and found free of open tasks, each with the updatedAt:size signature of the listing that was scanned. A note edited since, typed in this app, changed by sync or another editor, stops matching and is read again; a note with an open buffer is always read from the buffer, which costs nothing; a note the rollover just trimmed is read once more before it is trusted, so the trim itself can never hide a task. The command-palette run ignores the record and reads everything, as it ignored the marker before. Today's note now takes the tasks before the sources give them up, so a failure midway leaves a task in two notes rather than in none. extractOpenTaskBlocks also matched on lines split at "\n" alone, so a Windows file's trailing "\r" defeated the "$" in TASK_LINE_RE: its tasks showed in the Tasks view, whose scanner normalizes endings, and never rolled over. Matching now runs on "\r"-stripped copies while the remaining body keeps its endings byte for byte. Whether the reporter's files are CRLF is not known; the failure is silent and belongs to the same feature. Verified with a store test that walks the reported sequence (first open moves nothing, a second unchanged open reads no past note, the task typed in between rolls on the next open, the trimmed note is read once more) and a CRLF case in tasks-rollover.test.ts, both red on the old code, and in the built app over CDP with the reporter's steps. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 --- packages/app-core/src/store.test.ts | 68 +++++++++++ packages/app-core/src/store.ts | 115 ++++++++++++------ packages/shared-domain/src/tasklists.ts | 11 +- .../shared-domain/src/tasks-rollover.test.ts | 8 ++ 4 files changed, 165 insertions(+), 37 deletions(-) diff --git a/packages/app-core/src/store.test.ts b/packages/app-core/src/store.test.ts index e414154d..8ee6506f 100644 --- a/packages/app-core/src/store.test.ts +++ b/packages/app-core/src/store.test.ts @@ -357,6 +357,74 @@ describe('daily task rollover', () => { expect(files.get(sourcePath)).toBe('## Tasks\n\n- [ ]\n- [x] Done\n') expect(await useStore.getState().rolloverUnfinishedTasksIntoToday({ force: true, open })).toBe(0) }) + + // The reported flow (#817): today's note is opened once with nothing to + // roll, then a task is typed into yesterday's note, then today's note is + // opened again. The once-per-day marker used to make the second open a + // no-op until the next day. + it('rolls a task added to a past daily note after today was already opened', async () => { + const iso = (date: Date) => [date.getFullYear(), String(date.getMonth() + 1).padStart(2, '0'), + String(date.getDate()).padStart(2, '0')].join('-') + const now = new Date() + const yesterday = new Date(now) + yesterday.setDate(yesterday.getDate() - 1) + const sourcePath = `inbox/Daily Notes/${iso(yesterday)}.md` + const targetPath = `inbox/Daily Notes/${iso(now)}.md` + const files = new Map([[sourcePath, `# ${iso(yesterday)}\n\n`]]) + const stamps = new Map([[sourcePath, 1]]) + const notes = () => + [...files].map(([path, body]) => ({ ...makeNote(body, path), updatedAt: stamps.get(path) ?? 1 })) + const readNote = vi.fn(async (path: string) => makeNote(files.get(path)!, path)) + installZen({ + createNote: vi.fn(async () => { + files.set(targetPath, `# ${iso(now)}\n\n`) + return makeNote(files.get(targetPath)!, targetPath) + }), + listNotes: vi.fn(async () => notes()), + readNote, + writeNote: vi.fn(async (path: string, body: string) => { + files.set(path, body) + return makeNote(body, path) + }) + }) + const { useStore } = await loadStore() + useStore.setState({ + notes: notes(), + vaultSettings: { + ...useStore.getState().vaultSettings, + dailyNotes: { enabled: true, directory: 'Daily Notes', rolloverUnfinishedTasks: true } + } + }) + + expect(await useStore.getState().rolloverUnfinishedTasksIntoToday()).toBe(0) + const readsAfterFirstRun = readNote.mock.calls.length + expect(readNote).toHaveBeenCalledWith(sourcePath) + + // Opening today again with nothing changed must not re-read the past + // note the record already vouches for. + useStore.setState({ notes: notes() }) + expect(await useStore.getState().rolloverUnfinishedTasksIntoToday()).toBe(0) + expect(readNote.mock.calls.length).toBe(readsAfterFirstRun) + + // The user types a task into yesterday's note; the autosave lands on disk + // and the watcher re-lists the vault with a fresh mtime and size. + files.set(sourcePath, `# ${iso(yesterday)}\n\n- [ ] Call the bank\n`) + stamps.set(sourcePath, 2) + useStore.setState({ notes: notes() }) + + expect(await useStore.getState().rolloverUnfinishedTasksIntoToday()).toBe(1) + expect(files.get(targetPath)).toBe(`# ${iso(now)}\n- [ ] Call the bank\n`) + expect(files.get(sourcePath)).toBe(`# ${iso(yesterday)}\n\n`) + + // A note the rollover just trimmed is read once more before it is + // trusted again, so the trim itself can never hide a task. + const readsAfterMove = readNote.mock.calls.length + stamps.set(sourcePath, 3) + useStore.setState({ notes: notes() }) + expect(await useStore.getState().rolloverUnfinishedTasksIntoToday()).toBe(0) + expect(readNote.mock.calls.length).toBe(readsAfterMove + 1) + expect(readNote).toHaveBeenLastCalledWith(sourcePath) + }) }) describe('weekly note patterns', () => { diff --git a/packages/app-core/src/store.ts b/packages/app-core/src/store.ts index 70157cb2..e8f15b0f 100644 --- a/packages/app-core/src/store.ts +++ b/packages/app-core/src/store.ts @@ -1825,25 +1825,46 @@ function parseIsoDateLocal(iso: string): Date | null { return Number.isNaN(d.getTime()) ? null : d } -// Per-vault "we already rolled over today" marker, persisted in localStorage so -// opening today's daily note across sessions doesn't re-scan past notes once -// it's done for the day. Keyed by vault root so multiple vaults don't collide. -function rolloverMarkerKey(root: string): string { - return `zen.tasks.rollover.${root || 'default'}` -} -function readRolloverMarker(root: string): string | null { +// Per-vault record of the past daily notes the rollover already read and found +// free of open tasks, persisted in localStorage so opening today's note does +// not re-read years of daily notes every time. Each entry is keyed by note +// path and holds the `updatedAt:size` signature of the listing that was +// scanned, so a note edited since (a task typed into yesterday's note later +// today, a file changed by sync) stops matching and is read again. Its +// predecessor was a once-per-day marker written even when nothing had moved, +// which meant a task added to a past daily note after today's note had been +// opened once never rolled over until the next day (#817). Keyed by vault +// root so multiple vaults don't collide. +type RolloverCleanRecord = Record + +function rolloverCleanKey(root: string): string { + return `zen.tasks.rolloverClean.${root || 'default'}` +} +function rolloverNoteSignature(note: NoteMeta): string { + return `${note.updatedAt}:${note.size}` +} +function readRolloverClean(root: string): RolloverCleanRecord { try { - return typeof localStorage !== 'undefined' - ? localStorage.getItem(rolloverMarkerKey(root)) - : null + const raw = + typeof localStorage !== 'undefined' ? localStorage.getItem(rolloverCleanKey(root)) : null + if (!raw) return {} + const parsed: unknown = JSON.parse(raw) + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return {} + const record: RolloverCleanRecord = {} + for (const [path, signature] of Object.entries(parsed)) { + if (typeof signature === 'string') record[path] = signature + } + return record } catch { - return null + return {} } } -function writeRolloverMarker(root: string, iso: string): void { +function writeRolloverClean(root: string, record: RolloverCleanRecord): void { try { if (typeof localStorage !== 'undefined') { - localStorage.setItem(rolloverMarkerKey(root), iso) + localStorage.setItem(rolloverCleanKey(root), JSON.stringify(record)) + // The once-per-day marker this record replaces; nothing reads it any more. + localStorage.removeItem(`zen.tasks.rollover.${root || 'default'}`) } } catch { // localStorage may be unavailable (private mode); the in-session flow still works. @@ -3634,7 +3655,8 @@ interface Store { addTaskForDate: (dateIso: string, text: string) => Promise /** Move unfinished tasks from past daily notes into today's note. Returns the * number of task lines moved. Without `force`, it is gated by the - * `rolloverUnfinishedTasks` setting and a once-per-day marker. */ + * `rolloverUnfinishedTasks` setting and skips past notes it already found + * clean and that have not changed on disk since; `force` re-reads them all. */ rolloverUnfinishedTasksIntoToday: (opts?: { force?: boolean open?: boolean @@ -9117,10 +9139,7 @@ export const useStore = create((set, get) => { const today = new Date() const todayIso = noteTitleForDate(today) const vaultRoot = get().vault?.root ?? '' - if (!force) { - if (!settings.dailyNotes.rolloverUnfinishedTasks) return 0 - if (readRolloverMarker(vaultRoot) === todayIso) return 0 - } + if (!force && !settings.dailyNotes.rolloverUnfinishedTasks) return 0 const todayNote = await get().ensureDailyNoteForDate(today) if (!todayNote) return 0 if (opts?.open) { @@ -9140,9 +9159,20 @@ export const useStore = create((set, get) => { } pastNotes.sort((a, b) => (a.iso < b.iso ? -1 : a.iso > b.iso ? 1 : 0)) + // The explicit command is the escape hatch: it re-reads every past note + // instead of trusting the record. Notes that vanished from the listing + // drop out of the record because only notes seen this run are carried. + const clean = force ? {} : readRolloverClean(vaultRoot) + const nextClean: RolloverCleanRecord = {} const movedLines: string[] = [] + const trimmed: Array<{ path: string; rest: string; buffered: boolean }> = [] for (const { note } of pastNotes) { + const signature = rolloverNoteSignature(note) const buffer = get().noteContents[note.path] + if (!buffer && clean[note.path] === signature) { + nextClean[note.path] = signature + continue + } let body: string try { body = buffer?.body ?? (await window.zen.readNote(note.path)).body @@ -9151,29 +9181,24 @@ export const useStore = create((set, get) => { continue } const { moved, rest } = extractOpenTaskBlocks(body) - if (moved.length === 0) continue - movedLines.push(...moved) - if (buffer) { - // Open buffer: route through the normal edit pipeline (marks dirty, - // autosaves, watcher rescans tasks) — same as toggleTaskFromList. A disk - // rescan here would read the not-yet-flushed file and go stale. - get().updateNoteBody(note.path, rest) - } else { - try { - await window.zen.writeNote(note.path, rest) - await get().rescanTasksForPath(note.path) - } catch (err) { - console.error('rollover writeNote (source) failed', note.path, err) - // Don't drop the lines we already pulled — they'll still land in today. - } + if (moved.length === 0) { + // The signature describes the file on disk, so only a disk read may + // vouch for it: an open buffer can be ahead of the listing, and it + // costs nothing to read again. + if (!buffer) nextClean[note.path] = signature + continue } + movedLines.push(...moved) + trimmed.push({ path: note.path, rest, buffered: Boolean(buffer) }) } if (movedLines.length === 0) { - writeRolloverMarker(vaultRoot, todayIso) + writeRolloverClean(vaultRoot, nextClean) return 0 } + // Today's note takes the tasks first and the sources give them up after, + // so a failure midway leaves a task in two notes rather than in none. const todayBuffer = get().noteContents[todayNote.path] let todayBody: string try { @@ -9198,7 +9223,27 @@ export const useStore = create((set, get) => { return 0 } } - writeRolloverMarker(vaultRoot, todayIso) + + for (const { path, rest, buffered } of trimmed) { + if (buffered) { + // Open buffer: route through the normal edit pipeline (marks dirty, + // autosaves, watcher rescans tasks), same as toggleTaskFromList. A disk + // rescan here would read the not-yet-flushed file and go stale. + get().updateNoteBody(path, rest) + } else { + try { + await window.zen.writeNote(path, rest) + await get().rescanTasksForPath(path) + } catch (err) { + console.error('rollover writeNote (source) failed', path, err) + // The task already landed in today; the copy left here rolls again + // next time and the user sees a duplicate, not a lost task. + } + } + } + // Trimmed notes are left out on purpose: their signature changes with the + // write, and the next run reads them once more before vouching for them. + writeRolloverClean(vaultRoot, nextClean) return movedLines.length }), diff --git a/packages/shared-domain/src/tasklists.ts b/packages/shared-domain/src/tasklists.ts index 5e10f21b..3bc9c381 100644 --- a/packages/shared-domain/src/tasklists.ts +++ b/packages/shared-domain/src/tasklists.ts @@ -505,7 +505,14 @@ export function extractOpenTaskBlocks(markdown: string): { moved: string[] rest: string } { - const lines = markdown.split('\n') + // Windows files end lines with `\r\n`, and a `\r` left on the line defeats + // the `$` in TASK_LINE_RE: such a note shows its tasks in the Tasks view + // (the scanner normalizes) but never rolled them over. Matching runs on + // `\r`-stripped copies; `rest` is rebuilt from the original lines so the + // note keeps its endings byte for byte, and the moved lines are the bare + // ones, since today's note is joined on `\n`. + const rawLines = markdown.split('\n') + const lines = rawLines.map((line) => (line.endsWith('\r') ? line.slice(0, -1) : line)) const consumed = new Array(lines.length).fill(false) const moved: string[] = [] let inFence = false @@ -547,7 +554,7 @@ export function extractOpenTaskBlocks(markdown: string): { i = blockEnd - 1 // skip the consumed block (its children are not new tasks) } - const rest = lines.filter((_, idx) => !consumed[idx]).join('\n') + const rest = rawLines.filter((_, idx) => !consumed[idx]).join('\n') return { moved, rest } } diff --git a/packages/shared-domain/src/tasks-rollover.test.ts b/packages/shared-domain/src/tasks-rollover.test.ts index 615198ed..a30498f3 100644 --- a/packages/shared-domain/src/tasks-rollover.test.ts +++ b/packages/shared-domain/src/tasks-rollover.test.ts @@ -10,6 +10,14 @@ describe('daily task rollover placeholders', () => { }) }) + it('rolls tasks out of a CRLF note and keeps its line endings (#817)', () => { + const body = '## Tasks\r\n\r\n- [ ] Call the bank\r\n - [ ] Ask about the fee\r\n- [x] Done\r\n- [ ]\r\n' + expect(extractOpenTaskBlocks(body)).toEqual({ + moved: ['- [ ] Call the bank', ' - [ ] Ask about the fee'], + rest: '## Tasks\r\n\r\n- [x] Done\r\n- [ ]\r\n' + }) + }) + it('keeps an empty parent with meaningful indented children', () => { const body = '- [ ]\n - [ ] Child task\n\n- [ ]\n Details to carry\n' expect(extractOpenTaskBlocks(body).moved).toEqual([ From 5ccc3507447a92a3e4b474f2da0cf864786d1bce Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 15:52:31 -0500 Subject: [PATCH 06/12] Feat(vim): `:version` prints what a bug report needs (#814) In Vim mode, `:version` printed "Codemirror-vim version: 6.3.0" in red and nothing else. That is the stock ex command of the Vim library ZenNotes embeds, and it answers a question nobody asks: the library's version is fixed by the ZenNotes build, and the line says nothing about the build. @vlcinsky asked for the ZenNotes version and the details a bug report needs (#814). `:version` (and `:ve`) is now ZenNotes' own. It prints the ZenNotes version and host kind, the operating system and architecture, the engine (Electron with its Chromium and Node, or the browser's user agent on the web client), how this copy was installed (macOS app bundle, Mac App Store, NSIS installer, portable exe, AppImage, deb, rpm, pacman, or a package manager or tarball install), and, when the window is connected to a remote workspace, the server's version and address. `:version copy` or `:version!` also puts the lines on the clipboard. The text is shown in the editor's own color for 15 seconds: red is what codemirror-vim uses for errors, and this is not one. Settings > About shows the same lines under the version, as a Version details block with a Copy details button, so a user who does not use Vim mode has the same report one click away. One function, lib/version-report.ts, builds the lines for both surfaces, and leaves out any field the host did not fill in instead of printing "unknown". The OS version and install format only main knows (os-release, resourcesPath, app.isPackaged), so the preload asks for them over one synchronous IPC, cached after the first getAppInfo() call, since that call runs many times at boot for its runtime field alone. The Linux install label reuses the updater's own format detection, so what `:version` prints is the format the updater acts on. Nothing in describeInstall() may throw: the preload asks while the window boots. The codemirror-vim version line is gone on purpose: the library does not expose it at runtime, and the ZenNotes version pins it anyway. Verified with unit tests for installLabel and osReleasePrettyName (desktop updater.test.ts) and buildVersionReport (app-core), and in the built app over CDP: `:version` prints the four lines, `:version copy` puts them on the clipboard, `:ve` resolves to this command, and the About block matches the ex output byte for byte. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 --- apps/desktop/src/main/index.ts | 11 +++ apps/desktop/src/main/updater.test.ts | 69 ++++++++++++++ apps/desktop/src/main/updater.ts | 95 +++++++++++++++++++ apps/desktop/src/preload/index.ts | 24 ++++- apps/web/src/bridge/http-bridge.ts | 7 +- packages/app-core/src/components/Editor.tsx | 62 +++++++++++- .../app-core/src/components/SettingsModal.tsx | 69 +++++++++++++- packages/app-core/src/lib/help.ts | 6 ++ .../app-core/src/lib/version-report.test.ts | 74 +++++++++++++++ packages/app-core/src/lib/version-report.ts | 31 ++++++ packages/bridge-contract/src/bridge.ts | 10 ++ packages/bridge-contract/src/ipc.ts | 1 + 12 files changed, 451 insertions(+), 8 deletions(-) create mode 100644 packages/app-core/src/lib/version-report.test.ts create mode 100644 packages/app-core/src/lib/version-report.ts diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 127fad70..323d50ba 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -242,6 +242,7 @@ import { } from "./raycast-integration"; import { checkForAppUpdates, + describeInstall, downloadAppUpdate, getAppUpdateState, initAppUpdater, @@ -4608,6 +4609,16 @@ function registerIpc(): void { event.returnValue = null; } }); + // Same shape as CONFIG_GET_SYNC: the preload folds the OS and install + // format into getAppInfo(), which renderer code calls synchronously (#814). + ipcMain.on(IPC.APP_INSTALL_INFO_SYNC, (event) => { + try { + assertTrustedIpcEvent(event); + event.returnValue = describeInstall(); + } catch { + event.returnValue = null; + } + }); handle(IPC.CONFIG_SET, async (_event, next: AppConfigPortable) => { const previousTitleBar = getPortableConfigSnapshot().showWindowTitleBar !== false; diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts index 285cd1bf..0b66c303 100644 --- a/apps/desktop/src/main/updater.test.ts +++ b/apps/desktop/src/main/updater.test.ts @@ -27,6 +27,7 @@ import FpmTarget from 'app-builder-lib/out/targets/FpmTarget' import electronUpdater from 'electron-updater' import { elevatedInstallScript, + installLabel, installedLinuxFormat, isNetworkUnreachableError, isOfficialLinuxSystemPackage, @@ -39,11 +40,79 @@ import { manualInstallHint, mismatchedUpdateMessage, offlineRetryDelayMs, + osReleasePrettyName, OFFLINE_POLL_MS, OFFLINE_RETRY_BASE_MS, OFFLINE_RETRY_MAX_MS } from './updater' +describe('installLabel (:version, issue #814)', () => { + const packaged = { + isPackaged: true, + mas: false, + windowsStore: false, + portableExecutableDir: undefined, + linuxFormat: (): 'unknown' => 'unknown' + } + + it('calls an unpackaged checkout a development build on every platform', () => { + for (const platform of ['darwin', 'win32', 'linux'] as const) { + expect(installLabel({ ...packaged, platform, isPackaged: false })).toBe('development build') + } + }) + + it('tells the Mac App Store copy apart from the dmg one', () => { + expect(installLabel({ ...packaged, platform: 'darwin' })).toBe('macOS app bundle') + expect(installLabel({ ...packaged, platform: 'darwin', mas: true })).toBe('Mac App Store') + }) + + it('tells the portable exe apart from the NSIS install and the Store', () => { + expect(installLabel({ ...packaged, platform: 'win32' })).toBe('NSIS installer') + expect( + installLabel({ ...packaged, platform: 'win32', portableExecutableDir: 'D:\\apps' }) + ).toBe('portable exe') + expect(installLabel({ ...packaged, platform: 'win32', windowsStore: true })).toBe( + 'Microsoft Store' + ) + }) + + it('names the Linux format the updater itself detected', () => { + const linux = (format: ReturnType) => + installLabel({ ...packaged, platform: 'linux', linuxFormat: () => format }) + expect(linux('appimage')).toBe('AppImage') + expect(linux('deb')).toBe('deb package') + expect(linux('rpm')).toBe('rpm package') + expect(linux('pacman')).toBe('pacman package') + expect(linux('managed')).toBe('package manager or tarball (updates are reported, not installed)') + expect(linux('unknown')).toBe('Linux package (format unknown)') + }) + + it('does not consult the Linux detector off Linux', () => { + const linuxFormat = vi.fn((): 'deb' => 'deb') + installLabel({ ...packaged, platform: 'darwin', linuxFormat }) + installLabel({ ...packaged, platform: 'linux', isPackaged: false, linuxFormat }) + expect(linuxFormat).not.toHaveBeenCalled() + }) +}) + +describe('osReleasePrettyName', () => { + it('reads PRETTY_NAME and strips its quotes', () => { + expect( + osReleasePrettyName('NAME="Ubuntu"\nPRETTY_NAME="Ubuntu 24.04.1 LTS"\nID=ubuntu\n') + ).toBe('Ubuntu 24.04.1 LTS') + expect(osReleasePrettyName("PRETTY_NAME='Arch Linux'")).toBe('Arch Linux') + expect(osReleasePrettyName('PRETTY_NAME=Fedora Linux 40 (Workstation Edition)')).toBe( + 'Fedora Linux 40 (Workstation Edition)' + ) + }) + + it('ignores NAME and an empty PRETTY_NAME, and answers null without a file', () => { + expect(osReleasePrettyName('NAME="Debian GNU/Linux"\nVERSION_ID="12"')).toBeNull() + expect(osReleasePrettyName('PRETTY_NAME=""')).toBeNull() + expect(osReleasePrettyName(null)).toBeNull() + }) +}) + describe('linuxPackageFormat', () => { it('detects each packaged Linux format', () => { expect(linuxPackageFormat('/tmp/ZenNotes-2.0.5.AppImage')).toBe('appimage') diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index 7bb30bdf..530fbf6c 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -793,6 +793,101 @@ function readOsReleaseOrNull(): string | null { } } +/** How this copy was installed, in the words a bug report wants (#814). The + * Linux answer reuses the detection the updater itself relies on, so what + * `:version` prints is the format the updater will act on. */ +export function installLabel(input: { + isPackaged: boolean + platform: NodeJS.Platform + mas: boolean + windowsStore: boolean + portableExecutableDir: string | undefined + linuxFormat: () => LinuxPackageFormat +}): string { + if (!input.isPackaged) return 'development build' + switch (input.platform) { + case 'darwin': + return input.mas ? 'Mac App Store' : 'macOS app bundle' + case 'win32': + if (input.windowsStore) return 'Microsoft Store' + return input.portableExecutableDir ? 'portable exe' : 'NSIS installer' + case 'linux': + switch (input.linuxFormat()) { + case 'appimage': + return 'AppImage' + case 'deb': + return 'deb package' + case 'rpm': + return 'rpm package' + case 'pacman': + return 'pacman package' + case 'managed': + return 'package manager or tarball (updates are reported, not installed)' + default: + return 'Linux package (format unknown)' + } + default: + return input.platform + } +} + +/** PRETTY_NAME from os-release, e.g. `Ubuntu 24.04.1 LTS`, or null. */ +export function osReleasePrettyName(osRelease: string | null): string | null { + if (!osRelease) return null + for (const line of osRelease.split('\n')) { + const match = /^\s*PRETTY_NAME\s*=\s*(.*)$/.exec(line) + if (match) { + const value = match[1].trim().replace(/^["']|["']$/g, '') + if (value) return value + } + } + return null +} + +/** Operating system and install format for the app info the preload hands + * to the renderer (#814). Reads os-release once; nothing here may throw, + * because the preload asks for it synchronously while the window boots. */ +export function describeInstall(): { os: string; install: string } { + const systemVersion = (() => { + try { + return process.getSystemVersion() + } catch { + return '' + } + })() + const osRelease = process.platform === 'linux' ? readOsReleaseOrNull() : null + const os = + process.platform === 'darwin' + ? `macOS ${systemVersion}`.trim() + : process.platform === 'win32' + ? `Windows ${systemVersion}`.trim() + : process.platform === 'linux' + ? `${osReleasePrettyName(osRelease) ?? 'Linux'} (kernel ${systemVersion || 'unknown'})` + : `${process.platform} ${systemVersion}`.trim() + let install: string + try { + install = installLabel({ + isPackaged: app.isPackaged, + platform: process.platform, + mas: Boolean(process.mas), + windowsStore: Boolean(process.windowsStore), + portableExecutableDir: process.env.PORTABLE_EXECUTABLE_DIR, + linuxFormat: () => + linuxUpdaterFormat({ + isAppImage: Boolean(process.env.APPIMAGE), + isOfficialSystemPackage: isOfficialLinuxSystemPackage( + process.resourcesPath, + existsSync(join(process.resourcesPath, 'package-type')) + ), + osRelease + }) + }) + } catch { + install = 'unknown' + } + return { os, install } +} + function shellQuote(value: string): string { return `'${value.replace(/'/g, `'\\''`)}'` } diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index 36850542..42f7b6c3 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -126,7 +126,27 @@ const DESKTOP_APP_INFO: ZenAppInfo = { description: appPackage.description, homepage: appPackage.homepage, runtime: 'desktop', - hostKind: 'desktop' + hostKind: 'desktop', + arch: process.arch, + engine: `Electron ${process.versions.electron} (Chromium ${process.versions.chrome}, Node ${process.versions.node})` +} + +// The OS version and install format live in main (os-release, resourcesPath, +// app.isPackaged). Fetched once, on the first getAppInfo() call, so the many +// `getAppInfo().runtime` checks at boot do not each pay for a sync IPC (#814). +let installInfo: Pick | null = null +function getInstallInfo(): Pick { + if (installInfo) return installInfo + try { + const result = ipcRenderer.sendSync(IPC.APP_INSTALL_INFO_SYNC) as { + os: string + install: string + } | null + installInfo = result ? { os: result.os, install: result.install } : {} + } catch { + installInfo = {} + } + return installInfo } let remoteWorkspaceInfo: RemoteWorkspaceInfo | null = null @@ -214,7 +234,7 @@ function remoteAssetUrl(assetPath: string): string | null { const api: ZenBridge = { getCapabilities: (): ZenCapabilities => DESKTOP_CAPABILITIES, - getAppInfo: (): ZenAppInfo => DESKTOP_APP_INFO, + getAppInfo: (): ZenAppInfo => ({ ...DESKTOP_APP_INFO, ...getInstallInfo() }), platform: (): Promise => ipcRenderer.invoke(IPC.APP_PLATFORM), platformSync: (): NodeJS.Platform => process.platform, listSystemFonts: (): Promise => ipcRenderer.invoke(IPC.APP_LIST_FONTS), diff --git a/apps/web/src/bridge/http-bridge.ts b/apps/web/src/bridge/http-bridge.ts index 3381fc18..72781907 100644 --- a/apps/web/src/bridge/http-bridge.ts +++ b/apps/web/src/bridge/http-bridge.ts @@ -107,7 +107,12 @@ const WEB_APP_INFO: ZenAppInfo = { description: appPackage.description, homepage: appPackage.homepage, runtime: 'web', - hostKind: 'browser' + hostKind: 'browser', + // The user agent is the one line a browser can answer for `:version`; it + // carries the OS too, so no separate os field (#814). + ...(typeof navigator !== 'undefined' && navigator.userAgent + ? { engine: navigator.userAgent } + : {}) } // Base path under which the server is mounted (e.g. "/zennotes" when diff --git a/packages/app-core/src/components/Editor.tsx b/packages/app-core/src/components/Editor.tsx index 4e6ac8d7..107ff572 100644 --- a/packages/app-core/src/components/Editor.tsx +++ b/packages/app-core/src/components/Editor.tsx @@ -54,6 +54,8 @@ import { import { promptApp } from "../lib/prompt-requests"; import { offerCreateNoteFromLink } from "../lib/create-note-from-link"; import { openWikilinkAttachment } from "../lib/open-wikilink-attachment"; +import { buildVersionReport } from "../lib/version-report"; +import { writeClipboardText } from "../lib/clipboard-text"; import { externalFileLink, openExternalFileLink, @@ -359,6 +361,20 @@ function syncVimKeymaps(overrides: KeymapOverrides): void { * unavailable. (#173) */ function alertEditorError(message: string): void { + showEditorNotification(message, { color: "red", duration: 4000 }); +} + +/** + * Bottom-of-editor Vim notification. Errors are red like codemirror-vim's + * own; informational output (`:version`) inherits the editor color, since + * red would read as "something failed". Multi-line text keeps its line + * breaks. Falls back to an alert (then refocuses) if the editor notification + * is unavailable. + */ +function showEditorNotification( + message: string, + opts: { color?: string; duration: number }, +): void { const view = useStore.getState().editorViewRef; const cm = view ? getCM(view) : null; const openNotification = ( @@ -372,16 +388,48 @@ function alertEditorError(message: string): void { if (cm && typeof openNotification === "function") { const el = document.createElement("div"); el.className = "cm-vim-message"; - el.style.color = "red"; + if (opts.color) el.style.color = opts.color; el.style.whiteSpace = "pre"; el.textContent = message; - openNotification.call(cm, el, { bottom: true, duration: 4000 }); + openNotification.call(cm, el, { bottom: true, duration: opts.duration }); return; } window.alert(message); focusEditorNormalMode(); } +/** + * `:version` prints the details a bug report needs (ZenNotes version, OS, + * engine, install format, remote server) instead of the stock + * codemirror-vim line that only named the Vim library (#814). `:version copy` + * or `:version!` also puts the text on the clipboard. + */ +function runVersionEx(argString: string): void { + const state = useStore.getState(); + const remote = + state.workspaceMode === "remote" + ? { + baseUrl: state.remoteWorkspaceInfo?.baseUrl ?? null, + version: state.remoteWorkspaceInfo?.capabilities?.version ?? null, + } + : null; + const lines = buildVersionReport({ + app: window.zen.getAppInfo(), + remoteServer: remote, + }); + const arg = argString.trim(); + const copy = arg === "!" || arg.toLowerCase() === "copy"; + const shown = [...lines]; + if (copy) { + shown.push( + writeClipboardText(lines.join("\n")) + ? "Copied to the clipboard" + : "Could not reach the clipboard", + ); + } + showEditorNotification(shown.join("\n"), { duration: 15000 }); +} + // Minimal shape of the CodeMirror-Vim adapter + state the display-line motion // touches (the package's own types don't surface these helpers). // The j/k display-line motion (#290) now lives in lib/cm-vim-display-line.ts, @@ -726,6 +774,16 @@ function registerVimCommands(): void { Vim.defineEx("template", "template", runTemplateEx); Vim.defineEx("tmpl", "tmpl", runTemplateEx); + // Replaces the library's own `:version`, which reported only the + // codemirror-vim version (#814). + Vim.defineEx( + "version", + "ve", + (_cm: unknown, params: { argString?: string } | undefined) => { + runVersionEx(params?.argString ?? ""); + }, + ); + Vim.defineEx("daily", "daily", () => { void useStore.getState().openTodayDailyNote(); }); diff --git a/packages/app-core/src/components/SettingsModal.tsx b/packages/app-core/src/components/SettingsModal.tsx index 630a3161..4f2f5477 100644 --- a/packages/app-core/src/components/SettingsModal.tsx +++ b/packages/app-core/src/components/SettingsModal.tsx @@ -125,7 +125,12 @@ import { type SettingsSearchCategory, } from "../lib/settings-search"; import { useAppUpdateState } from "../lib/app-update-state"; -import { getZenBridge } from "@zennotes/bridge-contract/bridge"; +import { buildVersionReport } from "../lib/version-report"; +import { writeClipboardText } from "../lib/clipboard-text"; +import { + getZenBridge, + type ZenAppInfo, +} from "@zennotes/bridge-contract/bridge"; import companyLogo from "../assets/lumary-labs-logo.svg"; import { confirmApp } from "../lib/confirm-requests"; import { promptApp } from "../lib/prompt-requests"; @@ -5079,8 +5084,18 @@ export function SettingsModal(): JSX.Element { { id: "zen-notes-version", title: "ZenNotes version", - description: "App identity, current version, and product details.", - keywords: ["about", "version", "identity"], + description: + "App identity, current version, and the details to paste into a bug report.", + keywords: [ + "about", + "version", + "identity", + "bug report", + "electron", + "os", + "install", + "copy details", + ], }, { id: "updates", @@ -5123,6 +5138,7 @@ export function SettingsModal(): JSX.Element { v{appInfo.version} +
s.workspaceMode); + const remoteWorkspaceInfo = useStore((s) => s.remoteWorkspaceInfo); + const [copied, setCopied] = useState(false); + const lines = buildVersionReport({ + app: appInfo, + remoteServer: + workspaceMode === "remote" + ? { + baseUrl: remoteWorkspaceInfo?.baseUrl ?? null, + version: remoteWorkspaceInfo?.capabilities?.version ?? null, + } + : null, + }); + const text = lines.join("\n"); + useEffect(() => { + if (!copied) return; + const timer = window.setTimeout(() => setCopied(false), 1500); + return () => window.clearTimeout(timer); + }, [copied]); + return ( +
+
+
+ Version details +
+ +
+
+        {text}
+      
+

+ Paste these into a bug report. In Vim mode, :version{" "} + prints the same lines and :version copy copies them. +

+
+ ); +} + /** Desktop-only: surfaces the on-disk config file so users can find, copy, or * open the plain-text TOML they sync across machines (issue #203). */ function ConfigFileSection(): JSX.Element { diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index a44c1924..144046bb 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -802,6 +802,11 @@ export const HELP_VIM_COMMANDS: HelpExCommand[] = [ summary: 'New note from a template', detail: 'Open the template picker. With an argument like `:template ADR` it skips the picker and creates from the best-matching template directly.' }, + { + command: ':version / :ve', + summary: 'Show version details for a bug report', + detail: 'Print the ZenNotes version, operating system and architecture, the Electron or browser engine, how this copy was installed (AppImage, deb, package manager, macOS app bundle, and so on), and the remote server version when connected to one. `:version copy` (or `:version!`) also puts the lines on the clipboard. The same details, with a Copy button, sit in Settings → About.' + }, { command: ':daily', summary: "Open today's daily note", @@ -1175,6 +1180,7 @@ export const HELP_SETTINGS: HelpSettingsSection[] = [ title: 'About', items: [ { label: 'App identity', detail: 'See the ZenNotes app icon, current version, and a short description of the app as a keyboard-first markdown workflow with Vim motions and plain local files.' }, + { label: 'Version details', detail: 'Under the version, a Version details block lists what a bug report needs: operating system and architecture, the Electron or browser engine, how this copy was installed, and the remote server version when connected to one. Copy details puts the block on the clipboard. In Vim mode, `:version` prints the same lines and `:version copy` copies them.' }, { label: 'Updates and releases', detail: 'Check for updates, download a newer build, install and relaunch, or jump straight to the latest GitHub release from inside the app. AUR and tarball installs get the check and the notice only; the package manager does the install.' }, { label: 'Website, community, and issue links', detail: 'The app now exposes direct links to the ZenNotes website, Discord, GitHub repository, and issue tracker so support paths stay discoverable.' }, { label: 'Configuration file', detail: 'Your preferences (theme, editor, Vim, keymaps, fonts, search backend, and more) are mirrored to a plain-text `config.toml` so you can sync them across machines with git, stow, or chezmoi. It lives at `$XDG_CONFIG_HOME/zennotes/config.toml` (`~/.config/zennotes/config.toml` on macOS and Linux, `%APPDATA%\\zennotes\\config.toml` on Windows), or wherever `$ZENNOTES_CONFIG_DIR` points. The file is self-documenting: every available setting is listed with its allowed values, and every keymap action is listed with its default binding (commented out: uncomment a line and edit it to remap, or set it to `""` to remove the key entirely), so you can discover and change anything without opening the app. Settings → About has Reveal and Copy-path buttons. Existing setups are written out automatically the first time you launch this version, and edits to the file, by hand or via a synced dotfile, apply live without a restart. Machine-specific layout (window size, pane widths, collapsed folders) stays local so the file does not churn.' }, diff --git a/packages/app-core/src/lib/version-report.test.ts b/packages/app-core/src/lib/version-report.test.ts new file mode 100644 index 00000000..7c109b94 --- /dev/null +++ b/packages/app-core/src/lib/version-report.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest' +import type { ZenAppInfo } from '@bridge-contract/bridge' +import { buildVersionReport } from './version-report' + +const DESKTOP: ZenAppInfo = { + name: 'zennotes', + productName: 'ZenNotes', + version: '2.53.0', + description: 'notes', + homepage: 'https://zennotes.org', + runtime: 'desktop', + hostKind: 'desktop', + arch: 'x64', + os: 'Ubuntu 24.04.1 LTS (kernel 6.8.0-45-generic)', + engine: 'Electron 38.1.0 (Chromium 140.0.7339.133, Node 22.19.0)', + install: 'AppImage' +} + +describe('buildVersionReport', () => { + it('prints every line a desktop bug report needs, in a fixed order', () => { + expect(buildVersionReport({ app: DESKTOP })).toEqual([ + 'ZenNotes 2.53.0 (desktop)', + 'OS: Ubuntu 24.04.1 LTS (kernel 6.8.0-45-generic), x64', + 'Engine: Electron 38.1.0 (Chromium 140.0.7339.133, Node 22.19.0)', + 'Install: AppImage' + ]) + }) + + it('omits the lines a host could not fill in instead of printing unknowns', () => { + const web: ZenAppInfo = { + name: 'zennotes', + productName: 'ZenNotes', + version: '2.53.0', + description: 'notes', + homepage: 'https://zennotes.org', + runtime: 'web', + hostKind: 'browser', + engine: 'Mozilla/5.0 (X11; Linux x86_64) Firefox/131.0' + } + expect(buildVersionReport({ app: web })).toEqual([ + 'ZenNotes 2.53.0 (browser)', + 'Engine: Mozilla/5.0 (X11; Linux x86_64) Firefox/131.0' + ]) + }) + + it('keeps the arch when the OS name is missing, and vice versa', () => { + expect(buildVersionReport({ app: { ...DESKTOP, os: undefined } })[1]).toBe('OS: x64') + expect(buildVersionReport({ app: { ...DESKTOP, arch: undefined } })[1]).toBe( + 'OS: Ubuntu 24.04.1 LTS (kernel 6.8.0-45-generic)' + ) + expect(buildVersionReport({ app: { ...DESKTOP, os: undefined, arch: undefined } })).not.toContain( + expect.stringMatching(/^OS:/) + ) + }) + + it('falls back to the legacy runtime when hostKind is absent', () => { + expect(buildVersionReport({ app: { ...DESKTOP, hostKind: undefined } })[0]).toBe( + 'ZenNotes 2.53.0 (desktop)' + ) + }) + + it('adds the server line only for a remote workspace', () => { + expect(buildVersionReport({ app: DESKTOP, remoteServer: null })).toHaveLength(4) + expect( + buildVersionReport({ + app: DESKTOP, + remoteServer: { baseUrl: 'https://notes.example.com', version: '2.52.0' } + }).at(-1) + ).toBe('Workspace: remote, server 2.52.0 at https://notes.example.com') + expect( + buildVersionReport({ app: DESKTOP, remoteServer: { baseUrl: null, version: null } }).at(-1) + ).toBe('Workspace: remote, server') + }) +}) diff --git a/packages/app-core/src/lib/version-report.ts b/packages/app-core/src/lib/version-report.ts new file mode 100644 index 00000000..3127f0d7 --- /dev/null +++ b/packages/app-core/src/lib/version-report.ts @@ -0,0 +1,31 @@ +import type { ZenAppInfo } from '@bridge-contract/bridge' + +/** + * The lines `:version` prints and Settings > About shows (#814). One place + * builds them so a bug report pasted from either surface reads the same. + * A field the host did not fill in is left out rather than shown as + * "unknown": the host (desktop preload, web bridge, mobile shell) is the one + * that knows what it can answer. Pure, so the shape is testable without a + * window. + */ +export interface VersionReportInput { + app: ZenAppInfo + /** The `remote` workspace a desktop window is connected to, if any. */ + remoteServer?: { baseUrl: string | null; version: string | null } | null +} + +export function buildVersionReport(input: VersionReportInput): string[] { + const { app } = input + const host = app.hostKind ?? (app.runtime === 'desktop' ? 'desktop' : 'browser') + const lines = [`${app.productName ?? 'ZenNotes'} ${app.version} (${host})`] + const os = [app.os, app.arch].filter((part): part is string => Boolean(part)).join(', ') + if (os) lines.push(`OS: ${os}`) + if (app.engine) lines.push(`Engine: ${app.engine}`) + if (app.install) lines.push(`Install: ${app.install}`) + if (input.remoteServer) { + const server = input.remoteServer.version ? `server ${input.remoteServer.version}` : 'server' + const at = input.remoteServer.baseUrl ? ` at ${input.remoteServer.baseUrl}` : '' + lines.push(`Workspace: remote, ${server}${at}`) + } + return lines +} diff --git a/packages/bridge-contract/src/bridge.ts b/packages/bridge-contract/src/bridge.ts index 33c495eb..c48ddd51 100644 --- a/packages/bridge-contract/src/bridge.ts +++ b/packages/bridge-contract/src/bridge.ts @@ -127,6 +127,16 @@ export interface ZenAppInfo { /** Legacy renderer family. Use hostKind to distinguish native mobile shells. */ runtime: 'desktop' | 'web' hostKind?: 'desktop' | 'browser' | 'ios' | 'android' + /** The details a bug report needs beside the version, read by `:version` + * and Settings > About (#814). Each host fills in what it knows; a field + * it cannot answer is left out of the report, never guessed. */ + arch?: string + /** Operating system name and version, e.g. `macOS 26.0`, `Ubuntu 24.04.1 LTS (kernel 6.8.0)`. */ + os?: string + /** What runs the renderer, e.g. `Electron 38.1.0 (Chromium 140.0.7339.133, Node 22.19.0)`. */ + engine?: string + /** How this copy was installed, e.g. `AppImage`, `deb package`, `package manager or tarball`. */ + install?: string } export interface ZenBridge { diff --git a/packages/bridge-contract/src/ipc.ts b/packages/bridge-contract/src/ipc.ts index 69c6749a..4cad4356 100644 --- a/packages/bridge-contract/src/ipc.ts +++ b/packages/bridge-contract/src/ipc.ts @@ -180,6 +180,7 @@ export const IPC = { RAYCAST_GET_STATUS: 'raycast:get-status', RAYCAST_INSTALL: 'raycast:install', CONFIG_GET_SYNC: 'config:get-sync', + APP_INSTALL_INFO_SYNC: 'app:install-info-sync', CONFIG_SET: 'config:set', CONFIG_GET_PATH: 'config:get-path', CONFIG_REVEAL: 'config:reveal', From 967f701e7bf96edf1cc9ccc149c6d3ddf16b81f1 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 16:29:00 -0500 Subject: [PATCH 07/12] Feat(cli): `zn open -n` opens a second window instead of raising the first (#815) `zn open ` bound to a key, a window on that folder already up in another workspace, and the app raised that window: focus jumped back to the workspace the user had just left, tabs and all. @radiorambo wanted what Chrome's `-n` gives: a fresh window on the same notes beside the new workspace, with its own tabs, the first window left where it was (#815). `zn open -n ` (or `--new-window`) does that now, in both the bundled Node CLI and the Go `zn`. The CLI hands the app a `--new-window` argv switch ahead of the paths. Chromium ignores switches it does not know and Electron does not define this one, so it reaches `second-instance` untouched, and the app's path collector already skips `-`-prefixed entries, so an older app raises the existing window as before. On a cold start the switch changes nothing: there is no window to reuse yet. What "a second window" means depends on what the path is: - A vault a window has open for real: a second real vault window on the same root, exactly what "Open Vault in New Window" does. A temporary session on that root is not an option, because the ephemeral registry is keyed by root and never unregistered, so it would silently switch the first window's workspace-state and settings writes off and send its deleted notes to the system Trash. - A folder that is not a vault: a second ephemeral session window. - A note inside a vault: a new vault window with the note queued. - A markdown file outside every vault keeps reusing its window even with `-n`: nothing keeps two standalone editors of one file in sync, so the second would overwrite the first's saves. The flag also needed a parser fix, since `zn open --new-window ~/notes` parsed as new-window="~/notes" with no path. `VALUELESS_FLAGS` in cli/args.ts lists the long flags that are switches, and the parser never takes the next token as their value (`--flag=value` still works). Every flag on the list was read only through getBool(), so nothing else changes, and `zn delete --yes inbox/a.md` stops swallowing its path too. The Go CLI's parser mirrors the list. Help: the `zn --help` OPEN row names the flag with an example, and the in-app manual's CLI section gains a card for opening a second window. Verified with unit tests (switch detection, switch-before-positional for every valueless flag, the argv the CLI spawns) and in the built app over CDP with isolated stores: `-n ` gives a second real window on the same root while the first keeps its note; `-n /Ideas.md` opens the note in a third window with the first untouched; a plain folder reuses without `-n` and gets a second session window with it; closing the second window leaves the first intact. The Go `zn open -n` was run end to end against the same app. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 --- apps/desktop/src/cli/args.test.ts | 42 ++++++++++++++++- apps/desktop/src/cli/args.ts | 23 ++++++++- apps/desktop/src/cli/commands/open.test.ts | 36 +++++++++++++- apps/desktop/src/cli/commands/open.ts | 21 +++++++-- apps/desktop/src/cli/help.ts | 5 +- apps/desktop/src/main/file-open.test.ts | 24 ++++++++++ apps/desktop/src/main/file-open.ts | 13 +++++ apps/desktop/src/main/index.ts | 55 +++++++++++++++++++--- packages/app-core/src/lib/help.ts | 5 ++ 9 files changed, 205 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/cli/args.test.ts b/apps/desktop/src/cli/args.test.ts index dbf5da2f..9b45b2cf 100644 --- a/apps/desktop/src/cli/args.test.ts +++ b/apps/desktop/src/cli/args.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { getString, parse } from './args' +import { getBool, getString, parse, VALUELESS_FLAGS } from './args' // A markdown task passed to `zn capture "- [ ] task"` arrives as one token // that starts with `-`; it must stay positional, not be parsed as a flag. @@ -24,3 +24,43 @@ describe('cli args parse — leading-dash text', () => { expect(parse(['--', '--not-a-flag']).positionals).toEqual(['--not-a-flag']) }) }) + +// A switch written before a positional used to swallow it: `zn open +// --new-window ~/notes` parsed as new-window="~/notes" with no path (#815). +describe('cli args parse: switches never take the next token as a value', () => { + it('keeps the path after --new-window positional', () => { + const args = parse(['open', '--new-window', '/home/user/notes']) + expect(args.positionals).toEqual(['open', '/home/user/notes']) + expect(getBool(args, 'new-window')).toBe(true) + }) + + it('applies to every listed switch, before or after the positional', () => { + for (const name of VALUELESS_FLAGS) { + const before = parse([`--${name}`, 'inbox/a.md']) + expect(before.positionals, `--${name} before`).toEqual(['inbox/a.md']) + expect(getBool(before, name), `--${name} before`).toBe(true) + + const after = parse(['inbox/a.md', `--${name}`]) + expect(after.positionals, `--${name} after`).toEqual(['inbox/a.md']) + expect(getBool(after, name), `--${name} after`).toBe(true) + } + }) + + it('still accepts an explicit --flag=value for a switch', () => { + expect(getString(parse(['--json=false']), 'json')).toBe('false') + expect(getBool(parse(['--json=false']), 'json')).toBe(false) + }) + + it('leaves value flags alone', () => { + const args = parse(['list', '--tag', 'idea', '--limit', '5']) + expect(args.positionals).toEqual(['list']) + expect(getString(args, 'tag')).toBe('idea') + expect(getString(args, 'limit')).toBe('5') + }) + + it('reads the short -n switch without eating the path', () => { + const args = parse(['open', '-n', '/home/user/notes']) + expect(args.positionals).toEqual(['open', '/home/user/notes']) + expect(getBool(args, 'n')).toBe(true) + }) +}) diff --git a/apps/desktop/src/cli/args.ts b/apps/desktop/src/cli/args.ts index aa6cdf2f..766fb3f0 100644 --- a/apps/desktop/src/cli/args.ts +++ b/apps/desktop/src/cli/args.ts @@ -3,7 +3,7 @@ * zn [] [positional...] [--flag value | --flag=value | -x value] * Repeated flags (e.g. `--tag a --tag b`) collect into an array via getMany(). * Boolean flags are inferred when no value follows or when the next token - * starts with `--`. + * starts with `--`, and the flags in VALUELESS_FLAGS never take one. */ export interface ParsedArgs { @@ -14,6 +14,25 @@ export interface ParsedArgs { flags: Map } +/** + * Long flags that are switches, never `--flag `. Without this list a + * switch written before a positional swallowed it: `zn open --new-window + * ~/notes` parsed as new-window="~/notes" and no path (#815), and `zn delete + * --yes inbox/a.md` the same way. `--flag=value` still works for all of them. + * The Go CLI's parser mirrors this list; keep the two in sync. + */ +export const VALUELESS_FLAGS: ReadonlySet = new Set([ + 'all', + 'include-excluded', + 'json', + 'meta', + 'new-window', + 'page', + 'reopen', + 'unchecked', + 'yes' +]) + export function parse(argv: string[]): ParsedArgs { const positionals: string[] = [] const flags = new Map() @@ -36,7 +55,7 @@ export function parse(argv: string[]): ParsedArgs { } const name = token.slice(2) const next = argv[i + 1] - if (next != null && !next.startsWith('--')) { + if (!VALUELESS_FLAGS.has(name) && next != null && !next.startsWith('--')) { push(flags, name, next) i += 1 } else { diff --git a/apps/desktop/src/cli/commands/open.test.ts b/apps/desktop/src/cli/commands/open.test.ts index 841707b3..2398adb7 100644 --- a/apps/desktop/src/cli/commands/open.test.ts +++ b/apps/desktop/src/cli/commands/open.test.ts @@ -19,8 +19,16 @@ import { spawn } from 'node:child_process' import { cmdOpen } from './open' import type { ParsedArgs } from '../args' -function makeArgs(positionals: string[]): ParsedArgs { - return { positionals, flags: new Map() } +function makeArgs(positionals: string[], flags: Record = {}): ParsedArgs { + return { + positionals, + flags: new Map(Object.entries(flags).map(([name, value]) => [name, [value]])) + } +} + +function lastMessage(): string { + const writes = vi.mocked(process.stdout.write).mock.calls + return String(writes[writes.length - 1]?.[0] ?? '') } let tmpDir: string @@ -166,6 +174,30 @@ describe('cmdOpen', () => { expect(argv).toEqual([mdFile, vaultDir]) }) + // #815: `-n` / `--new-window` asks the app for a fresh window instead of + // raising the one that already shows the vault or folder. + it('forwards --new-window ahead of the paths when -n is given', async () => { + await cmdOpen('', makeArgs([vaultDir], { n: 'true' })) + expect(spawn).toHaveBeenCalledTimes(1) + const [, argv] = vi.mocked(spawn).mock.calls[0] + expect(argv).toEqual(['--new-window', vaultDir]) + expect(lastMessage()).toBe(`Opening folder ${vaultDir} in a new ZenNotes window\n`) + }) + + it('accepts the long --new-window spelling and covers every path of the launch', async () => { + await cmdOpen('', makeArgs([mdFile, vaultDir], { 'new-window': 'true' })) + const [, argv] = vi.mocked(spawn).mock.calls[0] + expect(argv).toEqual(['--new-window', mdFile, vaultDir]) + expect(lastMessage()).toBe('Opening 2 items in a new ZenNotes window\n') + }) + + it('passes no switch, and says so, without the flag', async () => { + await cmdOpen('', makeArgs([vaultDir])) + const [, argv] = vi.mocked(spawn).mock.calls[0] + expect(argv).toEqual([vaultDir]) + expect(lastMessage()).toBe(`Opening folder ${vaultDir} in ZenNotes\n`) + }) + it('reports a launch that dies instead of printing success', async () => { // A crash in the app's first moments (no display, broken sandbox) used to // be invisible: the CLI printed "Opening …" regardless. diff --git a/apps/desktop/src/cli/commands/open.ts b/apps/desktop/src/cli/commands/open.ts index 3693e5d4..267befb4 100644 --- a/apps/desktop/src/cli/commands/open.ts +++ b/apps/desktop/src/cli/commands/open.ts @@ -7,12 +7,18 @@ * zn open inbox/demo/03 — Tables and Task Lists.md (unquoted is fine) * zn open ~/code/myproject/docs (a folder → focused session) * zn open ~/notes (a vault) + * zn open -n ~/notes (a second window on that vault) * * A file must be markdown; a folder opens as a focused, non-persisted * session rooted at that folder — the app scopes the window to it and * shows only its notes, without registering a vault. That's the way to * read a repo's `docs/` or zoom in on one folder of a big vault (#466). * + * When a window already shows the vault or folder, the app raises that + * window. `-n` / `--new-window` asks for a fresh one instead, the way + * Chrome's flag of the same name does (#815): a second workspace on the + * same notes, with its own tabs, leaving the first where it was. + * * Paths resolve against the current directory first, then the active * vault root — so the vault-relative paths `zn list` prints open from * anywhere. When the shell has split an unquoted path with spaces into @@ -27,8 +33,8 @@ import { spawn } from 'node:child_process' import { promises as fsp } from 'node:fs' import path from 'node:path' -import { isMarkdownFilePath } from '../../main/file-open.js' -import { type ParsedArgs } from '../args.js' +import { isMarkdownFilePath, NEW_WINDOW_SWITCH } from '../../main/file-open.js' +import { getBool, type ParsedArgs } from '../args.js' import { emitOk } from '../format.js' export interface ResolvedOpenTarget { @@ -106,13 +112,17 @@ export async function cmdOpen(vault: string, args: ParsedArgs): Promise { } const absPaths = resolved.map((r) => r.abs) + const newWindow = getBool(args, 'n') || getBool(args, 'new-window') + // The switch rides in argv ahead of the paths; the app reads it per launch, + // so it covers every path given here and none of a later `zn open`. + const launchArgs = newWindow ? [NEW_WINDOW_SWITCH, ...absPaths] : absPaths // Re-launch our own binary in GUI mode. The CLI wrapper set // ELECTRON_RUN_AS_NODE so this process runs as plain Node, so we must // drop it for the child or it would start as Node too instead of the app. const env = { ...process.env } delete env.ELECTRON_RUN_AS_NODE - const child = spawn(process.execPath, absPaths, { + const child = spawn(process.execPath, launchArgs, { detached: true, stdio: 'ignore', env @@ -140,10 +150,11 @@ export async function cmdOpen(vault: string, args: ParsedArgs): Promise { child.unref() if (failure) throw new Error(failure) + const where = newWindow ? 'in a new ZenNotes window' : 'in ZenNotes' if (resolved.length === 1) { const only = resolved[0]! - emitOk(`Opening ${only.isDirectory ? 'folder ' : ''}${only.abs} in ZenNotes`) + emitOk(`Opening ${only.isDirectory ? 'folder ' : ''}${only.abs} ${where}`) } else { - emitOk(`Opening ${resolved.length} items in ZenNotes`) + emitOk(`Opening ${resolved.length} items ${where}`) } } diff --git a/apps/desktop/src/cli/help.ts b/apps/desktop/src/cli/help.ts index 8b7c51e6..7ef82e0e 100644 --- a/apps/desktop/src/cli/help.ts +++ b/apps/desktop/src/cli/help.ts @@ -174,7 +174,7 @@ const SECTIONS: Array<{ heading: string; rows: CommandRow[] }> = [ { heading: 'OPEN', rows: [ - { name: 'open ', description: 'Open markdown files, or a folder / vault (a focused session), in the app' } + { name: 'open ', description: 'Open markdown files, or a folder / vault (a focused session), in the app', flags: '-n, --new-window' } ] }, { @@ -215,7 +215,8 @@ const EXAMPLES: string[] = [ 'zn comment list inbox/Plan.md', 'zn comment reply inbox/Plan.md "Agreed, fixed in the second paragraph." --author Claude', 'zn open ~/Downloads/notes.md', - 'zn open ~/code/project/docs # focus a folder as a session' + 'zn open ~/code/project/docs # focus a folder as a session', + 'zn open -n ~/notes # a second window on a vault that is already open' ] function header(width: number): string[] { diff --git a/apps/desktop/src/main/file-open.test.ts b/apps/desktop/src/main/file-open.test.ts index d3c3ba0c..47a8e9ba 100644 --- a/apps/desktop/src/main/file-open.test.ts +++ b/apps/desktop/src/main/file-open.test.ts @@ -1,9 +1,11 @@ import path from 'node:path' import { describe, expect, it } from 'vitest' import { + argvRequestsNewWindow, candidatePathsFromArgv, isMarkdownFilePath, markdownPathsFromArgv, + NEW_WINDOW_SWITCH, resolveMarkdownOpenTarget, vaultRelativeNotePath } from './file-open' @@ -188,3 +190,25 @@ describe('candidatePathsFromArgv own-app-path filter (#579)', () => { } ) }) + +describe('--new-window switch (#815)', () => { + // `zn open -n` forwards the switch ahead of the paths; the app must both + // read it and keep treating it as a switch, never as a path to open. + it('is read from anywhere in argv, and only as the exact switch', () => { + expect(argvRequestsNewWindow(['/bin/ZenNotes', NEW_WINDOW_SWITCH, '/home/user/vault'])).toBe( + true + ) + expect(argvRequestsNewWindow(['/bin/ZenNotes', '/home/user/vault', NEW_WINDOW_SWITCH])).toBe( + true + ) + expect(argvRequestsNewWindow(['/bin/ZenNotes', '/home/user/vault'])).toBe(false) + // A path that merely contains the words is a path. + expect(argvRequestsNewWindow(['/bin/ZenNotes', '/vault/--new-window'])).toBe(false) + }) + + it('does not leak into the candidate paths', () => { + const argv = ['/bin/ZenNotes', NEW_WINDOW_SWITCH, '/home/user/vault', '/home/user/todo.md'] + expect(candidatePathsFromArgv(argv)).toEqual(['/home/user/vault', '/home/user/todo.md']) + expect(markdownPathsFromArgv(argv)).toEqual(['/home/user/todo.md']) + }) +}) diff --git a/apps/desktop/src/main/file-open.ts b/apps/desktop/src/main/file-open.ts index ec10ea28..1eb2f3f1 100644 --- a/apps/desktop/src/main/file-open.ts +++ b/apps/desktop/src/main/file-open.ts @@ -27,6 +27,19 @@ export function vaultRelativeNotePath(vaultRoot: string, absPath: string): strin return segments.join('/') } +/** + * The argv switch that asks for a fresh window even when one already shows the + * same vault, folder or note (#815). `zn open -n` forwards it; Chrome spells + * its equivalent the same way, so a launcher can also pass it to the app + * directly. Chromium ignores switches it does not know, and Electron does not + * define this one, so it reaches `second-instance` untouched. + */ +export const NEW_WINDOW_SWITCH = '--new-window' + +export function argvRequestsNewWindow(argv: readonly string[]): boolean { + return argv.includes(NEW_WINDOW_SWITCH) +} + export type MarkdownOpenTarget = | { kind: 'vault'; vaultRoot: string; relPath: string } | { kind: 'external'; absPath: string } diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 323d50ba..e5a00e13 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -269,6 +269,7 @@ import { ZENNOTES_DEEP_LINK_SCHEME, } from "./deep-links"; import { + argvRequestsNewWindow, isMarkdownFilePath, MARKDOWN_FILE_EXTENSIONS, candidatePathsFromArgv, @@ -391,7 +392,13 @@ let cloudAuthLoopbackServer: CloudAuthLoopbackServer | null = null; let cloudSyncService: DesktopCloudSyncService | null = null; // Markdown files handed to us by the OS (Finder "Open With", a file // double-click, drag onto the dock, or a Windows/Linux argv launch). -const pendingFileOpens: { absPath: string; reuseMainWindow: boolean }[] = []; +// `newWindow` is the `--new-window` ask (#815): open a fresh window even when +// one already shows the same vault or folder, instead of raising that one. +const pendingFileOpens: { + absPath: string; + reuseMainWindow: boolean; + newWindow: boolean; +}[] = []; // windowId -> absolute path of the standalone external file it edits. const externalFileWindows = new Map(); // Per-window renderer readiness, so note-open requests can target any @@ -707,8 +714,13 @@ function findWindowForVaultRoot(root: string): BrowserWindow | null { function queueMarkdownFileOpen( rawPath: string, reuseMainWindow: boolean, + newWindow = false, ): void { - pendingFileOpens.push({ absPath: path.resolve(rawPath), reuseMainWindow }); + pendingFileOpens.push({ + absPath: path.resolve(rawPath), + reuseMainWindow, + newWindow, + }); // Only flush eagerly once startup is finished. During startup `app.isReady()` // is already true (we're inside whenReady), so an eager flush here would // drain the queue before whenReady's own flush runs — that flush would then @@ -728,13 +740,16 @@ function handleStartupMarkdownArgs( // skipping by index alone let the app dir through as a folder to open. const isUnpackagedElectronLaunch = (process as NodeJS.Process & { defaultApp?: boolean }).defaultApp === true; + // `--new-window` covers every path of this launch (#815). On a cold start it + // changes nothing: there is no window to reuse yet. + const newWindow = argvRequestsNewWindow(argv); let queued = 0; for (const candidate of candidatePathsFromArgv( argv, isUnpackagedElectronLaunch, app.getAppPath(), )) { - queueMarkdownFileOpen(candidate, reuseMainWindow); + queueMarkdownFileOpen(candidate, reuseMainWindow, newWindow); queued += 1; } return queued; @@ -773,7 +788,13 @@ async function drainPendingFileOpens(): Promise { let openedAny = false; for (const item of items) { try { - if (await openMarkdownFileFromOS(item.absPath, item.reuseMainWindow)) { + if ( + await openMarkdownFileFromOS( + item.absPath, + item.reuseMainWindow, + item.newWindow, + ) + ) { openedAny = true; } } catch (err) { @@ -783,9 +804,14 @@ async function drainPendingFileOpens(): Promise { return openedAny; } +// `newWindow` (#815) opens a fresh window on the vault or folder even when one +// already shows it. A file outside every vault keeps reusing its window on +// purpose: nothing keeps two standalone editors of one file in sync, so the +// second would silently overwrite the first's saves. async function openMarkdownFileFromOS( absPath: string, reuseMainWindow: boolean, + newWindow = false, ): Promise { let stat; try { @@ -795,7 +821,7 @@ async function openMarkdownFileFromOS( } // A dropped folder opens as a temporary, non-persisted session. if (stat.isDirectory()) { - return await openTemporaryFolder(absPath, reuseMainWindow); + return await openTemporaryFolder(absPath, reuseMainWindow, newWindow); } if (!stat.isFile() || !isMarkdownFilePath(absPath)) return false; @@ -812,7 +838,7 @@ async function openMarkdownFileFromOS( const target = resolveMarkdownOpenTarget(absPath, knownRoots); if (target.kind === "vault") { - const existing = findWindowForVaultRoot(target.vaultRoot); + const existing = newWindow ? null : findWindowForVaultRoot(target.vaultRoot); if (existing) { focusWindow(existing); queueNoteOpenForWindow(existing, target.relPath); @@ -869,13 +895,28 @@ async function openMarkdownFileViaDialog( async function openTemporaryFolder( dir: string, reuseMainWindow: boolean, + newWindow = false, ): Promise { const resolved = path.resolve(dir); const existing = findWindowForVaultRoot(resolved); - if (existing) { + if (existing && !newWindow) { focusWindow(existing); return true; } + if (existing && windowVaults.vaultForWindow(existing.id)?.temporary !== true) { + // `zn open -n ` on a vault a window has open for real (#815). A + // temporary session on that root is not an option: the ephemeral registry + // is keyed by root, not by window, so it would also switch the first + // window's workspace-state and settings writes off and send its deleted + // notes to the system Trash. Open a second real window on the vault + // instead, exactly what "Open Vault in New Window" does. + const win = await createWindow({ + initialVaultRoot: resolved, + persistInitialVault: true, + }); + if (!reuseMainWindow) focusWindow(win); + return true; + } // No markdown-content gate here anymore. It used to bail when a bounded scan // found no markdown, which read as protection against opening junk folders, // but every caller is an explicit ask (`zn open `, a folder handed to diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index 144046bb..5282a863 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -1225,6 +1225,11 @@ export const HELP_CLI: HelpCard[] = [ body: 'Use `zn list` to see recent notes, `zn list --tag work --limit 5` to filter, `zn read inbox/Project.md` to print a body, and `zn search "deadline"` for full-text matches with file:line previews. Quote paths with spaces, like `zn read "hellointerview/system design.md"`, or use `--path`. Add `--json` to any command to get structured output you can pipe into `jq`.' }, + { + title: 'Open notes, folders, and a second window', + body: + '`zn open inbox/Today.md` brings the ZenNotes window to the front with that note loaded, and `zn open ~/code/project/docs` opens a folder as a focused session without turning it into a vault. When a window already shows the vault or folder, `zn open` raises that window. Add `-n` (or `--new-window`) for a second window on the same notes instead, with its own tabs, leaving the first window where it was: `zn open -n ~/notes`. A markdown file outside every vault always reuses its editor window, since nothing keeps two standalone editors of one file in sync.' + }, { title: 'Raycast uses the same CLI', body: From 6ace1cbd3d8acc477dd36412d38e60adc6eab0b2 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 19:31:47 -0500 Subject: [PATCH 08/12] Feat(home): Favorites on the home view, and a way to favorite a note from anywhere (#810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @saran-ncsu asked for a list of pinned or favorite notes to open from without keeping them around as tabs, and on the phones, which open Recent from the home screen, that same list (#810). ZenNotes already had Favorites: the sidebar section that `Space l s` and the row's context menu fill, stored in vault.json so it travels with the vault and with sync. What it lacked was a place to see them when no note is open, and a way to favorite a note that does not go through the sidebar. The home view now shows them. A Favorites section sits right after Recent, in the order the user arranged, with the icon and color the sidebar gives each entry and, for a note, where it lives ("Projects / Alpha"). A favorited note opens in place. A favorited folder opens that folder in the note list, brings the sidebar back if it was hidden and expands the folder and its ancestors in the tree first: folders start collapsed, and in the unified sidebar the tree is the only place the folder's notes appear, so a highlighted collapsed row would look like nothing happened. j/k (Vim mode) and the arrows walk from Recent into Favorites and Enter opens. The section stays out of the way until something is favorited, and a favorite whose note is gone (trashed, moved away) is skipped, as in the sidebar. Favoriting gains two routes. The command palette has "Add Note to Favorites" / "Remove Note from Favorites" (`note.favorite`; the title follows the active note's state, `when` hides it for trashed notes, the shortcut is resolved from the `vim.leaderToggleFavorite` chord). Hosts get `requestToggleNoteFavorite(host, path)`, wrapped in the usual `requestNoteAction` staleness guard, and `ShellSnapshot.favorites` (a frozen array, rebuilt only when the contents change so a folder-color save does not wake subscribers), which is what the phone shells use to put an "Add to Favorites" row in their ••• sheet and long-press menu, next to Pin. Pin stays the phone's own list order; Favorites is the vault's. The shell halves live in the phone repos. `resolveFavoriteItems(favorites, notes, folders)` moves out of Sidebar.tsx into lib/vault-layout.ts, and both the sidebar and HomeView read the same list through it, so a stale key can never show on one surface and not the other. Home stamps `data-home-section` (recent, favorites, today) on its sections and `data-home-favorite` (note or folder) plus `data-home-note-path` on rows; the phone shells' CSS and their row-swipe resolver key off these instead of counting sections and resolving Recent rows by position, which is what they did before. Pinned tabs are unchanged. The in-app manual's home-view card and the `Space l s` entries name the new surface. Verified with unit tests (vault-layout, commands, note-actions, public-host-api) and in the built app over CDP with isolated stores: a note favorite, a folder favorite and a deleted note's key resolve to two rows with the deleted one skipped; j moves past Recent onto the first favorite and Enter opens it; the folder row restores the hidden sidebar and expands the collapsed folder; unfavoriting removes the row at once; a vault without favorites shows no section. The phone stylesheet was checked against this build and against a core with no section markers; the Android shell was driven on the emulator and the iPhone shell with an XCUITest. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 --- packages/app-core/src/components/HomeView.tsx | 129 +++++++++++++++++- packages/app-core/src/components/Sidebar.tsx | 49 ++----- packages/app-core/src/components/icons.tsx | 7 + packages/app-core/src/lib/commands.test.ts | 32 +++++ packages/app-core/src/lib/commands.ts | 19 +++ packages/app-core/src/lib/help.ts | 6 +- .../app-core/src/lib/vault-layout.test.ts | 55 ++++++++ packages/app-core/src/lib/vault-layout.ts | 52 +++++++ packages/app-core/src/note-actions.test.ts | 39 ++++++ packages/app-core/src/notes.ts | 17 +++ packages/app-core/src/public-host-api.test.ts | 17 +++ packages/app-core/src/shell.ts | 21 ++- 12 files changed, 394 insertions(+), 49 deletions(-) diff --git a/packages/app-core/src/components/HomeView.tsx b/packages/app-core/src/components/HomeView.tsx index b30994dc..ba117d0f 100644 --- a/packages/app-core/src/components/HomeView.tsx +++ b/packages/app-core/src/components/HomeView.tsx @@ -4,6 +4,15 @@ import { filterTasksForDisplay, type VaultTask } from '@shared/tasks' import { useStore } from '../store' import { computeTasksRender } from '../lib/tasks-filter' import { InlineMarkdown } from '../lib/inline-markdown' +import { getSystemFolderLabel } from '../lib/system-folder-labels' +import { + isPrimaryNotesAtRoot, + noteFolderSubpath, + resolveFavoriteItems, + type FavoriteItem +} from '../lib/vault-layout' +import { colorGlyphClassById, resolveFolderColorGlyphClass } from './FolderColors' +import { iconOptionById, resolveFolderIconOption } from './FolderIcons' import { ArrowUpRightIcon, CalendarIcon, @@ -13,6 +22,7 @@ import { ExcalidrawIcon, NotePlusIcon, PanelLeftIcon, + StarIcon, ZapIcon } from './icons' @@ -49,8 +59,8 @@ function timeAgo(ts: number, now: number): string { } /** A light landing view shown when no note is open: the few most recently - * edited notes plus the open tasks for today. Keyboard: ↑/↓ (and j/k in vim - * mode) move between rows, Enter opens. */ + * edited notes, the vault's favorites, plus the open tasks for today. + * Keyboard: ↑/↓ (and j/k in vim mode) move between rows, Enter opens. */ export function HomeView({ sidebarOpen, onShowSidebar @@ -59,11 +69,14 @@ export function HomeView({ onShowSidebar: () => void }): JSX.Element { const notes = useStore((s) => s.notes) + const folders = useStore((s) => s.folders) const vaultTasks = useStore((s) => s.vaultTasks) const showArchivedTasks = useStore((s) => s.showArchivedTasks) const tasksLoading = useStore((s) => s.tasksLoading) const vimMode = useStore((s) => s.vimMode) const selectNote = useStore((s) => s.selectNote) + const setView = useStore((s) => s.setView) + const systemFolderLabels = useStore((s) => s.systemFolderLabels) const openTaskAt = useStore((s) => s.openTaskAt) const toggleTaskFromList = useStore((s) => s.toggleTaskFromList) const refreshTasks = useStore((s) => s.refreshTasks) @@ -145,6 +158,55 @@ export function HomeView({ [notes] ) + // Favorites (#810) share the sidebar's resolver, so a stale key (note moved + // to trash, folder deleted) disappears from both surfaces at the same time. + // Stored order is kept: the user arranged the list, the home view shows it. + const favorites = useMemo( + () => resolveFavoriteItems(vaultSettings.favorites, notes, folders), + [vaultSettings.favorites, notes, folders] + ) + + // Where a favorited note lives, as the breadcrumb would spell it: the + // system-folder label (skipped for inbox notes kept at the vault root) and + // the subpath segments. Empty for a top-level note. + const favoriteNoteLocation = useCallback( + (path: string): string => { + const note = notes.find((n) => n.path === path) + if (!note) return '' + const segments: string[] = [] + if (!(note.folder === 'inbox' && isPrimaryNotesAtRoot(vaultSettings))) { + segments.push(getSystemFolderLabel(note.folder, systemFolderLabels)) + } + const sub = noteFolderSubpath(note, vaultSettings) + if (sub) segments.push(...sub.split('/')) + return segments.join(' / ') + }, + [notes, vaultSettings, systemFolderLabels] + ) + + // A favorited folder opens as the folder view, the same target the sidebar's + // Favorites row uses. Two things make that visible from here: the sidebar + // comes back if it was hidden, and the folder plus its ancestors expand in + // the tree. Folders start collapsed, and in the unified sidebar the tree is + // the only place the folder's notes appear, so a highlighted collapsed row + // would look like nothing happened. + const openFavoriteFolder = useCallback( + (item: Extract) => { + if (!sidebarOpen) onShowSidebar() + const chain = new Set([`${item.folder}:`]) + let acc = '' + for (const segment of item.subpath.split('/')) { + acc = acc ? `${acc}/${segment}` : segment + chain.add(`${item.folder}:${acc}`) + } + const { collapsedFolders, setCollapsedFolders } = useStore.getState() + const expanded = collapsedFolders.filter((key) => !chain.has(key)) + if (expanded.length !== collapsedFolders.length) setCollapsedFolders(expanded) + setView({ kind: 'folder', folder: item.folder, subpath: item.subpath }) + }, + [sidebarOpen, onShowSidebar, setView] + ) + const { today, overdueCount } = useMemo(() => { const render = computeTasksRender( filterTasksForDisplay(vaultTasks, showArchivedTasks), @@ -234,7 +296,7 @@ export function HomeView({ ))}
-
+
} text="Recent" /> {recent.length > 0 ? (
    @@ -243,6 +305,7 @@ export function HomeView({
-
+ {favorites.length > 0 && ( +
+ } text="Favorites" /> +
    + {favorites.map((item) => { + // Same glyph and tint the sidebar gives the favorite, so the + // item is recognisable across both surfaces. + const icon = + item.kind === 'note' + ? vaultSettings.folderIcons[item.path] + ? iconOptionById(vaultSettings.folderIcons[item.path]).icon + : item.isDrawing + ? + : + : resolveFolderIconOption(item.folder, item.subpath, vaultSettings.folderIcons) + .icon + const colorClass = + item.kind === 'note' + ? colorGlyphClassById(vaultSettings.folderColors[item.path]) + : resolveFolderColorGlyphClass( + item.folder, + item.subpath, + vaultSettings.folderColors + ) + const label = item.kind === 'note' ? item.title || 'Untitled' : item.label + const location = item.kind === 'note' ? favoriteNoteLocation(item.path) : 'Folder' + return ( +
  • + +
  • + ) + })} +
+
+ )} + +
} text="Today" diff --git a/packages/app-core/src/components/Sidebar.tsx b/packages/app-core/src/components/Sidebar.tsx index 74ed703c..5ed3f528 100644 --- a/packages/app-core/src/components/Sidebar.tsx +++ b/packages/app-core/src/components/Sidebar.tsx @@ -79,13 +79,13 @@ import { dateNoteDirectoryDisplayLabel, favoriteFolderKey, folderIconKey, - isFavoriteFolderKey, isPrimaryNotesAtRoot, folderForVaultRelativePath, normalizeVaultSettings, noteFolderSubpath, - parseFavoriteFolderKey, + resolveFavoriteItems, sidebarRevealTarget, + type FavoriteItem, } from "../lib/vault-layout"; import { resolveFolderPath } from "@shared/system-folder-paths"; import { @@ -323,11 +323,6 @@ type SidebarSelectionItem = | { kind: "note"; path: string } | { kind: "folder"; folder: NoteFolder; subpath: string }; -/** A favorite resolved to a live note or folder for rendering. */ -type FavoriteItem = - | { kind: "note"; key: string; path: string; title: string; isDrawing: boolean } - | { kind: "folder"; key: string; folder: NoteFolder; subpath: string; label: string }; - function noteSelectionKey(path: string): string { return `note:${encodeURIComponent(path)}`; } @@ -1261,40 +1256,12 @@ export function Sidebar(): JSX.Element { return next; }, [notes, allFolders, assetFiles, vaultSettings]); - // Resolve favorite keys to live notes/folders. Keys whose target no longer - // exists (renamed away, deleted, trashed) are silently skipped — the Favorites - // section never shows a broken row. Order follows the stored favorites list. - const favoriteItems = useMemo(() => { - const out: FavoriteItem[] = []; - for (const key of vaultSettings.favorites) { - if (isFavoriteFolderKey(key)) { - const parsed = parseFavoriteFolderKey(key); - if (!parsed || !parsed.subpath) continue; - const exists = allFolders.some( - (f) => f.folder === parsed.folder && f.subpath === parsed.subpath, - ); - if (!exists) continue; - out.push({ - kind: "folder", - key, - folder: parsed.folder, - subpath: parsed.subpath, - label: parsed.subpath.split("/").slice(-1)[0], - }); - } else { - const note = notes.find((n) => n.path === key); - if (!note || note.folder === "trash") continue; - out.push({ - kind: "note", - key, - path: note.path, - title: note.title, - isDrawing: isExcalidrawPath(note.path), - }); - } - } - return out; - }, [vaultSettings.favorites, notes, allFolders]); + // Shared with the home view's Favorites section, so both surfaces agree on + // which keys still resolve to a live note or folder. + const favoriteItems = useMemo( + () => resolveFavoriteItems(vaultSettings.favorites, notes, allFolders), + [vaultSettings.favorites, notes, allFolders], + ); // Daily/weekly notes grouped for the pinned date-nav: daily by year → month → // day, weekly by year → week, all newest-first. diff --git a/packages/app-core/src/components/icons.tsx b/packages/app-core/src/components/icons.tsx index 1769ce3a..4c6d3305 100644 --- a/packages/app-core/src/components/icons.tsx +++ b/packages/app-core/src/components/icons.tsx @@ -347,6 +347,13 @@ export const ZapIcon = (p: IconProps): JSX.Element => ( ) +/** Favorites: the section label on the home view. */ +export const StarIcon = (p: IconProps): JSX.Element => ( + + + +) + export const ExternalIcon = (p: IconProps): JSX.Element => ( diff --git a/packages/app-core/src/lib/commands.test.ts b/packages/app-core/src/lib/commands.test.ts index 37eba34a..b7d4876d 100644 --- a/packages/app-core/src/lib/commands.test.ts +++ b/packages/app-core/src/lib/commands.test.ts @@ -351,6 +351,38 @@ describe('note commands for a trashed note (#712)', () => { }) +describe('favorite command (#810)', () => { + const note = { folder: 'inbox', path: 'inbox/Plan.md', title: 'Plan', body: '' } as never + + it('flips its title with the active note and runs the store toggle', async () => { + const { buildCommands, useStore } = await loadCommands() + const toggleFavoriteActiveNote = vi.fn().mockResolvedValue(undefined) + useStore.setState({ activeNote: note, selectedPath: 'inbox/Plan.md', toggleFavoriteActiveNote }) + const add = buildCommands().find((c) => c.id === 'note.favorite') + expect(add?.title).toBe('Add Note to Favorites') + expect(add?.when?.()).toBe(true) + await add?.run() + expect(toggleFavoriteActiveNote).toHaveBeenCalledTimes(1) + + const settings = useStore.getState().vaultSettings + useStore.setState({ vaultSettings: { ...settings, favorites: ['inbox:Work', 'inbox/Plan.md'] } }) + expect(buildCommands().find((c) => c.id === 'note.favorite')?.title).toBe( + 'Remove Note from Favorites' + ) + }) + + it('stays out of the palette with no note open or a trashed one, like the sidebar menu', async () => { + const { buildCommands, useStore } = await loadCommands() + useStore.setState({ activeNote: null, selectedPath: null }) + expect(buildCommands().some((c) => c.id === 'note.favorite')).toBe(false) + useStore.setState({ + activeNote: { folder: 'trash', path: 'trash/Gone.md', title: 'Gone', body: '' } as never, + selectedPath: 'trash/Gone.md' + }) + expect(buildCommands().some((c) => c.id === 'note.favorite')).toBe(false) + }) +}) + describe('saved Tasks filters (#731)', () => { it('lists one palette entry per saved filter, which opens Tasks and applies it', async () => { const { buildCommands, useStore } = await loadCommands() diff --git a/packages/app-core/src/lib/commands.ts b/packages/app-core/src/lib/commands.ts index c457ff69..b3f6347a 100644 --- a/packages/app-core/src/lib/commands.ts +++ b/packages/app-core/src/lib/commands.ts @@ -99,6 +99,10 @@ export function buildCommands(options?: { includeUnavailable?: boolean }): Comma const openExternal = (url: string): void => { window.open(url, '_blank') } + const isActiveNoteFavorite = (): boolean => { + const state = getState() + return !!state.activeNote && state.vaultSettings.favorites.includes(state.activeNote.path) + } const cmds: Command[] = [] /* ---------------- Note actions ---------------- */ @@ -327,6 +331,21 @@ export function buildCommands(options?: { includeUnavailable?: boolean }): Comma await getState().renameActive(next) } }, + { + id: 'note.favorite', + // The sidebar row's context menu was the only mouse route and the + // leader chord the only keyboard one; the palette gives every host, + // phones included, a way to fill the Favorites section on Home. (#810) + title: isActiveNoteFavorite() ? 'Remove Note from Favorites' : 'Add Note to Favorites', + category: 'Note', + keywords: 'favourite star bookmark home sidebar', + shortcut: chord('vim.leaderPrefix', 'vim.leaderNoteActions', 'vim.leaderToggleFavorite'), + when: () => { + const active = getState().activeNote + return !!active && active.folder !== 'trash' + }, + run: () => getState().toggleFavoriteActiveNote() + }, { id: 'note.archive', title: `Move Note to ${labels().archive}`, diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index 5282a863..c3641601 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -237,7 +237,7 @@ export const HELP_CORE_CONCEPTS: HelpCard[] = [ { title: 'The home view is where you land', body: - 'When no note is open (outside Zen mode), ZenNotes shows a light home view instead of a blank pane: a greeting, quick-create actions (new note, database, drawing — plus daily and weekly notes when those are enabled in Settings), your most recently edited notes, and today’s open tasks with an overdue count. Click a note or task to open it, tick a checkbox to complete a task in place, and use ↑/↓ — or j/k in Vim mode — then Enter to move and open from the keyboard.' + 'When no note is open (outside Zen mode), ZenNotes shows a light home view instead of a blank pane: a greeting, quick-create actions (new note, database, drawing, plus daily and weekly notes when those are enabled in Settings), your most recently edited notes, your Favorites, and today’s open tasks with an overdue count. The Favorites section sits right after Recent and mirrors the sidebar’s list in the same order: a favorited note opens in place, a favorited folder opens that folder in the note list (bringing the sidebar back if it was hidden), and the section stays out of the way until you favorite something. Favorite a note from the sidebar row’s context menu, `Space l s`, or the command palette’s “Add Note to Favorites” (on the phones, the same entry sits in the ••• sheet and in the long-press note menu). Click a note or task to open it, tick a checkbox to complete a task in place, and use ↑/↓ (or j/k in Vim mode) then Enter to move and open from the keyboard.' }, { title: 'Sessions restore on relaunch', @@ -551,7 +551,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ { keys: 'Space q', action: 'Quick capture window', detail: 'Open the floating, always-on-top capture window, same as the global hotkey.' }, { keys: 'Space i', action: 'Insert template into note', detail: 'Pick a template and insert it at the cursor of the active note, instead of creating a new note from it.' }, { keys: 'Space c', action: 'Toggle calendar', detail: 'Show or hide the calendar panel for the active pane.' }, - { keys: 'Space l s', action: 'Toggle favorite', detail: 'Add or remove the active note from the sidebar’s Favorites section. Folders join it from their context menu.' }, + { keys: 'Space l s', action: 'Toggle favorite', detail: 'Add or remove the active note from the Favorites section shown in the sidebar and on the home view. Folders join it from their context menu.' }, { keys: 'Space, then pause', action: 'Show leader hints', detail: 'If enabled in Settings, open a which-key style guide for the next available leader actions. Sticky mode keeps it open until `Space` or `Esc`.' }, { keys: 'Mod+3', action: 'Toggle outline panel', detail: 'Show or hide the persistent outline in the active pane. Once focused (Ctrl+W l or Alt+L from the editor), j / k — or the arrows — walk the headings, gg / G jump to the first and last, Enter jumps the editor to the heading under the cursor, and Esc hands focus back.' }, { keys: 'zc / zo', action: 'Fold / unfold heading', detail: 'Collapse or expand the section below the heading at the cursor.' }, @@ -995,7 +995,7 @@ export const HELP_VIM_COMMANDS: HelpExCommand[] = [ { command: ' l s', summary: 'Leader toggle favorite', - detail: 'Add or remove the active note from the sidebar’s Favorites section.' + detail: 'Add or remove the active note from the Favorites section shown in the sidebar and on the home view.' }, { command: ' d', diff --git a/packages/app-core/src/lib/vault-layout.test.ts b/packages/app-core/src/lib/vault-layout.test.ts index cd7915b1..468fb7ba 100644 --- a/packages/app-core/src/lib/vault-layout.test.ts +++ b/packages/app-core/src/lib/vault-layout.test.ts @@ -16,6 +16,7 @@ import { resolveCreateLocation, parseFavoriteFolderKey, removeFavoritesForFolder, + resolveFavoriteItems, rewriteFavoriteNotePath, rewriteFavoritesForFolderRename, toggleFavorite, @@ -676,6 +677,60 @@ describe('favorites', () => { const settings = normalizeVaultSettings({} as unknown as VaultSettings) expect(settings.favorites).toEqual([]) }) + + describe('resolveFavoriteItems', () => { + const notes = [ + note('inbox/Plan.md', 'Plan'), + note('inbox/Projects/Sketch.excalidraw', 'Sketch'), + { ...note('trash/Old.md', 'Old'), folder: 'trash' as const }, + note('inbox/Untitled.md', '') + ] + const folders = [ + { folder: 'inbox' as const, subpath: 'Projects' }, + { folder: 'inbox' as const, subpath: 'Projects/Sub' } + ] + + it('keeps the stored order and resolves notes and folders to live rows', () => { + const items = resolveFavoriteItems( + ['inbox:Projects/Sub', 'inbox/Plan.md', 'inbox/Projects/Sketch.excalidraw'], + notes, + folders + ) + expect(items).toEqual([ + { + kind: 'folder', + key: 'inbox:Projects/Sub', + folder: 'inbox', + subpath: 'Projects/Sub', + label: 'Sub' + }, + { kind: 'note', key: 'inbox/Plan.md', path: 'inbox/Plan.md', title: 'Plan', isDrawing: false }, + { + kind: 'note', + key: 'inbox/Projects/Sketch.excalidraw', + path: 'inbox/Projects/Sketch.excalidraw', + title: 'Sketch', + isDrawing: true + } + ]) + }) + + it('skips keys whose target is gone, trashed, or a top-level folder key', () => { + const items = resolveFavoriteItems( + ['inbox/Missing.md', 'trash/Old.md', 'inbox:Archive2020', 'inbox:', 'inbox/Untitled.md'], + notes, + folders + ) + // The untitled note is the only live target; its empty title is passed + // through untouched so each surface picks its own placeholder. + expect(items.map((i) => i.key)).toEqual(['inbox/Untitled.md']) + expect(items[0]).toMatchObject({ kind: 'note', title: '' }) + }) + + it('returns nothing for a vault without favorites', () => { + expect(resolveFavoriteItems([], notes, folders)).toEqual([]) + }) + }) }) describe('resolveCreateLocation (#362)', () => { diff --git a/packages/app-core/src/lib/vault-layout.ts b/packages/app-core/src/lib/vault-layout.ts index 64229ba8..388def97 100644 --- a/packages/app-core/src/lib/vault-layout.ts +++ b/packages/app-core/src/lib/vault-layout.ts @@ -12,12 +12,14 @@ import { type AssetMeta, type DateNotePatternSettings, type FileLocationSetting, + type FolderEntry, type FolderIconId, type FolderColorId, type NoteFolder, type NoteMeta, type VaultSettings } from '@shared/ipc' +import { isExcalidrawPath } from '@shared/excalidraw' import { normalizeSystemFolderPaths, resolveFolderPath, @@ -702,6 +704,56 @@ export function parseFavoriteFolderKey( return { folder, subpath: key.slice(idx + 1) } } +/** A favorite key resolved to the live note or folder it names, for rendering. */ +export type FavoriteItem = + | { kind: 'note'; key: string; path: string; title: string; isDrawing: boolean } + | { kind: 'folder'; key: string; folder: NoteFolder; subpath: string; label: string } + +/** + * Resolve favorite keys to live notes and folders, in the stored order. A key + * whose target no longer exists (renamed away, deleted, trashed) is skipped, + * so no surface ever shows a broken row. The sidebar's Favorites section and + * the home view's (#810) read the same list through this one function, so a + * favorite can never be visible in one and missing from the other. + */ +export function resolveFavoriteItems( + favorites: readonly string[], + notes: readonly NoteMeta[], + folders: readonly Pick[] +): FavoriteItem[] { + const out: FavoriteItem[] = [] + let byPath: Map | null = null + for (const key of favorites) { + if (isFavoriteFolderKey(key)) { + const parsed = parseFavoriteFolderKey(key) + if (!parsed || !parsed.subpath) continue + const exists = folders.some( + (f) => f.folder === parsed.folder && f.subpath === parsed.subpath + ) + if (!exists) continue + out.push({ + kind: 'folder', + key, + folder: parsed.folder, + subpath: parsed.subpath, + label: parsed.subpath.split('/').slice(-1)[0] + }) + } else { + byPath ??= new Map(notes.map((n) => [n.path, n])) + const note = byPath.get(key) + if (!note || note.folder === 'trash') continue + out.push({ + kind: 'note', + key, + path: note.path, + title: note.title, + isDrawing: isExcalidrawPath(note.path) + }) + } + } + return out +} + /** Toggle a favorite key, returning the next list (added at the end, or removed). */ export function toggleFavorite(favorites: string[], key: string): string[] { return favorites.includes(key) diff --git a/packages/app-core/src/note-actions.test.ts b/packages/app-core/src/note-actions.test.ts index 2d841e00..bd3ca3ed 100644 --- a/packages/app-core/src/note-actions.test.ts +++ b/packages/app-core/src/note-actions.test.ts @@ -1173,6 +1173,45 @@ describe("public note lifecycle", () => { expect(s.files.has("trash/One.md")).toBe(false); }); + it("toggles a note in and out of Favorites and persists the list (#810)", async () => { + const s = await publicSetup(); + expect(await s.requestToggleNoteFavorite(s.host, "inbox/One.md")).toBe( + "completed", + ); + expect(s.useStore.getState().vaultSettings.favorites).toEqual([ + "inbox/One.md", + ]); + expect(s.bridge.setVaultSettings).toHaveBeenLastCalledWith( + expect.objectContaining({ favorites: ["inbox/One.md"] }), + ); + expect(await s.requestToggleNoteFavorite(s.host, "inbox/One.md")).toBe( + "completed", + ); + expect(s.useStore.getState().vaultSettings.favorites).toEqual([]); + }); + + it("keeps trashed and unknown notes out of Favorites, like the sidebar menu", async () => { + const s = await publicSetup(); + const trashing = s.requestTrashNote(s.host, "inbox/One.md"); + s.confirm(true); + await trashing; + s.bridge.setVaultSettings.mockClear(); + expect(await s.requestToggleNoteFavorite(s.host, "trash/One.md")).toBe( + "unavailable", + ); + expect(await s.requestToggleNoteFavorite(s.host, "missing.md")).toBe( + "unavailable", + ); + expect( + await s.requestToggleNoteFavorite( + { isCurrent: () => false }, + "inbox/Other.md", + ), + ).toBe("unavailable"); + expect(s.bridge.setVaultSettings).not.toHaveBeenCalled(); + expect(s.useStore.getState().vaultSettings.favorites).toEqual([]); + }); + it("archives and unarchives through the public boundary", async () => { const s = await publicSetup(); expect(await s.requestArchiveNote(s.host, "inbox/One.md")).toBe( diff --git a/packages/app-core/src/notes.ts b/packages/app-core/src/notes.ts index 63b9c74f..cd79b0ea 100644 --- a/packages/app-core/src/notes.ts +++ b/packages/app-core/src/notes.ts @@ -235,6 +235,23 @@ export function requestDeleteNotePermanently( return requestLifecycle(host, path, "delete"); } +/** + * Add a note to the vault's Favorites, or take it out again: the sidebar + * row's "Add to Favorites" for hosts without that menu. Favorites live in + * vault.json, so the list travels with the vault and shows up on Home and in + * the desktop sidebar. Trashed notes stay out, as on desktop. Reads go through + * the shell snapshot's `favorites`. (#810) + */ +export function requestToggleNoteFavorite( + host: NoteActionHost, + path: string, +): Promise { + return requestNoteAction(host, path, async (state, _note, isCurrent) => { + await state.toggleFavorite(path); + return isCurrent() ? "completed" : "stale"; + }); +} + export type NoteBatchAction = 'archive' | 'trash' | 'restore' | 'delete' | 'move' export interface NoteBatchResult { readonly status: NoteActionResult diff --git a/packages/app-core/src/public-host-api.test.ts b/packages/app-core/src/public-host-api.test.ts index 375633c4..97b09c06 100644 --- a/packages/app-core/src/public-host-api.test.ts +++ b/packages/app-core/src/public-host-api.test.ts @@ -79,6 +79,23 @@ describe('public host APIs', () => { requests.settlePromptRequest(request, 'answer') expect(await pending).toBe('answer') }) + it('publishes frozen favorites and ignores a settings save that keeps them (#810)', async () => { + const s = await setup(), shell = await import('./shell') + const settings = () => s.useStore.getState().vaultSettings + s.useStore.setState({ vaultSettings: { ...settings(), favorites: ['inbox/One.md', 'inbox:Work'] } }) + const first = shell.getShellSnapshot(), listener = vi.fn() + const dispose = shell.subscribeShell(listener) + expect(first.favorites).toEqual(['inbox/One.md', 'inbox:Work']) + expect(() => (first.favorites as string[]).push('wrong')).toThrow() + // A folder color save rebuilds vault settings, favorites array included. + s.useStore.setState({ vaultSettings: { ...settings(), favorites: [...settings().favorites], folderColors: { 'inbox:Work': 'red' } } }) + expect(shell.getShellSnapshot()).toBe(first) + expect(listener).not.toHaveBeenCalled() + s.useStore.setState({ vaultSettings: { ...settings(), favorites: ['inbox:Work'] } }) + expect(shell.getShellSnapshot().favorites).toEqual(['inbox:Work']) + expect(listener).toHaveBeenCalledTimes(1) + dispose() + }) it('rechecks command availability at invocation instead of retaining stale closures', async () => { const s = await setup(), commands = await import('./commands') expect(await commands.runAppCommand('not-a-command')).toBe(false) diff --git a/packages/app-core/src/shell.ts b/packages/app-core/src/shell.ts index 275ff31c..450a5571 100644 --- a/packages/app-core/src/shell.ts +++ b/packages/app-core/src/shell.ts @@ -39,14 +39,29 @@ export interface ShellSnapshot { readonly canGoBack: boolean readonly canGoForward: boolean readonly noteSortOrder: NoteSortOrder + /** + * The vault's Favorites in display order, as stored in vault settings: a + * note's path, or an opaque key for a favorited folder. `includes(path)` + * answers whether a note is a favorite; toggle through + * `requestToggleNoteFavorite`. (#810) + */ + readonly favorites: readonly string[] } let notesSource: readonly NoteMeta[] | undefined let notesLayout = '' let notes: readonly ShellNote[] = Object.freeze([]) let vault: ShellSnapshot['vault'] = null +let favorites: readonly string[] = Object.freeze([]) let snapshot: ShellSnapshot | undefined +// Every settings save rebuilds the favorites array, so compare contents: +// a saved folder color must not wake shell subscribers over unchanged +// favorites. +function sameStrings(a: readonly string[], b: readonly string[]): boolean { + return a.length === b.length && a.every((value, index) => value === b[index]) +} + /** Read frozen shell metadata, without note bodies, credentials, or mutable store values. */ export function getShellSnapshot(): ShellSnapshot { const state = useStore.getState() @@ -87,6 +102,9 @@ export function getShellSnapshot(): ShellSnapshot { temporary: state.vault.temporary }) } + if (!sameStrings(favorites, settings.favorites)) { + favorites = Object.freeze([...settings.favorites]) + } const next: ShellSnapshot = { vault, notes, @@ -99,7 +117,8 @@ export function getShellSnapshot(): ShellSnapshot { : (notes.find((note) => note.path === state.selectedPath) ?? null), canGoBack: state.noteBackstack.length > 0, canGoForward: state.noteForwardstack.length > 0, - noteSortOrder: state.noteSortOrder + noteSortOrder: state.noteSortOrder, + favorites } if ( !snapshot || From 35a97c2644e649cecc64a7700176ed31851fa6f8 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 19:32:14 -0500 Subject: [PATCH 09/12] Fix(preview): a heading link followed in reading mode stays in reading mode (android#74) Reported from the Android app by @Alastor1991, where Reading is often the default view (ZenNotes/zennotesandroid#74): tap `[[Note#Heading]]` and the target note opened in edit mode with the cursor on the heading, keyboard and all. The desktop did the same. Every jump inside a note (`[[Note#Heading]]`, `[[Note#^block]]`, a same-note `[[#Heading]]`, a search hit, Ctrl+O / Ctrl+I) went through one path written for the editor: set the cursor, then switch the pane to edit so the cursor could show. A pane in reading mode now plans the jump itself. `planPreviewJump(jump, body)` in lib/preview-outline-jump.ts is the pure decision: a jump carrying `highlightLine` (a task opened from the Tasks view) still goes to edit, on purpose, since the highlight it paints on the source line is something only the editor can show; the `editorScrollMode: 'preserve'` shape that Ctrl+O / Ctrl+I (the phone's Back) send restores the reading view to the `previewScrollTop` it had when the user left, rather than jumping to a line; everything else becomes a line, taken from `editorSelectionAnchor`, and the rendered block for that source line is scrolled to the top of the reading view. A note the user last left in edit mode still opens in edit mode with the cursor on the heading; nothing changes for it. Two details keep the scroll honest. `previewShowsNote(el, path)` guards against scrolling the previous note's blocks: Preview.tsx stamps `data-note-path` on the article in the same step as the DOM swap, because the render is asynchronous and the effect can run before it. And a line plan whose blocks are not rendered yet is parked in `pendingPreviewLineRef` (the #543 cursor-line ref, renamed; same `onRendered` consumer), which is reset on note change before the pending-jump effect runs, so a line the previous note never got to render cannot fire on a later visit. Verified with 14 cases in preview-outline-jump.test.ts (heading, block and same-note links, a search hit, the task jump, the restore shape) and in the built app over CDP with isolated stores and default mode Reading: clicking `[[Reference#Deploy]]` from a scrolled Guide keeps reading mode with the Deploy heading at the top; Ctrl+O returns to Guide in reading mode within 40 px of where it was; a same-note `[[#Appendix]]` and a `[[Reference#^gotcha]]` block link both land the target at the top; a note remembered in edit mode still puts the cursor on the heading. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 --- .../app-core/src/components/EditorPane.tsx | 64 +++++++++++--- packages/app-core/src/components/Preview.tsx | 5 ++ .../src/lib/preview-outline-jump.test.ts | 83 +++++++++++++++++++ .../app-core/src/lib/preview-outline-jump.ts | 45 ++++++++++ 4 files changed, 184 insertions(+), 13 deletions(-) diff --git a/packages/app-core/src/components/EditorPane.tsx b/packages/app-core/src/components/EditorPane.tsx index 38d3b13c..1b2fdd5e 100644 --- a/packages/app-core/src/components/EditorPane.tsx +++ b/packages/app-core/src/components/EditorPane.tsx @@ -216,7 +216,9 @@ import { findRenderedHeadingForOutlineLine, nextOutlinePreviewSyncLockUntil, outlineHeadingTextOffset, + planPreviewJump, previewScrollTopForHeading, + previewShowsNote, scrollTopForElementRelativeTop, scrollTopForScrollRatio, shouldSyncPreviewFromEditorViewport @@ -1027,10 +1029,11 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { // lets us tell our own restore scroll apart from a user scroll, so we never // yank a reader who scrolled during the render window. const previewRestoreTargetRef = useRef<{ path: string; top: number } | null>(null) - // Set when the user switches Edit/Split → Preview: the reading view opens on - // the line the cursor was on, instead of the top of the note. Applied (and - // cleared) once the preview has rendered blocks to anchor against. (#543) - const pendingPreviewCursorLineRef = useRef<{ path: string; line: number } | null>(null) + // A source line the reading view should open on once it has rendered blocks + // to anchor against: the cursor's line when the user switches Edit/Split → + // Preview (#543), or the target of a heading/block link followed while + // reading (android#74). Applied and cleared from `onRendered`. + const pendingPreviewLineRef = useRef<{ path: string; line: number } | null>(null) const lastProgrammaticPreviewTopRef = useRef(null) const lastRestoredPathRef = useRef(null) const vimCompartmentRef = useRef(null) @@ -1203,7 +1206,7 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { view && viewPathRef.current === activeTab ) { - pendingPreviewCursorLineRef.current = { + pendingPreviewLineRef.current = { path: activeTab, line: view.state.doc.lineAt(view.state.selection.main.head).number } @@ -1460,12 +1463,13 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { return } // A mode switch out of editing carries the cursor's line into the - // reading view; the live editing position outranks a remembered - // preview offset from an earlier visit. (#543) - const cursorTarget = pendingPreviewCursorLineRef.current - if (cursorTarget && cursorTarget.path === content?.path) { - if (scrollPreviewToSourceLine(cursorTarget.line)) { - pendingPreviewCursorLineRef.current = null + // reading view, and a heading or block link followed while reading + // carries its target; either outranks a remembered preview offset from + // an earlier visit. (#543, android#74) + const lineTarget = pendingPreviewLineRef.current + if (lineTarget && lineTarget.path === content?.path) { + if (scrollPreviewToSourceLine(lineTarget.line)) { + pendingPreviewLineRef.current = null previewRestoreTargetRef.current = null return } @@ -1498,6 +1502,10 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { useEffect(() => { pendingPreviewOutlineJumpLineRef.current = null + // A line the previous note never got to render must not fire on a later + // visit. Declared before the pending-jump effect, so a jump that opens a + // note in reading mode still sets its line after this reset. + pendingPreviewLineRef.current = null outlinePreviewSyncLockUntilRef.current = 0 }, [content?.path]) @@ -2604,7 +2612,29 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { if (!isActive) return if (!content || !pendingJumpLocation || pendingJumpLocation.path !== content.path) return if (mode === 'preview') { - applyPaneMode('edit') + // Reading mode stays reading mode: a heading or block link, a search hit + // and Ctrl+O land the rendered preview on the spot instead of dropping + // the reader into the editor. Only a task jump still needs the editor, + // for the highlight it paints on the line. (android#74) + const plan = planPreviewJump(pendingJumpLocation, content.body) + if (plan.kind === 'edit') { + applyPaneMode('edit') + return + } + const previewEl = previewScrollRef.current + const rendered = previewShowsNote(previewEl, content.path) + if (plan.kind === 'restore') { + previewRestoreTargetRef.current = { path: content.path, top: plan.top } + if (rendered && previewEl) { + previewEl.scrollTop = plan.top + lastProgrammaticPreviewTopRef.current = previewEl.scrollTop + } + } else if (rendered && scrollPreviewToSourceLine(plan.line)) { + previewRestoreTargetRef.current = null + } else { + pendingPreviewLineRef.current = { path: content.path, line: plan.line } + } + clearPendingJumpLocation() return } const raf = requestAnimationFrame(() => { @@ -2649,7 +2679,15 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { clearPendingJumpLocation() }) return () => cancelAnimationFrame(raf) - }, [applyPaneMode, isActive, mode, content?.path, clearPendingJumpLocation, pendingJumpLocation]) + }, [ + applyPaneMode, + isActive, + mode, + content?.path, + clearPendingJumpLocation, + pendingJumpLocation, + scrollPreviewToSourceLine + ]) useEffect(() => { return () => { diff --git a/packages/app-core/src/components/Preview.tsx b/packages/app-core/src/components/Preview.tsx index 31315427..ebaa663d 100644 --- a/packages/app-core/src/components/Preview.tsx +++ b/packages/app-core/src/components/Preview.tsx @@ -860,6 +860,11 @@ export const Preview = memo(function Preview({ // not found" and zero-size boards (#68). Mermaid renders to inline SVG, so // it is safe to render in the detached buffer above. root.replaceChildren(...Array.from(stage.childNodes)); + // Which note these blocks belong to. Until this swap the article still + // shows the previous note, and a jump that lands while a render is in + // flight must wait for `onRendered` instead of scrolling against the + // wrong blocks (see previewShowsNote). + root.dataset.notePath = notePath; await renderDiagrams(root, { themeKey: diagramTheme.key, expanded: false }); if (cancelled) return; // Typst math (a no-op when the KaTeX renderer is active, since it emits no diff --git a/packages/app-core/src/lib/preview-outline-jump.test.ts b/packages/app-core/src/lib/preview-outline-jump.test.ts index f635e798..bc85ac18 100644 --- a/packages/app-core/src/lib/preview-outline-jump.test.ts +++ b/packages/app-core/src/lib/preview-outline-jump.test.ts @@ -6,7 +6,9 @@ import { findRenderedHeadingForOutlineLine, nextOutlinePreviewSyncLockUntil, outlineHeadingTextOffset, + planPreviewJump, previewScrollTopForHeading, + previewShowsNote, scrollTopForElementRelativeTop, scrollTopForScrollRatio, shouldSyncPreviewAfterMarkdownSettles, @@ -99,3 +101,84 @@ describe('preview outline jump helpers', () => { expect(shouldSyncPreviewFromEditorViewport('preview', true, false, false)).toBe(false) }) }) + +describe('planPreviewJump (a jump landing in a pane that is reading)', () => { + const body = '# Intro\n\nSome text.\n\n## Target\n\nMore text. ^quote\n' + const targetHeadingFrom = body.indexOf('## Target') + const blockFrom = body.indexOf('More text.') + + it('lands a [[Note#Heading]] jump on the heading line and stays in reading mode', () => { + expect( + planPreviewJump( + { editorSelectionAnchor: targetHeadingFrom, previewScrollTop: 0, editorScrollMode: 'start' }, + body + ) + ).toEqual({ kind: 'line', line: 5 }) + }) + + it('lands a ^block jump and a search hit on the block that holds the offset', () => { + expect( + planPreviewJump( + { editorSelectionAnchor: blockFrom, previewScrollTop: 0, editorScrollMode: 'start' }, + body + ) + ).toEqual({ kind: 'line', line: 7 }) + // A search hit points into the middle of a line, not at its start. + expect( + planPreviewJump( + { editorSelectionAnchor: blockFrom + 5, previewScrollTop: 0, editorScrollMode: 'center' }, + body + ) + ).toEqual({ kind: 'line', line: 7 }) + // An offset past the end (the note shrank) clamps to the last line. + expect( + planPreviewJump( + { editorSelectionAnchor: body.length + 40, previewScrollTop: 0, editorScrollMode: 'center' }, + body + ) + ).toEqual({ kind: 'line', line: 8 }) + }) + + it('puts the reading view back where it was for a Ctrl+O / Ctrl+I jump', () => { + expect( + planPreviewJump( + { editorSelectionAnchor: 900, previewScrollTop: 412, editorScrollMode: 'preserve' }, + body + ) + ).toEqual({ kind: 'restore', top: 412 }) + // A location captured before scroll modes existed carries no mode: it is + // a history entry, so it restores instead of scrolling to a line. + expect( + planPreviewJump({ editorSelectionAnchor: 900, previewScrollTop: -3 }, body) + ).toEqual({ kind: 'restore', top: 0 }) + }) + + it('still hands a task jump to the editor, which owns the line highlight', () => { + expect( + planPreviewJump( + { + editorSelectionAnchor: blockFrom, + previewScrollTop: 0, + editorScrollMode: 'center', + highlightLine: true + }, + body + ) + ).toEqual({ kind: 'edit' }) + }) +}) + +describe('previewShowsNote', () => { + it('is true only when the rendered article carries the note path', () => { + const scroller = document.createElement('div') + const article = document.createElement('article') + article.setAttribute('data-preview-content', '') + scroller.appendChild(article) + + expect(previewShowsNote(scroller, 'inbox/A.md')).toBe(false) + article.dataset.notePath = 'inbox/A.md' + expect(previewShowsNote(scroller, 'inbox/A.md')).toBe(true) + expect(previewShowsNote(scroller, 'inbox/B.md')).toBe(false) + expect(previewShowsNote(null, 'inbox/A.md')).toBe(false) + }) +}) diff --git a/packages/app-core/src/lib/preview-outline-jump.ts b/packages/app-core/src/lib/preview-outline-jump.ts index b5082a71..d2bc5b12 100644 --- a/packages/app-core/src/lib/preview-outline-jump.ts +++ b/packages/app-core/src/lib/preview-outline-jump.ts @@ -1,6 +1,51 @@ +import { lineOfOffset } from '@shared/note-comments' import type { OutlineItem } from './outline' const RENDERED_HEADING_SELECTOR = 'h1, h2, h3, h4, h5, h6' + +/** The slice of a store `NoteJumpLocation` the reading view needs. */ +export interface PreviewJumpRequest { + editorSelectionAnchor: number + previewScrollTop: number + editorScrollMode?: 'preserve' | 'center' | 'start' + highlightLine?: boolean +} + +/** + * What a pane in reading mode does with a pending jump. + * + * - `edit`: the jump exists for the editor. A task jump paints a highlight on + * the source line, which only the editor can show. + * - `restore`: Ctrl+O / Ctrl+I. The location remembers how far the reading + * view was scrolled when the user left, so put it back there. + * - `line`: a `[[Note#Heading]]`, `[[Note#^block]]` or search hit. Land the + * rendered block for that source line at the top, and stay in reading mode. + */ +export type PreviewJumpPlan = + | { kind: 'edit' } + | { kind: 'restore'; top: number } + | { kind: 'line'; line: number } + +export function planPreviewJump(jump: PreviewJumpRequest, body: string): PreviewJumpPlan { + if (jump.highlightLine) return { kind: 'edit' } + if ((jump.editorScrollMode ?? 'preserve') === 'preserve') { + return { kind: 'restore', top: Math.max(0, jump.previewScrollTop) } + } + return { kind: 'line', line: lineOfOffset(body, jump.editorSelectionAnchor) } +} + +/** + * Whether the reading view's DOM is the render of `notePath`. The preview + * renders asynchronously (diagrams first, then one DOM swap), so right after a + * note opens the article still shows the previous note; scrolling against + * those blocks would land anywhere. `Preview` stamps the path on the article + * in the same step as the DOM swap. + */ +export function previewShowsNote(previewScrollEl: ParentNode | null, notePath: string): boolean { + const article = previewScrollEl?.querySelector('[data-preview-content]') + return article?.dataset.notePath === notePath +} + const ATX_HEADING_TEXT_OFFSET_RE = /^(#{1,6})[ \t]+/ export function outlineHeadingTextOffset(lineText: string): number { From ed6e080142a7753334acbbf1dc0f86e5f922b323 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Fri, 18 Sep 2026 19:32:14 -0500 Subject: [PATCH 10/12] Fix(templates): the template editor stacks its panes on narrow screens (android#78) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @uNyanda (ZenNotes/zennotesandroid#78): Settings → Templates → edit showed the editor and the preview side by side at any width, which on a phone left each about half a screen wide, too narrow to write in or to read. The two panes now follow the viewport. From 768 px up (the app's `md` breakpoint) they share a row as before, each 60vh tall. Below it the editor sits on top at 38vh and the preview under it at 22vh, each scrolling on its own, and the row of variable chips scrolls sideways (`flex-nowrap overflow-x-auto`) instead of wrapping into several lines that pushed the editor down. Viewport heights are the accepted exception to the no-arbitrary-values rule; no new tokens. The dialog itself, its header, name field and buttons are unchanged, so a narrow desktop window gets the stacked layout too. Verified in the built app over CDP, reading the layout from the DOM at 1280 px (two grid columns, 60vh each, chips wrapping) and at 390 px (one column, editor 38vh over preview 22vh, chips in one scrolling row, dialog still centered). Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a0b4e0-a062-700d-8add-505c3d057740 --- .../src/components/TemplateEditorModal.tsx | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/app-core/src/components/TemplateEditorModal.tsx b/packages/app-core/src/components/TemplateEditorModal.tsx index ee938c35..69f4e7d5 100644 --- a/packages/app-core/src/components/TemplateEditorModal.tsx +++ b/packages/app-core/src/components/TemplateEditorModal.tsx @@ -44,8 +44,10 @@ category: Custom {{cursor}} ` +// The editor fills whatever height its container gives it: the container is +// what changes between the side-by-side and the stacked layout below. const editorTheme = EditorView.theme({ - '&': { height: '60vh', fontSize: '13px', backgroundColor: 'transparent' }, + '&': { height: '100%', fontSize: '13px', backgroundColor: 'transparent' }, '&.cm-focused': { outline: 'none' }, '.cm-scroller': { fontFamily: 'var(--font-mono, ui-monospace, monospace)', @@ -224,26 +226,35 @@ export function TemplateEditorModal({ {vimMode ? 'Vim · ' : ''}YAML frontmatter + markdown body -
-
-
+ {/* Side by side from the md breakpoint up; below it (phones, a narrow + window) the two halves would each be too slim to use, so the editor + sits above the preview and takes the larger share of the height. + Decided by the viewport, not the platform. (android#78) */} +
+
+
{preview || Preview…}
-
- Variables + {/* One scrolling row of chips when stacked: wrapped, ten chips would + eat the height the editor just gained. */} +
+ Variables {TEMPLATE_VARIABLES.map((variable) => ( ))} - — or type {'{{'} to autocomplete + or type {'{{'} to autocomplete