diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 8ab6b3fa..cdd52695 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/desktop", "productName": "ZenNotes", - "version": "2.54.1", + "version": "2.55.0", "description": "ZenNotes desktop shell", "private": true, "main": "./out/main/index.js", diff --git a/apps/desktop/src/main/app-config.ts b/apps/desktop/src/main/app-config.ts index ea4c4030..aefa6dd0 100644 --- a/apps/desktop/src/main/app-config.ts +++ b/apps/desktop/src/main/app-config.ts @@ -327,6 +327,12 @@ const SCALAR_FIELDS: Partial> = { comment: 'opt in to the Workflows view, its sidebar row, command, and leader shortcut (off by default)' }, + workflowEventTriggers: { + section: 'view', + tomlKey: 'workflow_event_triggers', + comment: + 'let active workflows with "trigger: on " run on this device when you save, create, move or tag a note (on by default; needs workflows_enabled)' + }, assetSortOrder: { section: 'view', tomlKey: 'asset_sort_order', diff --git a/apps/desktop/src/main/note-creation-metadata.ts b/apps/desktop/src/main/note-creation-metadata.ts index f7ec1c16..46e971b0 100644 --- a/apps/desktop/src/main/note-creation-metadata.ts +++ b/apps/desktop/src/main/note-creation-metadata.ts @@ -5,12 +5,16 @@ import { randomInt } from 'node:crypto' const metadataDirectory = '.zennotes/note-metadata' const metadataSuffix = '.metadata.json' +export function noteMetadataRoot(root: string): string { + return path.resolve(root, metadataDirectory) +} + export async function noteMetadataPath( root: string, rel: string, directory = false, ): Promise { - const base = path.resolve(root, metadataDirectory) + const base = noteMetadataRoot(root) const target = path.resolve(base, rel + (directory ? '' : metadataSuffix)) if (target === base || !target.startsWith(base + path.sep)) throw new Error('Path escapes note metadata') @@ -124,52 +128,3 @@ export async function removeNoteCreation( recursive: directory, }) } - -export async function moveWithCreationMetadata( - root: string, - from: string, - to: string, - directory = false, -): Promise { - if (from === to) return - const source = await noteMetadataPath( - root, - path.relative(root, from), - directory, - ) - const target = await noteMetadataPath( - root, - path.relative(root, to), - directory, - ) - const exists = async (abs: string): Promise => - fs.lstat(abs).then( - () => true, - (error: NodeJS.ErrnoException) => { - if (error.code === 'ENOENT') return false - throw error - }, - ) - const hasMetadata = await exists(source) - if (await exists(target)) - throw new Error(`Destination metadata already exists: ${target}`) - if (hasMetadata) { - await fs.mkdir(path.dirname(target), { recursive: true }) - await fs.rename(source, target) - } - try { - await fs.rename(from, to) - } catch (error) { - if (hasMetadata) { - try { - await fs.rename(target, source) - } catch (rollback) { - throw new AggregateError( - [error, rollback], - 'Note move and metadata rollback failed; reload before editing', - ) - } - } - throw error - } -} diff --git a/apps/desktop/src/main/note-sidecars.test.ts b/apps/desktop/src/main/note-sidecars.test.ts new file mode 100644 index 00000000..43df2013 --- /dev/null +++ b/apps/desktop/src/main/note-sidecars.test.ts @@ -0,0 +1,177 @@ +import { mkdir, mkdtemp, readFile, readlink, rm, stat, symlink, writeFile } from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const environment = vi.hoisted(() => ({ userData: '' })) +vi.mock('electron', () => ({ + app: { + isPackaged: false, + getPath: () => { + if (!environment.userData) throw new Error('Vault test has not initialized app data') + return environment.userData + } + } +})) + +import * as desktop from './vault' +import * as mcp from '../mcp/vault-ops' + +const roots: string[] = [] + +async function makeVault(): Promise { + const root = await mkdtemp(path.join(os.tmpdir(), 'zn-note-sidecars-')) + roots.push(root) + environment.userData = path.join(root, '.test-app') + await mkdir(environment.userData, { recursive: true }) + vi.stubEnv('ZENNOTES_USER_DATA_PATH', environment.userData) + vi.stubEnv('ZENNOTES_CONFIG_DIR', environment.userData) + return root +} + +afterEach(async () => { + for (const root of roots.splice(0)) { + desktop.invalidateNoteMetaCache(root) + desktop.invalidateVaultSettingsCache(root) + await rm(root, { recursive: true, force: true }) + } + environment.userData = '' + vi.unstubAllEnvs() +}) + +function comment(notePath: string, body: string) { + return { notePath, anchorStart: 0, anchorEnd: 0, anchorText: '', id: body, body, createdAt: 1, updatedAt: 1 } +} + +function commentsFile(root: string, notePath: string): string { + return path.join(root, '.zennotes', 'comments', `${notePath}.comments.json`) +} + +async function missing(abs: string): Promise { + return stat(abs).then( + () => false, + (error: NodeJS.ErrnoException) => error.code === 'ENOENT' + ) +} + +// The desktop and the MCP server move notes through the same shared core +// (note-sidecars.ts), so they must agree on where a note's discussion goes. +// Before, the MCP server moved the Markdown alone and left the comments at +// the old name, where the next note given that name took them over. +const clients = [ + ['desktop', desktop], + ['MCP', mcp] +] as const + +describe.each(clients)('%s: a note keeps its comments', (_name, client) => { + async function seed(root: string, notePath: string): Promise { + await mkdir(path.dirname(path.join(root, notePath)), { recursive: true }) + await client.writeNote(root, notePath, '# Note\n\nBody.\n') + await client.writeNoteComments(root, notePath, [comment(notePath, 'Keep this discussion')]) + } + + it('through a rename', async () => { + const root = await makeVault() + await seed(root, 'inbox/One.md') + + await client.renameNote(root, 'inbox/One.md', 'Two') + + expect((await client.readNoteComments(root, 'inbox/Two.md')).map((c) => c.body)).toEqual(['Keep this discussion']) + expect(await missing(commentsFile(root, 'inbox/One.md'))).toBe(true) + }) + + it('through a move to another folder', async () => { + const root = await makeVault() + await seed(root, 'inbox/One.md') + + const moved = await client.moveNote(root, 'inbox/One.md', 'inbox', 'Work') + + expect(moved.path).toBe('inbox/Work/One.md') + expect((await client.readNoteComments(root, moved.path)).map((c) => c.body)).toEqual(['Keep this discussion']) + expect(await missing(commentsFile(root, 'inbox/One.md'))).toBe(true) + }) + + it('through Trash and back', async () => { + const root = await makeVault() + await seed(root, 'inbox/One.md') + + const trashed = await client.moveToTrash(root, 'inbox/One.md') + expect((await client.readNoteComments(root, trashed.path)).map((c) => c.body)).toEqual(['Keep this discussion']) + const restored = await client.restoreFromTrash(root, trashed.path) + + expect(restored.path).toBe('inbox/One.md') + expect((await client.readNoteComments(root, restored.path)).map((c) => c.body)).toEqual(['Keep this discussion']) + expect(await missing(commentsFile(root, trashed.path))).toBe(true) + }) + + it('through a folder rename', async () => { + const root = await makeVault() + await seed(root, 'inbox/Work/One.md') + + await client.renameFolder(root, 'inbox', 'Work', 'Projects') + + expect((await client.readNoteComments(root, 'inbox/Projects/One.md')).map((c) => c.body)).toEqual(['Keep this discussion']) + expect(await missing(commentsFile(root, 'inbox/Work/One.md'))).toBe(true) + }) + + it('refuses a rename onto an earlier note’s comments, and names the file', async () => { + const root = await makeVault() + await seed(root, 'inbox/One.md') + await client.writeNoteComments(root, 'inbox/Two.md', [comment('inbox/Two.md', 'An earlier discussion')]) + + await expect(client.renameNote(root, 'inbox/One.md', 'Two')).rejects.toThrow( + 'Comments from an earlier note named “Two” are still in .zennotes/comments/inbox/Two.md.comments.json' + ) + + expect(await readFile(path.join(root, 'inbox/One.md'), 'utf8')).toContain('Body.') + expect((await client.readNoteComments(root, 'inbox/One.md')).map((c) => c.body)).toEqual(['Keep this discussion']) + expect((await client.readNoteComments(root, 'inbox/Two.md')).map((c) => c.body)).toEqual(['An earlier discussion']) + }) + + it('and a deleted note takes them with it, so a new note of that name starts clean', async () => { + const root = await makeVault() + await seed(root, 'inbox/One.md') + + await client.deleteNote(root, 'inbox/One.md') + await client.writeNote(root, 'inbox/One.md', '# One again\n') + + expect(await client.readNoteComments(root, 'inbox/One.md')).toEqual([]) + }) + + it('Empty Trash takes the trashed notes’ comments', async () => { + const root = await makeVault() + await seed(root, 'inbox/One.md') + const trashed = await client.moveToTrash(root, 'inbox/One.md') + expect(await missing(commentsFile(root, trashed.path))).toBe(false) + + await client.emptyTrash(root) + + expect(await missing(commentsFile(root, trashed.path))).toBe(true) + }) + + it('a note that is a relative link keeps pointing at its file when moved deeper', async () => { + const root = await makeVault() + // Inside inbox, so the vault stays in its default layout. + await mkdir(path.join(root, 'inbox', 'sources'), { recursive: true }) + await writeFile(path.join(root, 'inbox', 'sources', 'Real.md'), '# Real\n\nElsewhere.\n') + await symlink('sources/Real.md', path.join(root, 'inbox', 'Rel.md')) + + const moved = await client.moveNote(root, 'inbox/Rel.md', 'inbox', 'Topics') + + // Moved verbatim, `sources/Real.md` from inbox/Topics would name nothing. + expect(moved.path).toBe('inbox/Topics/Rel.md') + expect(await readlink(path.join(root, 'inbox', 'Topics', 'Rel.md'))).toBe(path.join('..', 'sources', 'Real.md')) + expect(await readFile(path.join(root, 'inbox', 'Topics', 'Rel.md'), 'utf8')).toBe('# Real\n\nElsewhere.\n') + }) + + it('deleting a folder takes its notes’ comments', async () => { + const root = await makeVault() + await seed(root, 'inbox/Work/One.md') + await writeFile(path.join(root, 'inbox/Keep.md'), '# Keep\n') + + await client.deleteFolder(root, 'inbox', 'Work') + + expect(await missing(commentsFile(root, 'inbox/Work/One.md'))).toBe(true) + expect(await readFile(path.join(root, 'inbox/Keep.md'), 'utf8')).toBe('# Keep\n') + }) +}) diff --git a/apps/desktop/src/main/note-sidecars.ts b/apps/desktop/src/main/note-sidecars.ts new file mode 100644 index 00000000..58c3b8a3 --- /dev/null +++ b/apps/desktop/src/main/note-sidecars.ts @@ -0,0 +1,196 @@ +// A note's two app-owned sidecars, and moving a note (or a folder) together +// with them. Comments live at `.zennotes/comments/.comments.json` and +// the creation date at `.zennotes/note-metadata/.metadata.json` (see +// note-creation-metadata.ts). Desktop main, the MCP server and the workflow +// applier all move notes, and the MCP server cannot import vault.ts (it pulls +// in Electron), so this stays free of Electron and is the one place that knows +// how a note's sidecars travel with it. +import { promises as fs } from 'node:fs' +import { randomUUID } from 'node:crypto' +import path from 'node:path' +import { NOTE_COMMENTS_DIR, NOTE_COMMENTS_SUFFIX } from '@shared/note-comments' +import { noteMetadataPath, removeNoteCreation } from './note-creation-metadata' + +const INTERNAL_VAULT_DIR = '.zennotes' + +function toPosix(rel: string): string { + return rel.split(path.sep).join('/') +} + +function resolveSafe(root: string, rel: string): string { + const abs = path.resolve(root, rel) + const rootAbs = path.resolve(root) + if (abs !== rootAbs && !abs.startsWith(rootAbs + path.sep)) { + throw new Error(`Path escapes vault: ${rel}`) + } + return abs +} + +export function noteCommentsRoot(root: string): string { + return path.join(root, INTERNAL_VAULT_DIR, NOTE_COMMENTS_DIR) +} + +export function noteCommentsPath(root: string, rel: string): string { + return resolveSafe(noteCommentsRoot(root), `${toPosix(rel)}${NOTE_COMMENTS_SUFFIX}`) +} + +/** The words a rename or move shows when an earlier note's comments sit on + * the destination name. Shared so every writer refuses the same way. */ +export function leftoverCommentsMessage(root: string, noteAbs: string): string { + const comments = noteCommentsPath(root, toPosix(path.relative(root, noteAbs))) + return `Comments from an earlier note named “${path.parse(noteAbs).name}” are still in ${toPosix(path.relative(root, comments))}. Move or delete that file to use this name.` +} + +/** + * After a rename from `from` to `to`, keep a symlink pointing where it did. + * + * A link's relative text is read from the link's own folder, so a link moved + * verbatim to another depth names a different file, or none: `../sources/X.md` + * from `inbox/` becomes `inbox/sources/X.md` from `inbox/Topics/`. That is what + * `mv` and Finder do, and it is wrong for a note, which the user expects to + * keep reading the same file wherever it is filed. The text is re-based on the + * target it named from the old folder, which is also what makes a rename back + * (a rollback, an undo) land on the original text again. An absolute text + * needs nothing; links inside a moved folder move with their folder and keep + * resolving unless they pointed out of it, which this does not follow. + */ +export async function rebaseMovedLink(from: string, to: string): Promise { + let stats + try { + stats = await fs.lstat(to) + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return + throw error + } + if (!stats.isSymbolicLink()) return + const text = await fs.readlink(to) + if (path.isAbsolute(text)) return + const fromDir = path.dirname(from) + const toDir = path.dirname(to) + if (fromDir === toDir) return + const next = path.relative(toDir, path.resolve(fromDir, text)) + if (next === text) return + // A new link beside it, renamed over: no moment without a link at `to`. + const temporary = `${to}_relink_tmp_${randomUUID()}` + await fs.symlink(next, temporary) + try { + await fs.rename(temporary, to) + } catch (error) { + await fs.rm(temporary, { force: true }).catch(() => undefined) + throw error + } +} + +async function renameDirectory(from: string, to: string): Promise { + if (from === to) return + if (from.toLowerCase() !== to.toLowerCase()) { + await fs.rename(from, to) + await rebaseMovedLink(from, to) + return + } + const temporary = `${from}_rename_tmp_${randomUUID()}` + await fs.rename(from, temporary) + try { + await fs.rename(temporary, to) + } catch (error) { + try { + await fs.rename(temporary, from) + } catch (rollbackError) { + throw new AggregateError( + [error, rollbackError], + 'FOLDER_STATE_UNCERTAIN: Folder change could not be rolled back; reload the vault before editing' + ) + } + throw error + } +} + +export async function pathExists(abs: string): Promise { + return fs.lstat(abs).then( + () => true, + (error: NodeJS.ErrnoException) => { + if (error.code === 'ENOENT') return false + throw error + } + ) +} + +/** + * Move one note with its comments and its creation date. A creation date + * already waiting at the destination with no note beside it belongs to + * nobody: the note that owned it was moved or deleted outside ZenNotes (a + * file manager, git, sync, an older ZenNotes). It is discarded, the way + * `createNote` already discards it, or every rename and move onto that name + * was refused for good, silently (#839). Leftover comments still refuse: + * taking over another note's discussion and deleting it are both wrong, so + * the error names the file to move aside. + */ +export async function relocateNote( + root: string, + fromRel: string, + toAbs: string, + persistSettings: () => Promise +): Promise { + const toRel = toPosix(path.relative(root, toAbs)) + const toComments = noteCommentsPath(root, toRel) + if (!(await pathExists(toAbs))) { + if (await pathExists(toComments)) throw new Error(leftoverCommentsMessage(root, toAbs)) + await removeNoteCreation(root, toRel) + } + await relocateFolderTrees( + [ + [resolveSafe(root, fromRel), toAbs], + [noteCommentsPath(root, fromRel), toComments], + [await noteMetadataPath(root, fromRel), await noteMetadataPath(root, toRel)] + ], + persistSettings + ) +} + +/** Move content and its parallel comments together, retaining the originals on failure. */ +export async function relocateFolderTrees( + moves: Array<[string, string]>, + persistSettings: () => Promise +): Promise { + const present: Array<[string, string]> = [] + for (const [from, to] of moves) { + let source + try { + source = await fs.stat(from) + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error + } + try { + const target = await fs.stat(to) + if (!source || source.ino !== target.ino || source.dev !== target.dev) + throw new Error('The destination folder or its comments already exist') + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error + } + if (source) present.push([from, to]) + } + const moved: Array<[string, string]> = [] + try { + for (const [from, to] of present) { + await fs.mkdir(path.dirname(to), { recursive: true }) + await renameDirectory(from, to) + moved.push([from, to]) + } + await persistSettings() + } catch (error) { + const failures: unknown[] = [error] + for (const [from, to] of moved.reverse()) { + try { + await renameDirectory(to, from) + } catch (rollbackError) { + failures.push(rollbackError) + } + } + if (failures.length > 1) + throw new AggregateError( + failures, + 'FOLDER_STATE_UNCERTAIN: Folder change could not be rolled back; reload the vault before editing' + ) + throw error + } +} diff --git a/apps/desktop/src/main/vault-creation-metadata.test.ts b/apps/desktop/src/main/vault-creation-metadata.test.ts index 9c924327..44d0124c 100644 --- a/apps/desktop/src/main/vault-creation-metadata.test.ts +++ b/apps/desktop/src/main/vault-creation-metadata.test.ts @@ -21,6 +21,7 @@ import { invalidateNoteMetaCache, invalidateVaultSettingsCache, listNotes, + moveNote, moveToTrash, readNote, renameFolder, @@ -177,6 +178,26 @@ describe('portable note creation metadata across desktop vault operations', () = await expect(readFile(metadataPath(root, trashPath))).rejects.toMatchObject({ code: 'ENOENT' }) }) + it('moves and trashes a note onto stale destination dates, keeping its own (#839)', async () => { + const root = await makeVault() + await seedNote(root, 'inbox/Original.md') + const stale = JSON.stringify({ version: 1, createdAt: ORIGINAL_CREATED_AT + 1000 }) + for (const leftover of ['inbox/Work/Original.md', 'trash/Work/Original.md']) { + await mkdir(path.dirname(metadataPath(root, leftover)), { recursive: true }) + await writeFile(metadataPath(root, leftover), stale) + } + + const moved = await moveNote(root, 'inbox/Original.md', 'inbox', 'Work') + const trashed = await moveToTrash(root, moved.path) + + expect(moved.path).toBe('inbox/Work/Original.md') + expect(moved.createdAt).toBe(ORIGINAL_CREATED_AT) + expect(trashed.path).toBe('trash/Work/Original.md') + expect(trashed.createdAt).toBe(ORIGINAL_CREATED_AT) + expect(await readMetadata(root, trashed.path)).toEqual({ version: 1, createdAt: ORIGINAL_CREATED_AT }) + await expect(readFile(metadataPath(root, moved.path))).rejects.toMatchObject({ code: 'ENOENT' }) + }) + it('removes creation metadata on permanent deletion so a new note cannot inherit the old date', async () => { const root = await makeVault() const notePath = 'inbox/Original.md' @@ -255,7 +276,10 @@ describe.each(vaultClients)('%s creation metadata safety', (_name, client) => { } ) - it('refuses a note rename onto orphan destination metadata and preserves both dates', async () => { + // A date left behind by a note that was moved or deleted outside ZenNotes + // belongs to nobody. Refusing the rename over it blocked that name for good + // (#839); creating a note there already discards it. + it('renames a note onto a stale destination date, keeping its own date', async () => { const root = await makeVault() const sourcePath = 'inbox/Original.md' const targetPath = 'inbox/Destination.md' @@ -263,13 +287,28 @@ describe.each(vaultClients)('%s creation metadata safety', (_name, client) => { const orphanDate = ORIGINAL_CREATED_AT + 1000 await writeFile(metadataPath(root, targetPath), JSON.stringify({ version: 1, createdAt: orphanDate })) - await expect(client.renameNote(root, sourcePath, 'Destination')).rejects.toThrow() + await client.renameNote(root, sourcePath, 'Destination') - expect(await readFile(path.join(root, sourcePath))).toEqual(Buffer.from(ORIGINAL_BODY)) - expect(await readMetadata(root, sourcePath)).toEqual({ version: 1, createdAt: ORIGINAL_CREATED_AT }) - expect(await readMetadata(root, targetPath)).toEqual({ version: 1, createdAt: orphanDate }) - await expect(readFile(path.join(root, targetPath))).rejects.toMatchObject({ code: 'ENOENT' }) - expect((await client.readNote(root, sourcePath)).createdAt).toBe(ORIGINAL_CREATED_AT) + expect(await readFile(path.join(root, targetPath), 'utf8')).toContain('Keep **Markdown** and trailing spaces.') + expect(await readMetadata(root, targetPath)).toEqual({ version: 1, createdAt: ORIGINAL_CREATED_AT }) + expect((await client.readNote(root, targetPath)).createdAt).toBe(ORIGINAL_CREATED_AT) + await expect(readFile(path.join(root, sourcePath))).rejects.toMatchObject({ code: 'ENOENT' }) + await expect(readFile(metadataPath(root, sourcePath))).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('never hands a stale destination date to a renamed note that has none', async () => { + const root = await makeVault() + const sourcePath = 'inbox/Original.md' + const targetPath = 'inbox/Destination.md' + await mkdir(path.join(root, 'inbox'), { recursive: true }) + await writeFile(path.join(root, sourcePath), ORIGINAL_BODY) + await mkdir(path.dirname(metadataPath(root, targetPath)), { recursive: true }) + await writeFile(metadataPath(root, targetPath), JSON.stringify({ version: 1, createdAt: ORIGINAL_CREATED_AT })) + + await client.renameNote(root, sourcePath, 'Destination') + + expect(await readFile(path.join(root, targetPath), 'utf8')).toContain('Keep **Markdown** and trailing spaces.') + expect((await client.readNote(root, targetPath)).createdAt).not.toBe(ORIGINAL_CREATED_AT) }) it('refuses a folder move onto an orphan metadata tree without moving the source', async () => { diff --git a/apps/desktop/src/main/vault.test.ts b/apps/desktop/src/main/vault.test.ts index a64e11a5..397c3a3d 100644 --- a/apps/desktop/src/main/vault.test.ts +++ b/apps/desktop/src/main/vault.test.ts @@ -1566,6 +1566,29 @@ describe('note rename transaction', () => { expect((await readNoteComments(root,'inbox/Renamed.md'))[0].body).toBe('Keep destination') if (withComments) expect((await readNoteComments(root,'inbox/One.md'))[0].body).toBe('Source discussion') }) + + // The app shows these reasons in a toast now, so they have to read plainly (#839). + it('names the leftover comments file that blocks a rename', async () => { + const root = await makeTempDir('zennotes-note-rename-reason-') + await ensureVaultLayout(root) + await writeNote(root, 'inbox/One.md', 'Original.\n') + await writeNoteComments(root, 'inbox/Renamed.md', [{notePath:'inbox/Renamed.md',anchorStart:0,anchorEnd:0,anchorText:'',id:'destination', body:'Keep destination', createdAt:1, updatedAt:1}]) + await expect(renameNote(root, 'inbox/One.md', 'Renamed')).rejects.toThrow( + 'Comments from an earlier note named “Renamed” are still in .zennotes/comments/inbox/Renamed.md.comments.json' + ) + }) + + it('says when another note already has the name', async () => { + const root = await makeTempDir('zennotes-note-rename-taken-') + await ensureVaultLayout(root) + await writeNote(root, 'inbox/One.md', 'Original.\n') + await writeNote(root, 'inbox/Two.md', 'Other note.\n') + await expect(renameNote(root, 'inbox/One.md', 'Two')).rejects.toThrow( + 'A note named “Two” already exists in this folder' + ) + expect(await readFile(path.join(root, 'inbox/One.md'), 'utf8')).toBe('Original.\n') + expect(await readFile(path.join(root, 'inbox/Two.md'), 'utf8')).toBe('Other note.\n') + }) }) diff --git a/apps/desktop/src/main/vault.ts b/apps/desktop/src/main/vault.ts index 9c3cb375..3018d248 100644 --- a/apps/desktop/src/main/vault.ts +++ b/apps/desktop/src/main/vault.ts @@ -1,4 +1,10 @@ import { noteMetadataPath, readNoteCreatedAt, prepareNoteCreation, removeNoteCreation } from './note-creation-metadata' +import { + noteCommentsPath, + noteCommentsRoot, + relocateFolderTrees, + relocateNote +} from './note-sidecars' import { promises as fs, type Dirent } from 'node:fs' import { execFile, spawn } from 'node:child_process' import { randomUUID } from 'node:crypto' @@ -111,8 +117,6 @@ const DELETED_ASSET_META = '.zn-deleted.json' const VAULT_SETTINGS_FILE = 'vault.json' const NOTE_META_CACHE_FILE = 'note-meta-cache-v1.json' const NOTE_META_CACHE_VERSION = 3 -const NOTE_COMMENTS_DIR = 'comments' -const NOTE_COMMENTS_SUFFIX = '.comments.json' const RESERVED_ROOT_NAMES = new Set([...FOLDERS, ...ATTACHMENTS_DIRS, INTERNAL_VAULT_DIR]) // The subset that stays reserved however the system folders are remapped: // asset dirs and our own internal dir are never user note folders, while @@ -778,14 +782,6 @@ function noteMetaCachePath(root: string): string { return path.join(root, INTERNAL_VAULT_DIR, NOTE_META_CACHE_FILE) } -function noteCommentsRoot(root: string): string { - return path.join(root, INTERNAL_VAULT_DIR, NOTE_COMMENTS_DIR) -} - -function noteCommentsPath(root: string, rel: string): string { - return resolveSafe(noteCommentsRoot(root), `${toPosix(rel)}${NOTE_COMMENTS_SUFFIX}`) -} - /** Absolute path of a database's `.csv` data file (a normal vault file). */ export function databaseDataPath(root: string, rel: string): string { return resolveSafe(root, toPosix(rel)) @@ -3547,15 +3543,26 @@ export async function renameNote( const ext = isExcalidrawPath(abs) ? '.excalidraw' : '.md' const target = path.join(dir, `${trimmed}${ext}`) const willRename = target !== abs + if (willRename) { + // Said plainly, since the app shows this reason to whoever is renaming. A + // case-only rename finds the note itself on a case-insensitive disk. + const [source, taken] = await Promise.all([ + fs.stat(abs), + fs.stat(target).catch((error: NodeJS.ErrnoException) => { + if (error.code === 'ENOENT') return null + throw error + }) + ]) + if (taken && (taken.ino !== source.ino || taken.dev !== source.dev)) + throw new Error(`A note named “${trimmed}” already exists in this folder`) + } // Snapshot the vault before the rename so inbound [[wikilinks]] still // resolve to this note under its current name; we rewrite them afterwards. const notesBefore = willRename ? await listNotes(root) : [] - const nextRel = toPosix(path.relative(root, target)) let meta!: NoteMeta - await relocateFolderTrees( - [[abs, target], [noteCommentsPath(root, rel), noteCommentsPath(root, nextRel)], [await noteMetadataPath(root, rel), await noteMetadataPath(root, nextRel)]], - async () => { meta = await readMeta(root, target, folder) } - ) + await relocateNote(root, rel, target, async () => { + meta = await readMeta(root, target, folder) + }) invalidateNoteMetaCache(root, rel) invalidateNoteMetaCache(root, meta.path) invalidateVaultTextSearchCache(root) @@ -3628,12 +3635,10 @@ async function moveBetweenFolders( const destDir = subpath ? resolveSafe(targetRoot, subpath) : targetRoot await fs.mkdir(destDir, { recursive: true }) const destAbs = path.join(destDir, await uniqueFilename(destDir, filename)) - const nextRel = toPosix(path.relative(root, destAbs)) let meta!: NoteMeta - await relocateFolderTrees( - [[abs, destAbs], [noteCommentsPath(root, rel), noteCommentsPath(root, nextRel)], [await noteMetadataPath(root, rel), await noteMetadataPath(root, nextRel)]], - async () => { meta = await readMeta(root, destAbs, target) } - ) + await relocateNote(root, rel, destAbs, async () => { + meta = await readMeta(root, destAbs, target) + }) invalidateNoteMetaCache(root, rel) invalidateNoteMetaCache(root, meta.path) invalidateVaultTextSearchCache(root) @@ -3968,74 +3973,6 @@ export async function createFolder( await fs.mkdir(abs, { recursive: true }) } -async function renameDirectory(from: string, to: string): Promise { - if (from === to) return - if (from.toLowerCase() !== to.toLowerCase()) return fs.rename(from, to) - const temporary = `${from}_rename_tmp_${randomUUID()}` - await fs.rename(from, temporary) - try { - await fs.rename(temporary, to) - } catch (error) { - try { - await fs.rename(temporary, from) - } catch (rollbackError) { - throw new AggregateError( - [error, rollbackError], - 'FOLDER_STATE_UNCERTAIN: Folder change could not be rolled back; reload the vault before editing' - ) - } - throw error - } -} - -/** Move content and its parallel comments together, retaining the originals on failure. */ -async function relocateFolderTrees( - moves: Array<[string, string]>, - persistSettings: () => Promise -): Promise { - const present: Array<[string, string]> = [] - for (const [from, to] of moves) { - let source - try { - source = await fs.stat(from) - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error - } - try { - const target = await fs.stat(to) - if (!source || source.ino !== target.ino || source.dev !== target.dev) - throw new Error('The destination folder or its comments already exist') - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error - } - if (source) present.push([from, to]) - } - const moved: Array<[string, string]> = [] - try { - for (const [from, to] of present) { - await fs.mkdir(path.dirname(to), { recursive: true }) - await renameDirectory(from, to) - moved.push([from, to]) - } - await persistSettings() - } catch (error) { - const failures: unknown[] = [error] - for (const [from, to] of moved.reverse()) { - try { - await renameDirectory(to, from) - } catch (rollbackError) { - failures.push(rollbackError) - } - } - if (failures.length > 1) - throw new AggregateError( - failures, - 'FOLDER_STATE_UNCERTAIN: Folder change could not be rolled back; reload the vault before editing' - ) - throw error - } -} - /** Shared local folder move for ordinary folders and database containers. */ export async function renameFolderTrees( root: string, oldRelative: string, newRelative: string, @@ -4367,12 +4304,10 @@ export async function moveNote( await fs.mkdir(destDir, { recursive: true }) const finalName = await uniqueFilename(destDir, filename) const destAbs = path.join(destDir, finalName) - const nextRel = toPosix(path.relative(root, destAbs)) let meta!: NoteMeta - await relocateFolderTrees( - [[oldAbs, destAbs], [noteCommentsPath(root, oldRel), noteCommentsPath(root, nextRel)], [await noteMetadataPath(root, oldRel), await noteMetadataPath(root, nextRel)]], - async () => { meta = await readMeta(root, destAbs, targetFolder) } - ) + await relocateNote(root, oldRel, destAbs, async () => { + meta = await readMeta(root, destAbs, targetFolder) + }) invalidateNoteMetaCache(root, oldRel) invalidateNoteMetaCache(root, meta.path) invalidateVaultTextSearchCache(root) diff --git a/apps/desktop/src/main/workflow-apply.test.ts b/apps/desktop/src/main/workflow-apply.test.ts index dd6fba3d..3d4cdebe 100644 --- a/apps/desktop/src/main/workflow-apply.test.ts +++ b/apps/desktop/src/main/workflow-apply.test.ts @@ -7,7 +7,19 @@ // promise a rollback makes and a content comparison is the only thing that // checks it. import { createHash } from 'node:crypto' -import { lstat, mkdtemp, mkdir, readdir, readFile, rm, symlink, writeFile } from 'node:fs/promises' +import { promises as fsPromises } from 'node:fs' +import { + lstat, + mkdtemp, + mkdir, + readdir, + readFile, + readlink, + rm, + stat, + symlink, + writeFile +} from 'node:fs/promises' import os from 'node:os' import path from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' @@ -25,6 +37,8 @@ import { undoWorkflowRun, type WorkflowRunLedger } from './workflow-apply' +import { registerEphemeralRoot, unregisterEphemeralRoot } from './ephemeral-vaults' +import { readNoteCreatedAt } from './note-creation-metadata' /** * Paths whose atomic write should fail, so the "the rollback itself failed" @@ -153,6 +167,16 @@ async function isSymlink(abs: string): Promise { return (await lstat(abs)).isSymbolicLink() } +/** + * A link's text with forward slashes, whatever the platform stored. Windows + * keeps a symlink's target with backslashes even when it was created from + * `../sources/Real.md`, so a test that compares the text a run put back has + * to read past that spelling; the run itself wrote the recorded text as is. + */ +async function linkText(abs: string): Promise { + return (await readlink(abs)).split(path.sep).join('/') +} + /** * Whether two paths differing only in case are one file here. macOS and Windows * say yes, which is the whole reason the journal folds its keys; on Linux those @@ -518,6 +542,556 @@ describe('path operations', () => { }) }) +/* -------------------------------------------------------------------------- */ +/* A note's sidecars */ +/* -------------------------------------------------------------------------- */ + +/** Where `.zennotes` keeps a note's comments and its creation date. */ +function commentsRel(note: string): string { + return `.zennotes/comments/${note}.comments.json` +} + +function metadataRel(note: string): string { + return `.zennotes/note-metadata/${note}.metadata.json` +} + +/** + * Make every way of putting a file at `abs` fail: an atomic write, and the + * rename undo uses to move an untouched sidecar back. Only renames TO it, so + * the run can still move the file away. + */ +function failEveryWriteTo(abs: string): void { + injected.failingWrites.add(abs) + const rename = fsPromises.rename.bind(fsPromises) + vi.spyOn(fsPromises, 'rename').mockImplementation(async (from, to) => { + if (String(to) === abs) throw new Error('simulated disk failure') + return rename(from, to) + }) +} + +// The applier moves these as bytes and never parses them; realistic content +// only keeps the assertions readable. +const COMMENTS = '[{"id":"c1","body":"Keep this discussion"}]\n' +const CREATED = '{"version":1,"createdAt":1700000000000}\n' + +/** A note with both sidecars, the way the app leaves one it has saved. */ +async function seedWithSidecars(root: string, note: string): Promise { + await seed(root, note, 'body\n') + await seed(root, commentsRel(note), COMMENTS) + await seed(root, metadataRel(note), CREATED) +} + +async function pathExists(abs: string): Promise { + return lstat(abs).then( + () => true, + () => false + ) +} + +// Before, a path op moved the Markdown alone: the comments stayed behind at the +// old name, detached from the note, and the leftover files blocked or polluted +// the next note given that name (#839). +describe("a note's sidecars travel with it", () => { + it('moves its comments and creation date with it, journalled beside the note', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + + const receipt = await apply(root, [{ kind: 'move', path: 'inbox/Note.md', to: 'archive' }]) + const ledger = await readLedger(root, receipt.runId) + + expect(receipt.paths).toEqual(['inbox/Note.md', 'archive/Note.md']) + expect(await readOrNull(root, commentsRel('archive/Note.md'))).toBe(COMMENTS) + expect(await readOrNull(root, metadataRel('archive/Note.md'))).toBe(CREATED) + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBeNull() + expect(await readOrNull(root, metadataRel('inbox/Note.md'))).toBeNull() + // The notes' journal is exactly what it always was, so an older ZenNotes or + // the Go server reading this ledger still undoes the notes. + expect(ledger.journal).toEqual([ + { path: 'inbox/Note.md', before: 'body\n' }, + { path: 'archive/Note.md', before: null } + ]) + expect(ledger.sidecars).toEqual([ + { note: 'inbox/Note.md', sidecar: 'comments', before: COMMENTS, after: null }, + { note: 'archive/Note.md', sidecar: 'comments', before: null, after: sha256(COMMENTS) }, + { note: 'inbox/Note.md', sidecar: 'metadata', before: CREATED, after: null }, + { note: 'archive/Note.md', sidecar: 'metadata', before: null, after: sha256(CREATED) } + ]) + }) + + const pathOps: Array<[string, WorkflowOp, string]> = [ + ['rename', { kind: 'rename', path: 'inbox/demo/Note.md', to: 'Renamed' }, 'inbox/demo/Renamed.md'], + ['archive', { kind: 'archive', path: 'inbox/demo/Note.md' }, 'archive/demo/Note.md'], + ['trash', { kind: 'trash', path: 'inbox/demo/Note.md' }, 'trash/demo/Note.md'] + ] + + it.each(pathOps)('%s carries them too', async (_kind, op, landed) => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/demo/Note.md') + + await apply(root, [op]) + + expect(await readOrNull(root, landed)).toBe('body\n') + expect(await readOrNull(root, commentsRel(landed))).toBe(COMMENTS) + expect(await readOrNull(root, metadataRel(landed))).toBe(CREATED) + expect(await readOrNull(root, commentsRel('inbox/demo/Note.md'))).toBeNull() + expect(await readOrNull(root, metadataRel('inbox/demo/Note.md'))).toBeNull() + }) + + it('undo puts them back and leaves no empty sidecar folders behind', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/demo/Note.md') + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/demo/Note.md' }]) + expect(await readOrNull(root, commentsRel('archive/demo/Note.md'))).toBe(COMMENTS) + const undo = await undoWorkflowRun(root, receipt.runId) + + // Notes only: the toast says "files restored", and these are two. + expect(undo.restored).toBe(2) + expect(undo.driftedPaths).toEqual([]) + expect(await readOrNull(root, 'inbox/demo/Note.md')).toBe('body\n') + expect(await readOrNull(root, commentsRel('inbox/demo/Note.md'))).toBe(COMMENTS) + expect(await readOrNull(root, metadataRel('inbox/demo/Note.md'))).toBe(CREATED) + // A folder rename refuses a destination whose sidecar folder exists, so an + // empty one left here would block `archive/demo` with nothing to show why. + expect(await pathExists(path.join(root, '.zennotes', 'comments', 'archive'))).toBe(false) + expect(await pathExists(path.join(root, '.zennotes', 'note-metadata', 'archive'))).toBe(false) + }) + + it('undo moves an untouched sidecar back instead of rewriting it', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + const { ino } = await stat(path.join(root, commentsRel('inbox/Note.md'))) + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + expect((await stat(path.join(root, commentsRel('archive/Note.md')))).ino).toBe(ino) + await undoWorkflowRun(root, receipt.runId) + + // The same file, not a copy: a sync client sees it move back, and undoing a + // bulk move costs a rename per sidecar instead of a synced rewrite. + expect((await stat(path.join(root, commentsRel('inbox/Note.md')))).ino).toBe(ino) + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + expect(await readOrNull(root, commentsRel('archive/Note.md'))).toBeNull() + }) + + it('a rollback puts them back too', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + // The end state alone cannot tell "moved and put back" from "never moved". + const moved: string[] = [] + const rename = fsPromises.rename.bind(fsPromises) + vi.spyOn(fsPromises, 'rename').mockImplementation(async (from, to) => { + moved.push(path.relative(root, String(to)).split(path.sep).join('/')) + return rename(from, to) + }) + + const receipt = await apply(root, [ + { kind: 'archive', path: 'inbox/Note.md' }, + { kind: 'trash', path: 'inbox/Missing.md' } + ]) + + expect(moved).toContain(commentsRel('archive/Note.md')) + expect(moved).toContain(metadataRel('archive/Note.md')) + expect(receipt.rolledBack?.reason).toMatch(/vault is unchanged/) + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + expect(await readOrNull(root, metadataRel('inbox/Note.md'))).toBe(CREATED) + expect(await readOrNull(root, commentsRel('archive/Note.md'))).toBeNull() + expect(await readOrNull(root, metadataRel('archive/Note.md'))).toBeNull() + }) + + it('a note moved twice in one run comes back with only the comments it had', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + + const receipt = await apply(root, [ + { kind: 'move', path: 'inbox/Note.md', to: 'inbox/Work' }, + { kind: 'rename', path: 'inbox/Work/Note.md', to: 'Final' } + ]) + expect(await readOrNull(root, commentsRel('inbox/Work/Final.md'))).toBe(COMMENTS) + expect(await readOrNull(root, commentsRel('inbox/Work/Note.md'))).toBeNull() + + await undoWorkflowRun(root, receipt.runId) + + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + expect(await readOrNull(root, commentsRel('inbox/Work/Note.md'))).toBeNull() + expect(await readOrNull(root, commentsRel('inbox/Work/Final.md'))).toBeNull() + }) + + it('replaces a leftover creation date at the destination, and undo brings it back', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + const leftover = '{"version":1,"createdAt":1600000000000}\n' + await seed(root, metadataRel('archive/Note.md'), leftover) + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + + expect(receipt.rolledBack).toBeUndefined() + // A date alone does not take the name, so there is no `Note 2.md`. + expect(await readOrNull(root, 'archive/Note.md')).toBe('body\n') + expect(await readOrNull(root, metadataRel('archive/Note.md'))).toBe(CREATED) + + await undoWorkflowRun(root, receipt.runId) + + expect(await readOrNull(root, metadataRel('archive/Note.md'))).toBe(leftover) + expect(await readOrNull(root, metadataRel('inbox/Note.md'))).toBe(CREATED) + }) + + it('refuses a destination holding an earlier note’s comments, names the file, and rolls back', async () => { + const root = await makeVault() + await seed(root, 'inbox/A.md', 'a\n') + await seedWithSidecars(root, 'inbox/Note.md') + await seed(root, commentsRel('archive/Note.md'), 'an earlier discussion\n') + const before = await snapshot(root) + + const receipt = await apply(root, [ + { kind: 'append', path: 'inbox/A.md', text: 'edit' }, + { kind: 'archive', path: 'inbox/Note.md' } + ]) + + // One sentence ending, not two: the shared message brings its own. + expect(receipt.rolledBack?.reason).toBe( + 'Comments from an earlier note named “Note” are still in .zennotes/comments/archive/Note.md.comments.json. ' + + 'Move or delete that file to use this name. The run was rolled back; your vault is unchanged.' + ) + expect(await snapshot(root)).toEqual(before) + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + expect(await readOrNull(root, commentsRel('archive/Note.md'))).toBe('an earlier discussion\n') + }) + + it('writes down the date of a note ZenNotes never saved, so undo can bring it back', async () => { + const root = await makeVault() + await seed(root, 'inbox/Note.md', 'body\n') + const born = await stat(path.join(root, 'inbox', 'Note.md')) + const createdAt = Math.trunc(born.birthtimeMs || born.ctimeMs) + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + expect(JSON.parse((await readOrNull(root, metadataRel('archive/Note.md'))) ?? 'null')).toEqual({ + version: 1, + createdAt + }) + + await undoWorkflowRun(root, receipt.runId) + + // Undo writes the note back as a new file, born at the undo. The date the + // app shows comes from the sidecar the run wrote down; with no sidecar it + // would be this fallback. + expect(await readNoteCreatedAt(root, 'inbox/Note.md', -1)).toBe(createdAt) + expect(await readOrNull(root, metadataRel('archive/Note.md'))).toBeNull() + }) + + it('writes no date in a temporary folder session, and still clears a leftover one', async () => { + const root = await makeVault() + registerEphemeralRoot(root) + try { + await seed(root, 'inbox/Note.md', 'body\n') + await seed(root, metadataRel('archive/Note.md'), CREATED) + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + + expect(await readOrNull(root, 'archive/Note.md')).toBe('body\n') + expect(await readOrNull(root, metadataRel('archive/Note.md'))).toBeNull() + expect(await readOrNull(root, metadataRel('inbox/Note.md'))).toBeNull() + + await undoWorkflowRun(root, receipt.runId) + + expect(await readOrNull(root, metadataRel('archive/Note.md'))).toBe(CREATED) + } finally { + unregisterEphemeralRoot(root) + } + }) + + it('names a note whose comments changed since the run as drifted, and restores them anyway', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + await seed(root, commentsRel('archive/Note.md'), 'a reply added since\n') + const undo = await undoWorkflowRun(root, receipt.runId) + + expect(undo.driftedPaths).toEqual(['archive/Note.md']) + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + }) + + it('an undo that cannot put the comments back names the note and stays undoable', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + + failEveryWriteTo(path.join(root, commentsRel('inbox/Note.md'))) + await expect(undoWorkflowRun(root, receipt.runId)).rejects.toThrow( + /inbox\/Note\.md \(its comments: simulated disk failure\)/ + ) + + injected.failingWrites.clear() + vi.restoreAllMocks() + await undoWorkflowRun(root, receipt.runId) + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + }) + + it('a rollback that cannot put the comments back says so, and undo can retry', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + failEveryWriteTo(path.join(root, commentsRel('inbox/Note.md'))) + + const receipt = await apply(root, [ + { kind: 'archive', path: 'inbox/Note.md' }, + { kind: 'trash', path: 'inbox/Missing.md' } + ]) + + expect(receipt.rolledBack?.reason).toMatch(/ROLLBACK INCOMPLETE/) + expect(receipt.rolledBack?.reason).toContain('inbox/Note.md (its comments: simulated disk failure)') + expect(receipt.paths).toEqual(['inbox/Note.md']) + + injected.failingWrites.clear() + vi.restoreAllMocks() + const [run] = await listWorkflowRuns(root) + expect(run?.undoable).toBe(true) + await undoWorkflowRun(root, receipt.runId) + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + }) + + it('are in the crash journal before the note moves', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + const noteAbs = path.join(root, 'inbox', 'Note.md') + const rename = fsPromises.rename.bind(fsPromises) + let midRun: Record[] = [] + vi.spyOn(fsPromises, 'rename').mockImplementation(async (from, to) => { + if (from === noteAbs && midRun.length === 0) { + const [name] = await journalNames(root) + if (name) midRun = journalLines(await readFile(path.join(runsDirOf(root), name), 'utf8')) + } + return rename(from, to) + }) + + await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + + // No `path` on a sidecar line: an older ZenNotes recovering this journal + // skips those lines rather than failing the undo on a `.zennotes` path. + expect(midRun.slice(1)).toEqual([ + { path: 'inbox/Note.md', before: 'body\n' }, + { path: 'archive/Note.md', before: null }, + { note: 'inbox/Note.md', sidecar: 'comments', before: COMMENTS }, + { note: 'archive/Note.md', sidecar: 'comments', before: null }, + { note: 'inbox/Note.md', sidecar: 'metadata', before: CREATED }, + { note: 'archive/Note.md', sidecar: 'metadata', before: null } + ]) + }) + + it('share one sync with the note, however many files the move journals', async () => { + // A sync per journal line made a 400-note move with comments and dates about + // three times slower than before sidecars moved at all, for no extra safety. + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + let syncs = 0 + const open = fsPromises.open.bind(fsPromises) + vi.spyOn(fsPromises, 'open').mockImplementation(async (...args: Parameters) => { + const handle = await open(...args) + if (String(args[0]).endsWith('.journal.jsonl')) { + const sync = handle.sync.bind(handle) + handle.sync = async () => { + syncs += 1 + return sync() + } + } + return handle + }) + + await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + + // The header when the journal opens, then all six entries at once. + expect(syncs).toBe(2) + }) + + it('come back from the journal a dead process left', async () => { + const root = await makeVault() + // A run killed after its last rename: note and comments already at the + // destination, the journal on disk, no ledger. + await seed(root, 'archive/Note.md', 'body\n') + await seed(root, commentsRel('archive/Note.md'), COMMENTS) + const orphan = '1700000000000-001-aaaaaaaa' + await mkdir(runsDirOf(root), { recursive: true }) + await writeFile( + journalPath(root, orphan), + [ + { version: 1, runId: orphan, workflowId: 'dead', startedAt: 1700000000000 }, + { path: 'inbox/Note.md', before: 'body\n' }, + { path: 'archive/Note.md', before: null }, + { note: 'inbox/Note.md', sidecar: 'comments', before: COMMENTS }, + { note: 'archive/Note.md', sidecar: 'comments', before: null } + ] + .map((line) => `${JSON.stringify(line)}\n`) + .join(''), + 'utf8' + ) + + const [run] = await listWorkflowRuns(root) + expect(run?.interrupted).toBe(true) + expect(run?.paths).toEqual(['inbox/Note.md', 'archive/Note.md']) + expect((await readLedger(root, orphan)).sidecars).toEqual([ + { note: 'inbox/Note.md', sidecar: 'comments', before: COMMENTS }, + { note: 'archive/Note.md', sidecar: 'comments', before: null } + ]) + + const undo = await undoWorkflowRun(root, orphan) + + expect(undo.driftedPaths).toEqual([]) + expect(await readOrNull(root, 'inbox/Note.md')).toBe('body\n') + expect(await readOrNull(root, commentsRel('inbox/Note.md'))).toBe(COMMENTS) + expect(await readOrNull(root, commentsRel('archive/Note.md'))).toBeNull() + }) + + it('a ledger written before sidecars were journalled still undoes its notes', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + const ledgerPath = path.join(runsDirOf(root), `${receipt.runId}.json`) + const ledger = JSON.parse(await readFile(ledgerPath, 'utf8')) as WorkflowRunLedger + delete ledger.sidecars + await writeFile(ledgerPath, JSON.stringify(ledger), 'utf8') + + const undo = await undoWorkflowRun(root, receipt.runId) + + expect(undo.restored).toBe(2) + expect(await readOrNull(root, 'inbox/Note.md')).toBe('body\n') + expect(await readOrNull(root, 'archive/Note.md')).toBeNull() + }) + + it('refuses a ledger sidecar entry that does not name a note', async () => { + const root = await makeVault() + await seedWithSidecars(root, 'inbox/Note.md') + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + + // Sidecar files are derived from a note's path, so an edited or synced + // ledger cannot turn one into a write anywhere else, inside `.zennotes` + // included. + const ledgerPath = path.join(runsDirOf(root), `${receipt.runId}.json`) + const ledger = JSON.parse(await readFile(ledgerPath, 'utf8')) as WorkflowRunLedger + ledger.sidecars = [ + { note: '../escaped.md', sidecar: 'comments', before: 'owned' }, + { note: '.zennotes/workflows/flow.md', sidecar: 'metadata', before: 'owned' } + ] + await writeFile(ledgerPath, JSON.stringify(ledger), 'utf8') + + await expect(undoWorkflowRun(root, receipt.runId)).rejects.toThrow(/incomplete/) + expect(await readOrNull(root, '../escaped.md.comments.json')).toBeNull() + expect(await readOrNull(root, '.zennotes/escaped.md.comments.json')).toBeNull() + expect( + await readOrNull(root, '.zennotes/note-metadata/.zennotes/workflows/flow.md.metadata.json') + ).toBeNull() + }) +}) + +/* -------------------------------------------------------------------------- */ +/* A note's creation date */ +/* -------------------------------------------------------------------------- */ + +// A saved note keeps its creation date in a date file; one ZenNotes never saved +// shows its file's birth time, and an atomic write replaces that file with one +// born now. The editor's save writes the date down first. The applier did not, +// so every text op gave a note the run's time as its creation date. +describe('a note keeps its creation date', () => { + /** What the app would be left with and no date file: the fallback. */ + const NO_DATE_FILE = -1 + + const textOps: Array<[string, WorkflowOp]> = [ + ['append', { kind: 'append', path: 'inbox/A.md', text: 'more' }], + ['prepend', { kind: 'prepend', path: 'inbox/A.md', text: 'first' }], + ['add-tag', { kind: 'add-tag', path: 'inbox/A.md', tag: 'filed' }], + ['set-frontmatter', { kind: 'set-frontmatter', path: 'inbox/A.md', field: 'status', value: 'done' }], + ['write-section', { kind: 'write-section', path: 'inbox/A.md', heading: 'Log', text: 'entry' }], + ['write-note', { kind: 'write-note', path: 'inbox/A.md', text: 'replaced\n' }] + ] + + it.each(textOps)('through %s with no date file yet, and through the undo', async (_kind, op) => { + const root = await makeVault() + await seed(root, 'inbox/A.md', 'a\n') + const born = await stat(path.join(root, 'inbox', 'A.md')) + const createdAt = Math.trunc(born.birthtimeMs || born.ctimeMs) + + const receipt = await apply(root, [op]) + expect(receipt.rolledBack).toBeUndefined() + expect(await readOrNull(root, 'inbox/A.md')).not.toBe('a\n') + expect(await readNoteCreatedAt(root, 'inbox/A.md', NO_DATE_FILE)).toBe(createdAt) + + await undoWorkflowRun(root, receipt.runId) + + expect(await readOrNull(root, 'inbox/A.md')).toBe('a\n') + expect(await readNoteCreatedAt(root, 'inbox/A.md', NO_DATE_FILE)).toBe(createdAt) + }) + + it('has the date written down before the note is', async () => { + const root = await makeVault() + await seed(root, 'inbox/A.md', 'a\n') + let dateFileAtWrite: string | null = null + injected.beforeWrite = async (abs) => { + if (abs.endsWith(path.join('inbox', 'A.md')) && dateFileAtWrite === null) { + dateFileAtWrite = await readOrNull(root, metadataRel('inbox/A.md')) + } + } + + await apply(root, [{ kind: 'append', path: 'inbox/A.md', text: 'more' }]) + + expect(dateFileAtWrite).not.toBeNull() + }) + + it('leaves a date file that is already there alone, one it cannot read included', async () => { + const root = await makeVault() + await seed(root, 'inbox/A.md', 'a\n') + await seed(root, 'inbox/B.md', 'b\n') + await seed(root, metadataRel('inbox/A.md'), CREATED) + await seed(root, metadataRel('inbox/B.md'), 'not a date\n') + + const receipt = await apply(root, [ + { kind: 'append', path: 'inbox/A.md', text: 'more' }, + { kind: 'append', path: 'inbox/B.md', text: 'more' } + ]) + + // A save refuses a date file it cannot read; a run does not fail over one. + expect(receipt.rolledBack).toBeUndefined() + expect(await readOrNull(root, metadataRel('inbox/A.md'))).toBe(CREATED) + expect(await readOrNull(root, metadataRel('inbox/B.md'))).toBe('not a date\n') + }) + + it('writes no date file in a temporary folder session', async () => { + const root = await makeVault() + registerEphemeralRoot(root) + try { + await seed(root, 'inbox/A.md', 'a\n') + await apply(root, [{ kind: 'append', path: 'inbox/A.md', text: 'more' }]) + expect(await readOrNull(root, metadataRel('inbox/A.md'))).toBeNull() + } finally { + unregisterEphemeralRoot(root) + } + }) + + it('a created note has its own birth time, not a date left behind, and undo brings that back', async () => { + const root = await makeVault() + const leftover = '{"version":1,"createdAt":1600000000000}\n' + await seed(root, metadataRel('inbox/New.md'), leftover) + + const receipt = await apply(root, [{ kind: 'create-note', path: 'inbox/New.md', body: 'new' }]) + const ledger = await readLedger(root, receipt.runId) + + expect(await readOrNull(root, metadataRel('inbox/New.md'))).toBeNull() + expect(await readNoteCreatedAt(root, 'inbox/New.md', NO_DATE_FILE)).toBe(NO_DATE_FILE) + expect(ledger.sidecars).toEqual([ + { note: 'inbox/New.md', sidecar: 'metadata', before: leftover, after: null } + ]) + + await undoWorkflowRun(root, receipt.runId) + + expect(await readOrNull(root, 'inbox/New.md')).toBeNull() + expect(await readOrNull(root, metadataRel('inbox/New.md'))).toBe(leftover) + }) + + it('a created note gets no date file of its own', async () => { + const root = await makeVault() + await apply(root, [{ kind: 'create-note', path: 'inbox/New.md', body: 'new' }]) + expect(await readOrNull(root, 'inbox/New.md')).not.toBeNull() + expect(await readOrNull(root, metadataRel('inbox/New.md'))).toBeNull() + }) +}) + /* -------------------------------------------------------------------------- */ /* Rollback */ /* -------------------------------------------------------------------------- */ @@ -1151,6 +1725,213 @@ describe('symlinked notes', () => { await apply(root, [{ kind: 'create-note', path: 'inbox/New.md', body: 'hello' }]) expect(await isSymlink(path.join(root, 'inbox', 'New.md'))).toBe(false) }) + + // A move renames the link itself, so the destination holds the link and the + // file it points at never moves. Undo used to take that destination for a + // file the run had created and delete what the link pointed at, which can + // live outside the vault, then write a plain copy where the link had been. + describe('moved', () => { + /** A note that is a link to a file outside the vault. */ + async function linkedVault(): Promise<{ root: string; real: string }> { + const root = await makeVault() + const outside = await mkdtemp(path.join(os.tmpdir(), 'zennotes-outside-')) + tempDirs.push(outside) + const real = path.join(outside, 'real.md') + await writeFile(real, 'real\n', 'utf8') + await symlink(real, path.join(root, 'inbox', 'Link.md')) + return { root, real } + } + + it('undo puts the link itself back and leaves the file it points at alone', async () => { + const { root, real } = await linkedVault() + + const receipt = await apply(root, [{ kind: 'move', path: 'inbox/Link.md', to: 'archive' }]) + expect(await isSymlink(path.join(root, 'archive', 'Link.md'))).toBe(true) + const undo = await undoWorkflowRun(root, receipt.runId) + + expect(undo.restored).toBe(2) + expect(undo.driftedPaths).toEqual([]) + expect(await readFile(real, 'utf8')).toBe('real\n') + expect(await isSymlink(path.join(root, 'inbox', 'Link.md'))).toBe(true) + expect(await readlink(path.join(root, 'inbox', 'Link.md'))).toBe(real) + expect(await pathExists(path.join(root, 'archive', 'Link.md'))).toBe(false) + }) + + it('a rollback puts it back the same way', async () => { + const { root, real } = await linkedVault() + + const receipt = await apply(root, [ + { kind: 'archive', path: 'inbox/Link.md' }, + { kind: 'trash', path: 'inbox/Missing.md' } + ]) + + expect(receipt.rolledBack?.reason).toMatch(/vault is unchanged/) + expect(await readFile(real, 'utf8')).toBe('real\n') + expect(await readlink(path.join(root, 'inbox', 'Link.md'))).toBe(real) + expect(await pathExists(path.join(root, 'archive', 'Link.md'))).toBe(false) + }) + + it('undo after an edit through it restores the link and the bytes behind it', async () => { + const { root, real } = await linkedVault() + + const receipt = await apply(root, [ + { kind: 'append', path: 'inbox/Link.md', text: 'edited' }, + { kind: 'archive', path: 'inbox/Link.md' } + ]) + expect(await readFile(real, 'utf8')).toBe('real\nedited\n') + await undoWorkflowRun(root, receipt.runId) + + expect(await readlink(path.join(root, 'inbox', 'Link.md'))).toBe(real) + expect(await readFile(real, 'utf8')).toBe('real\n') + expect(await pathExists(path.join(root, 'archive', 'Link.md'))).toBe(false) + }) + + it('moved twice in one run, it comes back from the last place it went', async () => { + const { root, real } = await linkedVault() + + const receipt = await apply(root, [ + { kind: 'move', path: 'inbox/Link.md', to: 'inbox/Work' }, + { kind: 'rename', path: 'inbox/Work/Link.md', to: 'Renamed' } + ]) + expect(await isSymlink(path.join(root, 'inbox', 'Work', 'Renamed.md'))).toBe(true) + await undoWorkflowRun(root, receipt.runId) + + expect(await readlink(path.join(root, 'inbox', 'Link.md'))).toBe(real) + expect(await readFile(real, 'utf8')).toBe('real\n') + expect(await pathExists(path.join(root, 'inbox', 'Work', 'Renamed.md'))).toBe(false) + expect(await pathExists(path.join(root, 'inbox', 'Work', 'Link.md'))).toBe(false) + }) + + it('a relative link comes back with the same text', async () => { + const root = await makeVault() + await seed(root, 'sources/Real.md', 'real\n') + await symlink('../sources/Real.md', path.join(root, 'inbox', 'Rel.md')) + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Rel.md' }]) + await undoWorkflowRun(root, receipt.runId) + + expect(await linkText(path.join(root, 'inbox', 'Rel.md'))).toBe('../sources/Real.md') + expect(await readOrNull(root, 'sources/Real.md')).toBe('real\n') + expect(await pathExists(path.join(root, 'archive', 'Rel.md'))).toBe(false) + }) + + it('a relative link moved to another depth keeps pointing at its file, and undo spells it as before', async () => { + const root = await makeVault() + await seed(root, 'sources/Real.md', 'real\n') + await symlink('../sources/Real.md', path.join(root, 'inbox', 'Rel.md')) + + const receipt = await apply(root, [{ kind: 'move', path: 'inbox/Rel.md', to: 'inbox/Topics' }]) + + // Moved verbatim, `../sources/Real.md` from inbox/Topics would name + // inbox/sources/Real.md, which does not exist. + expect(await readlink(path.join(root, 'inbox', 'Topics', 'Rel.md'))).toBe(path.join('..', '..', 'sources', 'Real.md')) + expect(await readOrNull(root, 'inbox/Topics/Rel.md')).toBe('real\n') + const undo = await undoWorkflowRun(root, receipt.runId) + + expect(undo.driftedPaths).toEqual([]) + expect(await linkText(path.join(root, 'inbox', 'Rel.md'))).toBe('../sources/Real.md') + expect(await readOrNull(root, 'inbox/Rel.md')).toBe('real\n') + expect(await readOrNull(root, 'sources/Real.md')).toBe('real\n') + expect(await pathExists(path.join(root, 'inbox', 'Topics', 'Rel.md'))).toBe(false) + }) + + it('a text the move need not re-spell is kept as it was', async () => { + const root = await makeVault() + await seed(root, 'sources/Real.md', 'real\n') + // Not how `path.relative` would spell it, so putting it back by spelling + // alone would change it. + await symlink('./../sources/Real.md', path.join(root, 'inbox', 'Rel.md')) + + const receipt = await apply(root, [{ kind: 'move', path: 'inbox/Rel.md', to: 'inbox/Topics' }]) + expect(await readOrNull(root, 'inbox/Topics/Rel.md')).toBe('real\n') + await undoWorkflowRun(root, receipt.runId) + + expect(await linkText(path.join(root, 'inbox', 'Rel.md'))).toBe('./../sources/Real.md') + expect(await readOrNull(root, 'inbox/Rel.md')).toBe('real\n') + }) + + it('the crash journal records the link, and a recovered run puts it back', async () => { + const { root, real } = await linkedVault() + const noteAbs = path.join(root, 'inbox', 'Link.md') + let crashRaw = '' + let crashName = '' + const rename = fsPromises.rename.bind(fsPromises) + vi.spyOn(fsPromises, 'rename').mockImplementation(async (from, to) => { + if (from === noteAbs && !crashName) { + const [name] = await journalNames(root) + if (name) { + crashName = name + crashRaw = await readFile(path.join(runsDirOf(root), name), 'utf8') + } + } + return rename(from, to) + }) + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Link.md' }]) + vi.restoreAllMocks() + expect(journalLines(crashRaw)[1]).toEqual({ path: 'inbox/Link.md', before: 'real\n', link: real }) + // The process died after the move: the journal is all that is left. + await rm(path.join(runsDirOf(root), `${receipt.runId}.json`)) + await writeFile(path.join(runsDirOf(root), crashName), crashRaw, 'utf8') + + const [run] = await listWorkflowRuns(root) + expect(run?.interrupted).toBe(true) + await undoWorkflowRun(root, receipt.runId) + + expect(await readlink(noteAbs)).toBe(real) + expect(await readFile(real, 'utf8')).toBe('real\n') + expect(await pathExists(path.join(root, 'archive', 'Link.md'))).toBe(false) + }) + + it('a ledger from before links were recorded never deletes what the link points at', async () => { + const { root, real } = await linkedVault() + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Link.md' }]) + const ledgerPath = path.join(runsDirOf(root), `${receipt.runId}.json`) + const ledger = JSON.parse(await readFile(ledgerPath, 'utf8')) as WorkflowRunLedger + ledger.journal = ledger.journal.map(({ path: entryPath, before }) => ({ path: entryPath, before })) + await writeFile(ledgerPath, JSON.stringify(ledger), 'utf8') + + await undoWorkflowRun(root, receipt.runId) + + // Without the link's text there is no link to put back, so the note comes + // back as a copy; what matters is that the file behind it survives. + expect(await readFile(real, 'utf8')).toBe('real\n') + expect(await readOrNull(root, 'inbox/Link.md')).toBe('real\n') + expect(await pathExists(path.join(root, 'archive', 'Link.md'))).toBe(false) + }) + + it('an edited ledger cannot plant a link', async () => { + const root = await makeVault() + await seed(root, 'inbox/A.md', 'a\n') + const outside = await mkdtemp(path.join(os.tmpdir(), 'zennotes-outside-')) + tempDirs.push(outside) + const victim = path.join(outside, 'victim.md') + await writeFile(victim, 'victim\n', 'utf8') + const receipt = await apply(root, [{ kind: 'append', path: 'inbox/A.md', text: 'edit' }]) + const ledgerPath = path.join(runsDirOf(root), `${receipt.runId}.json`) + const ledger = JSON.parse(await readFile(ledgerPath, 'utf8')) as WorkflowRunLedger + ledger.journal = [...ledger.journal, { path: 'inbox/Planted.md', before: 'owned\n', link: victim }] + await writeFile(ledgerPath, JSON.stringify(ledger), 'utf8') + + await undoWorkflowRun(root, receipt.runId) + + // Links are only ever moved back, never made from what a ledger says. + expect(await isSymlink(path.join(root, 'inbox', 'Planted.md'))).toBe(false) + expect(await readFile(victim, 'utf8')).toBe('victim\n') + }) + + it('a move never replaces a link at the destination that points at nothing', async () => { + const root = await makeVault() + await seed(root, 'inbox/Note.md', 'note\n') + await mkdir(path.join(root, 'archive'), { recursive: true }) + await symlink(path.join(root, 'sources', 'Gone.md'), path.join(root, 'archive', 'Note.md')) + + const receipt = await apply(root, [{ kind: 'archive', path: 'inbox/Note.md' }]) + + expect(receipt.paths).toEqual(['inbox/Note.md', 'archive/Note 2.md']) + expect(await isSymlink(path.join(root, 'archive', 'Note.md'))).toBe(true) + expect(await readOrNull(root, 'archive/Note 2.md')).toBe('note\n') + }) + }) }) /* -------------------------------------------------------------------------- */ diff --git a/apps/desktop/src/main/workflow-apply.ts b/apps/desktop/src/main/workflow-apply.ts index 1bde72e8..b9ffab84 100644 --- a/apps/desktop/src/main/workflow-apply.ts +++ b/apps/desktop/src/main/workflow-apply.ts @@ -39,6 +39,22 @@ // startup. Until they make it, the vault holds whatever the dead run got // as far as, which is the one gap in the promise above. // +// A note's comments and creation date live in `.zennotes`, keyed by the note's +// path (see note-sidecars.ts), so a path op that moved only the Markdown would +// detach the discussion, lose the date, and leave both behind to block or +// pollute the next note given the old name (#839). Path ops carry them, under +// the same five decisions: each is journalled as `{ note, sidecar, before }` +// before it moves, undo restores those bytes, and the file itself is derived +// from the note's path at restore time rather than stored, so a ledger can +// never name an arbitrary file inside `.zennotes`. +// +// A symlinked note is the one path its bytes do not describe: a move renames +// the LINK, so the file it points at, which may live outside the vault, never +// moves at all. The journal records the link's text beside its bytes, and undo +// moves that same link back from wherever the run left it, found by its text +// and never made from what a ledger says, and removes what the run put down as +// itself, never through a link. See `restoreEntries`. +// // `notify` and `clipboard` are not applied here (the main process is not where // the user's clipboard and toasts live) and are not journalled. They are // counted into `receipt.irreversible` so the promise the UI makes about undo @@ -81,6 +97,14 @@ import { type SystemFolderDirs } from '@shared/workflows/paths' import { WORKFLOWS_REL_DIR } from '@shared/workflows-view' +import { isEphemeralRoot } from './ephemeral-vaults' +import { noteMetadataPath, noteMetadataRoot, prepareNoteCreation } from './note-creation-metadata' +import { + leftoverCommentsMessage, + noteCommentsPath, + noteCommentsRoot, + rebaseMovedLink +} from './note-sidecars' import { getVaultSettings, writeFileAtomic } from './vault' /* -------------------------------------------------------------------------- */ @@ -146,8 +170,17 @@ export interface WorkflowRunLedger { irreversible: number paths: string[] ops: WorkflowOp[] - journal: WorkflowJournalEntry[] + journal: NoteJournalEntry[] hashes: Record + /** + * The sidecars the run moved with its notes. Beside `journal` rather than in + * it, and absent when there were none, because every other reader of a + * ledger (an older ZenNotes, the Go server) resolves each journal entry as a + * note path and would refuse a `.zennotes` one, which would leave the run's + * undo failing forever. Kept apart, they skip the sidecars and still undo the + * notes. + */ + sidecars?: LedgerSidecarEntry[] /** Set once undone, so the same run cannot be taken back twice. */ undone: boolean undoneAt?: number @@ -162,6 +195,53 @@ export interface WorkflowRunLedger { interrupted?: { reason: string } } +/** + * A note's journal entry. `link` is the text of the symlink the path was + * before the run, when it was one: its bytes cannot say that, and a move + * renames the link itself, so undo needs the text to find that link again + * (see `restoreEntries`). Absent in ledgers from before it was recorded, which + * still undo, just without putting links back. + */ +interface NoteJournalEntry extends WorkflowJournalEntry { + link?: string +} + +/** The two files `.zennotes` keeps for a note. */ +type SidecarKind = 'comments' | 'metadata' + +const SIDECAR_KINDS: readonly SidecarKind[] = ['comments', 'metadata'] + +/** How a failure names a sidecar, since the user knows it by what it holds. */ +const SIDECAR_LABELS: Record = { + comments: 'comments', + metadata: 'creation date' +} + +/** + * A sidecar's pre-run bytes: `before: null` means the note had none. It names + * the NOTE, as the run spelled it, and never the sidecar's own path. + * + * On a crash journal line the missing `path` is deliberate: an older ZenNotes + * takes every line with a `path` for a note's entry, and would fail the whole + * undo on one inside `.zennotes`. Without it those lines are skipped, and the + * notes still undo. + */ +interface SidecarJournalEntry { + note: string + sidecar: SidecarKind + before: string | null +} + +/** + * As a finished ledger keeps it: plus the hash of what the run left there + * (null where it moved the sidecar away), which is what undo's drift report + * compares. Absent where the run cannot know, as in a ledger rebuilt from a + * crash journal, the same way `hashes` is empty there. + */ +interface LedgerSidecarEntry extends SidecarJournalEntry { + after?: string | null +} + /* -------------------------------------------------------------------------- */ /* Paths */ /* -------------------------------------------------------------------------- */ @@ -231,6 +311,24 @@ export function resolveVaultPath(root: string, rel: string): string { return abs } +/** + * Where a note's sidecar lives, derived from the note's path every time. + * + * The note's path is checked by `resolveVaultPath` first, exactly as a journal + * entry is, because at undo time it came off disk. Deriving the rest is what + * keeps the `.zennotes` refusal above meaningful: a ledger can only ever aim a + * restore at some note's own comments or creation date, never at a workflow, + * a template or another run's record. + */ +async function sidecarPathOf(root: string, note: string, sidecar: SidecarKind): Promise { + const rel = toPosix(path.relative(path.resolve(root), resolveVaultPath(root, note))) + return sidecar === 'comments' ? noteCommentsPath(root, rel) : await noteMetadataPath(root, rel) +} + +function sidecarRootOf(root: string, sidecar: SidecarKind): string { + return sidecar === 'comments' ? noteCommentsRoot(root) : noteMetadataRoot(root) +} + /** Every vault path an op could touch, for the pre-flight containment check. */ function opTargets(op: WorkflowOp, dirs?: SystemFolderDirs): string[] { switch (op.kind) { @@ -272,6 +370,47 @@ async function exists(abs: string): Promise { } } +/** + * Whether anything at all sits at `abs`, a symlink that points at nothing + * included. `exists` follows links, so it calls such a link free, and a rename + * onto it replaces the link without a trace. + */ +async function pathTaken(abs: string): Promise { + try { + await fs.lstat(abs) + return true + } catch (err) { + if (isMissing(err)) return false + throw err + } +} + +/** + * Give the symlink at `abs` the text `text`, which the caller has checked + * names the same file the link already names: a spelling, not a new target. + */ +async function respellLink(abs: string, text: string): Promise { + const temporary = `${abs}_relink_tmp_${process.pid}_${Date.now()}` + await fs.symlink(text, temporary) + try { + await fs.rename(temporary, abs) + } catch (err) { + await fs.rm(temporary, { force: true }).catch(() => undefined) + throw err + } +} + +/** The text of the symlink at `abs`, or undefined when `abs` is not one. */ +async function linkTextOf(abs: string): Promise { + try { + if (!(await fs.lstat(abs)).isSymbolicLink()) return undefined + } catch (err) { + if (isMissing(err)) return undefined + throw err + } + return await fs.readlink(abs) +} + /** * The path a symlink finally names, or null when `abs` is not a symlink. * @@ -307,8 +446,9 @@ async function linkTargetOf(abs: string): Promise { * Resolving the link and doing the atomic dance at the target keeps both * properties: the link survives and no reader ever sees a half-written file. * `writeFileAtomic` resolves links itself now, so this is belt and braces; the - * resolved path is still wanted here, because undo removes the file the run - * created and that file is the target, never the link. + * resolved path is still wanted here, because through a link that points at + * nothing the file a write creates is the one the link's text names, and that + * is the file undo removes again, never the link. * * The target may sit outside the vault. That is what following a link means, * and it is the same reach every other save in the app has; see @@ -325,6 +465,8 @@ async function writeNoteThroughLinks(abs: string, data: string): Promise { * A move that clobbered an existing note would still be undoable, but a second * run without an undo would silently merge two notes into one, and nothing in * the dry run warned about it. Suffixing is what the rest of the app does. + * "Taken" includes a symlink that points at nothing: the rename would replace + * it, and nothing could put it back. */ async function uniqueRel(root: string, rel: string): Promise { const ext = noteExtensionOf(rel) @@ -334,7 +476,7 @@ async function uniqueRel(root: string, rel: string): Promise { // everything (a permissions oddity, a broken network mount) must fail loudly // instead of spinning forever inside a run that holds a half-written journal. for (let n = 2; n < 1000; n += 1) { - if (!(await exists(path.resolve(root, candidate)))) return candidate + if (!(await pathTaken(path.resolve(root, candidate)))) return candidate candidate = `${stem} ${n}${ext}` } throw new Error(`Cannot find a free destination for ${rel}`) @@ -520,8 +662,9 @@ function resolveLedgerPath(root: string, runId: string): string { * at most its last line, while rewriting a whole document per entry would put * every earlier entry at risk on every op. The first line is the run's header * (what the finished ledger would call `runId`, `workflowId`, `startedAt`, - * `irreversible` and `ops`); every line after it is one `WorkflowJournalEntry`, - * in the order the run touched the paths. + * `irreversible` and `ops`); every line after it is one `NoteJournalEntry` + * or, for a sidecar a path op carries, one `SidecarJournalEntry`, in the order + * the run touched them. */ interface RunJournalFile { abs: string @@ -565,14 +708,18 @@ async function writeJournalLine(handle: FileHandle, line: string): Promise } /** - * Record one journal entry on disk, opening the file on first use. + * Record journal entries on disk, opening the file on first use. * * Opened lazily so a run that touches nothing (a notify-only workflow, or one * whose ops all turn out to be no-ops) leaves no file to recover. What matters - * is the ordering, and it holds either way: this returns only once the entry is - * durable, and the caller only then performs the write it describes. + * is the ordering, and it holds either way: this returns only once the entries + * are durable, and the caller only then performs the writes they describe. */ -async function appendJournalEntry(file: RunJournalFile, entry: WorkflowJournalEntry): Promise { +async function appendJournalEntries( + file: RunJournalFile, + entries: ReadonlyArray +): Promise { + if (entries.length === 0) return try { if (!file.handle) { await fs.mkdir(path.dirname(file.abs), { recursive: true }) @@ -580,7 +727,10 @@ async function appendJournalEntry(file: RunJournalFile, entry: WorkflowJournalEn file.created = true await writeJournalLine(file.handle, file.header) } - await writeJournalLine(file.handle, JSON.stringify(entry)) + // Several entries share one write and one sync. A process killed inside + // that write leaves a prefix of the lines, the tail of which the reader + // skips as a fragment, and the writes those lines protect have not begun. + await writeJournalLine(file.handle, entries.map((entry) => JSON.stringify(entry)).join('\n')) } catch (err) { // Same reasoning as a ledger that cannot be written: a change nothing can // record is a change nothing can take back, so the run stops here and the @@ -621,7 +771,8 @@ interface ParsedRunJournal { startedAt: number irreversible: number ops: WorkflowOp[] - journal: WorkflowJournalEntry[] + journal: NoteJournalEntry[] + sidecars: SidecarJournalEntry[] } /** @@ -636,7 +787,8 @@ async function readRunJournalFile(abs: string): Promise const raw = await fs.readFile(abs, 'utf8') const lines = raw.split('\n').filter((line) => line.trim().length > 0) let header: Record | null = null - const journal: WorkflowJournalEntry[] = [] + const journal: NoteJournalEntry[] = [] + const sidecars: SidecarJournalEntry[] = [] for (const line of lines) { let parsed: unknown try { @@ -649,11 +801,14 @@ async function readRunJournalFile(abs: string): Promise header = parsed continue } - const entryPath = parsed.path - const before = parsed.before - if (typeof entryPath !== 'string') continue - if (typeof before !== 'string' && before !== null) continue - journal.push({ path: entryPath, before }) + const sidecar = parseSidecarEntry(parsed) + if (sidecar) { + // Never with `after`: a crash journal cannot know how the run left things. + sidecars.push({ note: sidecar.note, sidecar: sidecar.sidecar, before: sidecar.before }) + continue + } + const entry = parseNoteEntry(parsed) + if (entry) journal.push(entry) } if (!header) return null const ops = Array.isArray(header.ops) @@ -664,7 +819,8 @@ async function readRunJournalFile(abs: string): Promise startedAt: parseNumber(header.startedAt, 0), irreversible: parseNumber(header.irreversible, 0), ops, - journal + journal, + sidecars } } @@ -727,6 +883,8 @@ async function recoverInterruptedRunsNow(root: string): Promise { ops: parsed.ops, journal: parsed.journal, hashes: {}, + // Without `after`, for the reason `hashes` is empty. + ...(parsed.sidecars.length > 0 ? { sidecars: parsed.sidecars } : {}), undone: false, interrupted: { reason: @@ -762,7 +920,7 @@ interface RunState { * first-touch-wins rule: the entry must describe the state before the run, * not before the latest op. */ - journal: Map + journal: Map /** The same entries on disk, so a killed process leaves them behind. */ journalFile: RunJournalFile /** @@ -772,6 +930,15 @@ interface RunState { * journal entry against the hash recorded for it. */ written: Map + /** + * The sidecars path ops carried, keyed by `sidecarKey`, first touch wins for + * the reason `journal` does: a note moved twice in one run leaves comments at + * the middle path only in between, and undo must remove them there rather + * than restore what the first move put down. + */ + sidecars: Map + /** Sidecar key to the hash of what the run left there (null: moved away). */ + sidecarsWritten: Map /** * Destination the plan promised to where the file actually landed, for the * one case where they differ: `uniqueRel` had to suffix around a collision. @@ -797,18 +964,12 @@ function journalKey(rel: string): string { } /** - * Record a path's pre-run bytes, on disk before it is recorded in memory. - * - * The order is the durability guarantee: every caller awaits this before the - * write it describes, so a process killed at any point leaves a journal that - * covers at least every file it had begun to change. + * The entry, plus the text of the symlink its path is, when it is one. Read at + * the first touch, like the bytes, so it describes the path before the run. */ -async function journalTouch(state: RunState, rel: string, before: string | null): Promise { - const key = journalKey(rel) - if (state.journal.has(key)) return - const entry: WorkflowJournalEntry = { path: rel, before } - await appendJournalEntry(state.journalFile, entry) - state.journal.set(key, entry) +async function withLinkText(root: string, entry: WorkflowJournalEntry): Promise { + const link = await linkTextOf(resolveVaultPath(root, entry.path)) + return link === undefined ? entry : { ...entry, link } } /** Note what the run left at a path. The spelling of the first touch wins, so @@ -820,6 +981,45 @@ function recordWritten(state: RunState, rel: string, hash: string | null): void else state.written.set(key, { path: rel, hash }) } +/** One key per note and sidecar, folded like the note's own journal key. */ +function sidecarKey(note: string, sidecar: SidecarKind): string { + return `${sidecar}:${journalKey(note)}` +} + +/** + * Record the pre-run state of everything one op is about to change, on disk + * before it is recorded in memory: the note (both ends of it, for a path op) + * and each sidecar the op touches, first touch winning throughout. + * + * The order is the durability guarantee: every caller awaits this before the + * write it describes, so a process killed at any point leaves a journal that + * covers at least every file it had begun to change. It is ONE append and ONE + * sync for all of them: a sync per line made a bulk move several times slower + * for no extra safety, since the op writes none of these files until all of + * them are durable either way. + */ +async function journalOp( + state: RunState, + notes: WorkflowJournalEntry[], + sidecars: SidecarJournalEntry[] +): Promise { + const newNotes = new Map() + for (const entry of notes) { + const key = journalKey(entry.path) + if (!state.journal.has(key) && !newNotes.has(key)) { + newNotes.set(key, await withLinkText(state.root, entry)) + } + } + const newSidecars = new Map() + for (const entry of sidecars) { + const key = sidecarKey(entry.note, entry.sidecar) + if (!state.sidecars.has(key) && !newSidecars.has(key)) newSidecars.set(key, entry) + } + await appendJournalEntries(state.journalFile, [...newNotes.values(), ...newSidecars.values()]) + for (const [key, entry] of newNotes) state.journal.set(key, entry) + for (const [key, entry] of newSidecars) state.sidecars.set(key, entry) +} + /** * Where an op's stated path actually lives right now. * @@ -891,11 +1091,98 @@ async function applyTextOpToVault(state: RunState, op: TextOp): Promise { // for a change that did not happen. Only for a file that already exists; // creating an empty note is a real change even though '' equals ''. if (live !== null && next === live) return - await journalTouch(state, rel, live) + const metadata = await sidecarPathOf(state.root, rel, 'metadata') + // Creating a note where a date file waits with no note beside it: that date + // belongs to nobody (#839), and `createNote` discards it too. Journalled, + // so undo puts it back; the new note's date is its own birth time. + const leftover = live === null ? await readIfExists(metadata) : null + if (live !== null) await keepCreationDate(state, rel) + await journalOp( + state, + [{ path: rel, before: live }], + leftover === null ? [] : [{ note: rel, sidecar: 'metadata', before: leftover }] + ) + if (leftover !== null) { + await fs.rm(metadata, { force: true }) + state.sidecarsWritten.set(sidecarKey(rel, 'metadata'), null) + } await writeNoteThroughLinks(abs, next) recordWritten(state, rel, hashText(next)) } +/** + * Write a note's creation date down before its file is replaced. + * + * A note ZenNotes has never saved (one from before 2.51, or from git, sync or + * a file manager) has no date file: its date is the file's own birth time, + * and an atomic write, temp file plus rename, replaces that file with one + * born now. Saving from the editor writes the date down first (`writeNote`), + * and so must every write here, and every move: the rename keeps the file, + * but undo writes it back as a new one. A date file already there is left + * alone, valid or not, so a corrupt one cannot fail a run the way it fails a + * save. This is the one write that is not journalled first, safely: it + * records a date the note already had, where the app already looks for it, so + * nothing anyone can see changes, even if the run dies right after it. Not in + * a temporary folder session, which `writeNote` never writes app state into + * either. + */ +async function keepCreationDate(state: RunState, rel: string): Promise { + if (isEphemeralRoot(state.root)) return + if ((await readIfExists(await sidecarPathOf(state.root, rel, 'metadata'))) !== null) return + await prepareNoteCreation(state.root, rel) +} + +/** A sidecar a path op is about to carry. */ +interface SidecarMove { + kind: SidecarKind + fromAbs: string + toAbs: string + /** The note's own, or null when it has none. */ + fromBytes: string | null + /** Whatever already sits at the destination's name. With no note there, it + * belongs to nobody. */ + toBytes: string | null +} + +/** + * The sidecars a path op carries, read before anything is journalled. + * + * `to` is free (`uniqueRel`), so anything already at its sidecar paths was left + * by a note moved or deleted without them: outside ZenNotes, or by an older + * version of this very function. The policy is the one every other writer + * follows (`relocateNote` in note-sidecars.ts): a leftover + * creation date is replaced, or removed, since it would stamp another note's + * date on this one; leftover comments refuse, because taking over another + * note's discussion and deleting it are both wrong. The refusal fails the run, + * which rolls it back, and the message names the file to move aside. + */ +async function sidecarsToCarry( + state: RunState, + from: string, + to: string, + toAbs: string +): Promise { + const moves: SidecarMove[] = [] + // Refused before anything is written down, so a refusal leaves no trace. + const toComments = await sidecarPathOf(state.root, to, 'comments') + if ((await readIfExists(toComments)) !== null) { + throw new Error(leftoverCommentsMessage(state.root, toAbs)) + } + // The rename keeps the file's birth time, but undo cannot: it writes the + // note back as a new file. Written down now, the date travels and comes back + // like any other sidecar. + await keepCreationDate(state, from) + for (const kind of SIDECAR_KINDS) { + const fromAbs = await sidecarPathOf(state.root, from, kind) + const toSidecar = await sidecarPathOf(state.root, to, kind) + const toBytes = await readIfExists(toSidecar) + const fromBytes = await readIfExists(fromAbs) + if (fromBytes === null && toBytes === null) continue + moves.push({ kind, fromAbs, toAbs: toSidecar, fromBytes, toBytes }) + } + return moves +} + /** One path change. Journals both ends, which is what makes undo need no inverse. */ async function movePathInVault( state: RunState, @@ -918,15 +1205,47 @@ async function movePathInVault( const to = await uniqueRel(state.root, nominal) const toAbs = resolveVaultPath(state.root, to) - await journalTouch(state, from, live) - // `uniqueRel` just said this path is free, but it is read rather than assumed - // null: another process can create a file inside that window, and journalling - // it as "did not exist" would turn undo into a delete of somebody's note. - await journalTouch(state, to, await readIfExists(toAbs)) + // Before the first journal line, so an op refused over its sidecars leaves + // nothing of itself to take back. + const sidecars = await sidecarsToCarry(state, from, to, toAbs) + await journalOp( + state, + [ + { path: from, before: live }, + // `uniqueRel` just said this path is free, but it is read rather than + // assumed null: another process can create a file inside that window, and + // journalling it as "did not exist" would turn undo into a delete of + // somebody's note. + { path: to, before: await readIfExists(toAbs) } + ], + sidecars.flatMap((sidecar) => [ + ...(sidecar.fromBytes === null + ? [] + : [{ note: from, sidecar: sidecar.kind, before: sidecar.fromBytes }]), + { note: to, sidecar: sidecar.kind, before: sidecar.toBytes } + ]) + ) await fs.mkdir(path.dirname(toAbs), { recursive: true }) await fs.rename(fromAbs, toAbs) + // A symlinked note keeps pointing where it did, as the app's own move keeps + // it (see `rebaseMovedLink`). + await rebaseMovedLink(fromAbs, toAbs) recordWritten(state, from, null) recordWritten(state, to, hashText(live)) + for (const sidecar of sidecars) { + if (sidecar.fromBytes === null) { + // Only a leftover date was waiting there, and this note has none of its + // own to put in its place. + await fs.rm(sidecar.toAbs, { force: true }) + state.sidecarsWritten.set(sidecarKey(to, sidecar.kind), null) + continue + } + await fs.mkdir(path.dirname(sidecar.toAbs), { recursive: true }) + // Replaces a leftover date, when one was waiting there. + await fs.rename(sidecar.fromAbs, sidecar.toAbs) + state.sidecarsWritten.set(sidecarKey(from, sidecar.kind), null) + state.sidecarsWritten.set(sidecarKey(to, sidecar.kind), hashText(sidecar.fromBytes)) + } // Later ops in this plan name the destination the engine promised; when the // vault forced a different one, leave a forwarding address. const promised = normalizeRel(promisedRel) @@ -998,19 +1317,83 @@ async function applyOp(state: RunState, op: WorkflowOp): Promise { * caller is already handling a failure and needs the full list to report: a * rollback that stops at the first problem leaves more of the vault wrong than * one that carries on. + * + * A symlinked note needs more than its bytes. A move renames the link itself, + * so the destination holds the link while the file it points at, which may + * live outside the vault, never moved. Putting the vault back means moving + * that same link back, and a path the run filled is emptied by removing what + * sits there as itself, never through a link: the file a link points at is not + * the run's to delete. The one exception is a link that pointed at nothing + * before the run, whose target the run created by writing through it. Its + * entry records the link's text, and only while that same link is there is + * its target removed. + * + * A link is only ever moved back, never made: it is found by its recorded text + * among the paths the run filled. Creating one from what a ledger says would + * let an edited or synced ledger plant a link to anywhere and then write + * through it, the very thing `resolveVaultPath` is here to rule out. With no + * link to move back (a ledger from before the text was recorded, or a link + * removed since), the bytes come back as a plain file, as they always did. */ async function restoreEntries( root: string, - entries: Iterable + entries: Iterable ): Promise<{ restored: number; failures: RestoreFailure[] }> { const failures: RestoreFailure[] = [] let restored = 0 - for (const { path: rel, before } of entries) { + const list = [...entries] + // Where the run left the links it moved, by the file each names: a path + // the run found empty that now holds a link it did not have before. By the + // file rather than the text, because a relative text was re-based when the + // link moved (`rebaseMovedLink`), and is again on the way back. + const movedLinks = new Map() + for (const entry of list) { + if (entry.before !== null) continue + try { + const abs = resolveVaultPath(root, entry.path) + const text = await linkTextOf(abs) + if (text === undefined || text === entry.link) continue + const names = path.resolve(path.dirname(abs), text) + const same = movedLinks.get(names) + if (same) same.push(abs) + else movedLinks.set(names, [abs]) + } catch { + // The loop below meets the same entry and reports it. + } + } + for (const { path: rel, before, link } of list) { try { // Re-validated on the way back out. At rollback time these paths came // from this run, but undo replays the same code over a file read off // disk, and that file must never be able to point a write anywhere. const abs = resolveVaultPath(root, rel) + if (link !== undefined && !(await pathTaken(abs))) { + const names = path.resolve(path.dirname(abs), link) + const moved = movedLinks.get(names)?.pop() + const text = moved === undefined ? undefined : await linkTextOf(moved) + if (moved !== undefined && text !== undefined && path.resolve(path.dirname(moved), text) === names) { + await fs.mkdir(path.dirname(abs), { recursive: true }) + await fs.rename(moved, abs) + await rebaseMovedLink(moved, abs) + await pruneEmptyDirs(root, path.dirname(moved)) + // The recorded text, when it names the very file the link now names + // from here: the same reach, spelled the way it was. + if ((await linkTextOf(abs)) !== link) await respellLink(abs, link) + } + } + if (before === null) { + // Empty before the run, so whatever sits there now goes, as itself. + // Behind the link the path had before the run, pointing at nothing + // then, it is the file the run created by writing through that link. + const text = await linkTextOf(abs) + const gone = (text !== undefined && text === link ? await linkTargetOf(abs) : null) ?? abs + if (await pathTaken(gone)) { + await fs.rm(gone, { force: true }) + await pruneEmptyDirs(root, path.dirname(gone)) + } + restored += 1 + continue + } // A path is journalled BEFORE its write is attempted, so a run that failed // mid-way has journalled paths it never actually changed. Restoring those // is pointless at best, and actively harmful here: whatever failed the @@ -1023,16 +1406,7 @@ async function restoreEntries( restored += 1 continue } - if (before === null) { - // Through a symlink, what the run created is the TARGET file: the link - // itself was there before the run and putting the vault back means - // leaving it there, pointing at nothing again. - const target = (await linkTargetOf(abs)) ?? abs - await fs.rm(target, { force: true }) - await pruneEmptyDirs(root, path.dirname(target)) - } else { - await writeNoteThroughLinks(abs, before) - } + await writeNoteThroughLinks(abs, before) restored += 1 } catch (err) { failures.push({ path: rel, message: messageOf(err) }) @@ -1041,6 +1415,93 @@ async function restoreEntries( return { restored, failures } } +/** + * Put every journalled sidecar back, the way `restoreEntries` does notes. + * + * A failure is reported under its NOTE's path, which is the name the user + * knows and the one a receipt lists; the message says which sidecar it was. + */ +async function restoreSidecars( + root: string, + entries: Iterable +): Promise { + const failures: RestoreFailure[] = [] + const list = [...entries] + // Files this run put where none had been, by what they hold now. When one + // still holds exactly the bytes another entry must get back (a sidecar the + // run moved and nobody touched since), renaming it there restores those + // bytes and removes it from where the run put it, which is what writing the + // one and deleting the other would do, minus a synced write per sidecar: + // without it, undoing a 400-note move took three times as long as it did + // before sidecars moved at all. Only ever after comparing the bytes, so this + // is the same restore, not an inverse. + const moved = new Map() + for (const entry of list) { + if (entry.before !== null) continue + try { + const abs = await sidecarPathOf(root, entry.note, entry.sidecar) + const live = await readIfExists(abs) + if (live === null) continue + const key = `${entry.sidecar}\n${live}` + const same = moved.get(key) + if (same) same.push(abs) + else moved.set(key, [abs]) + } catch { + // The loop below meets the same entry and reports it. + } + } + for (const entry of list) { + try { + const abs = await sidecarPathOf(root, entry.note, entry.sidecar) + const live = await readIfExists(abs) + // Unchanged since the run journalled it, for the reason given in + // `restoreEntries`: a failed run journals sidecars it never moved. + if (live === entry.before) continue + if (entry.before === null) { + await fs.rm(abs, { force: true }) + await pruneEmptySidecarDirs(sidecarRootOf(root, entry.sidecar), path.dirname(abs)) + continue + } + const source = live === null ? moved.get(`${entry.sidecar}\n${entry.before}`)?.pop() : undefined + // Read again right before the rename, so nothing written in between + // rides along with it. + if (source !== undefined && (await readIfExists(source)) === entry.before) { + try { + await fs.mkdir(path.dirname(abs), { recursive: true }) + await fs.rename(source, abs) + await pruneEmptySidecarDirs(sidecarRootOf(root, entry.sidecar), path.dirname(source)) + continue + } catch { + // Writing the bytes is the restore; the rename was only quicker. + } + } + await writeFileAtomic(abs, entry.before) + } catch (err) { + failures.push({ + path: entry.note, + message: `its ${SIDECAR_LABELS[entry.sidecar]}: ${messageOf(err)}` + }) + } + } + return failures +} + +/** + * A run's notes and their sidecars, put back together. + * + * `restored` counts notes only: it is what the undo toast reports, and a note + * whose comments came back with it is still one note. + */ +async function restoreRun( + root: string, + journal: Iterable, + sidecars: Iterable +): Promise<{ restored: number; failures: RestoreFailure[] }> { + const notes = await restoreEntries(root, journal) + const sidecarFailures = await restoreSidecars(root, sidecars) + return { restored: notes.restored, failures: [...notes.failures, ...sidecarFailures] } +} + /** A path a restore could not put back, kept structured so both the receipt's * `paths` and its human-readable reason can be built from the same fact. */ interface RestoreFailure { @@ -1078,6 +1539,27 @@ async function pruneEmptyDirs(root: string, startDir: string): Promise { } } +/** + * The same for the sidecar trees, where nothing below the tree's own root is + * furniture, and where an empty leftover is not harmless clutter either: a + * folder rename refuses a destination whose comments or metadata directory + * already exists (`relocateFolderTrees`), so one left behind by an undo would + * block that folder name for good, with nothing visible to explain why. + */ +async function pruneEmptySidecarDirs(base: string, startDir: string): Promise { + const baseAbs = path.resolve(base) + let dir = startDir + while (dir.startsWith(baseAbs + path.sep)) { + try { + if ((await fs.readdir(dir)).length > 0) return + await fs.rmdir(dir) + } catch { + return + } + dir = path.dirname(dir) + } +} + async function writeLedger(root: string, ledger: WorkflowRunLedger): Promise { await fs.mkdir(runsDir(root), { recursive: true }) await writeFileAtomic(ledgerPathFor(root, ledger.runId), `${JSON.stringify(ledger, null, 2)}\n`) @@ -1211,6 +1693,8 @@ async function applyWorkflowOpsNow( journal: new Map(), journalFile: newRunJournalFile(root, run), written: new Map(), + sidecars: new Map(), + sidecarsWritten: new Map(), redirects: new Map() } let applied = 0 @@ -1291,10 +1775,20 @@ function ledgerBase( journal: journalEntries(state), hashes: Object.fromEntries( [...state.written.values()].map((entry) => [entry.path, entry.hash] as const) - ) + ), + ...(state.sidecars.size > 0 ? { sidecars: sidecarEntries(state) } : {}) } } +/** The journalled sidecars, each with what the run left there when it got as + * far as moving it (a rolled-back run may not have). */ +function sidecarEntries(state: RunState): LedgerSidecarEntry[] { + return [...state.sidecars.entries()].map(([key, entry]) => { + const after = state.sidecarsWritten.get(key) + return after === undefined ? { ...entry } : { ...entry, after } + }) +} + /** * Unwind everything the run wrote and report it, whatever went wrong. * @@ -1310,7 +1804,7 @@ async function rollBackRun( state: RunState, cause: string ): Promise { - const { failures } = await restoreEntries(root, state.journal.values()) + const { failures } = await restoreRun(root, state.journal.values(), state.sidecars.values()) const base = { runId: run.runId, workflowId: run.workflowId, @@ -1318,6 +1812,10 @@ async function rollBackRun( applied: 0, irreversible: run.irreversible } + // A cause can be a whole sentence already (the create-note refusal, the + // leftover-comments one shared with the rest of the app), and the reason + // below adds its own full stop. + const stated = cause.replace(/\.\s*$/, '') if (failures.length === 0) { // The vault is back the way it was found, so the crash journal is // describing a run there is nothing left to recover from. @@ -1325,14 +1823,15 @@ async function rollBackRun( return { ...base, paths: [], - rolledBack: { reason: `${cause}. The run was rolled back; your vault is unchanged.` } + rolledBack: { reason: `${stated}. The run was rolled back; your vault is unchanged.` } } } // The paths still differing from their pre-run bytes are exactly the ones the - // rollback could not reach, which is what `paths` promises to list. - const unrestored = failures.map((failure) => failure.path) + // rollback could not reach, which is what `paths` promises to list. Once + // each: a note and its comments can both have failed. + const unrestored = [...new Set(failures.map((failure) => failure.path))] let reason = - `${cause}. ROLLBACK INCOMPLETE: ${failures.length} of ${state.journal.size} ` + + `${stated}. ROLLBACK INCOMPLETE: ${failures.length} of ${state.journal.size + state.sidecars.size} ` + `files could not be restored (${describeFailures(failures)}). ` + `Undo run ${run.runId} to try again.` try { @@ -1359,7 +1858,7 @@ async function rollBackRun( return { ...base, paths: unrestored, rolledBack: { reason } } } -function journalEntries(state: RunState): WorkflowJournalEntry[] { +function journalEntries(state: RunState): NoteJournalEntry[] { return [...state.journal.values()] } @@ -1371,18 +1870,40 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null } -function parseJournal(value: unknown): WorkflowJournalEntry[] { +/** One note entry, from a ledger or a crash journal line, or null. */ +function parseNoteEntry(value: unknown): NoteJournalEntry | null { + if (!isRecord(value)) return null + const { path: entryPath, before, link } = value + if (typeof entryPath !== 'string') return null + if (typeof before !== 'string' && before !== null) return null + return typeof link === 'string' ? { path: entryPath, before, link } : { path: entryPath, before } +} + +function parseJournal(value: unknown): NoteJournalEntry[] { if (!Array.isArray(value)) return [] - const entries: WorkflowJournalEntry[] = [] - for (const item of value) { - if (!isRecord(item)) continue - const entryPath = item.path - const before = item.before - if (typeof entryPath !== 'string') continue - if (typeof before !== 'string' && before !== null) continue - entries.push({ path: entryPath, before }) - } - return entries + return value + .map((item) => parseNoteEntry(item)) + .filter((entry): entry is NoteJournalEntry => entry !== null) +} + +/** One sidecar entry, from a ledger or a crash journal line, or null. A kind + * this version does not know is dropped rather than guessed at. */ +function parseSidecarEntry(value: unknown): LedgerSidecarEntry | null { + if (!isRecord(value)) return null + const { note, sidecar, before, after } = value + if (typeof note !== 'string') return null + if (sidecar !== 'comments' && sidecar !== 'metadata') return null + if (typeof before !== 'string' && before !== null) return null + const entry: LedgerSidecarEntry = { note, sidecar, before } + if (typeof after === 'string' || after === null) entry.after = after + return entry +} + +function parseSidecars(value: unknown): LedgerSidecarEntry[] { + if (!Array.isArray(value)) return [] + return value + .map((item) => parseSidecarEntry(item)) + .filter((entry): entry is LedgerSidecarEntry => entry !== null) } function parseStringArray(value: unknown): string[] { @@ -1434,6 +1955,8 @@ async function readLedger(abs: string): Promise { hashes: parseHashes(parsed.hashes), undone: parsed.undone === true } + const sidecars = parseSidecars(parsed.sidecars) + if (sidecars.length > 0) ledger.sidecars = sidecars if (typeof parsed.undoneAt === 'number') ledger.undoneAt = parsed.undoneAt const rolledBack = parsed.rolledBack if (isRecord(rolledBack) && typeof rolledBack.reason === 'string') { @@ -1472,6 +1995,19 @@ async function driftedPathsOf(root: string, ledger: WorkflowRunLedger): Promise< } if ((live === null ? null : hashText(live)) !== after) drifted.push(entry.path) } + // A comment added to a moved note since the run is an edit to that note as + // far as its author is concerned, and undo takes it away all the same, so + // it is reported under the note's name. + for (const entry of ledger.sidecars ?? []) { + if (entry.after === undefined || drifted.includes(entry.note)) continue + let live: string | null + try { + live = await readIfExists(await sidecarPathOf(root, entry.note, entry.sidecar)) + } catch { + continue + } + if ((live === null ? null : hashText(live)) !== entry.after) drifted.push(entry.note) + } return drifted } @@ -1508,10 +2044,11 @@ async function undoWorkflowRunNow(root: string, runId: string): Promise 0) { throw new Error( - `Undo of run ${runId} is incomplete: ${failures.length} of ${ledger.journal.length} ` + + `Undo of run ${runId} is incomplete: ${failures.length} of ${ledger.journal.length + sidecars.length} ` + `files could not be restored (${describeFailures(failures)}). ` + `The run is still undoable; try again.` ) diff --git a/apps/desktop/src/mcp/vault-ops.ts b/apps/desktop/src/mcp/vault-ops.ts index ce337c27..d1396ffc 100644 --- a/apps/desktop/src/mcp/vault-ops.ts +++ b/apps/desktop/src/mcp/vault-ops.ts @@ -1,4 +1,5 @@ -import { readNoteCreatedAt, prepareNoteCreation, removeNoteCreation, moveWithCreationMetadata } from '../main/note-creation-metadata' +import { readNoteCreatedAt, prepareNoteCreation, removeNoteCreation, noteMetadataPath } from '../main/note-creation-metadata' +import { noteCommentsPath, noteCommentsRoot, relocateFolderTrees, relocateNote } from '../main/note-sidecars' /** * Vault operations used by the MCP server. Mirrors the filesystem * behavior of src/main/vault.ts, but without Electron dependencies — @@ -14,11 +15,7 @@ import path from 'node:path' import os from 'node:os' import { parse as parseToml } from 'smol-toml' import { retitleLeadingHeading } from '@shared/note-heading-sync' -import { - NOTE_COMMENTS_DIR, - NOTE_COMMENTS_SUFFIX, - normalizeNoteComments -} from '@shared/note-comments' +import { normalizeNoteComments } from '@shared/note-comments' import type { NoteComment, NoteCommentInput } from '@shared/ipc' export type { NoteComment, NoteCommentInput } import { noteTasksMode, type NoteTasksMode } from '@shared/tasks' @@ -996,13 +993,9 @@ export async function renameNote(root: string, rel: string, nextTitle: string): } catch (e) { if ((e as NodeJS.ErrnoException).code !== 'ENOENT') throw e } - if (abs.toLowerCase() === target.toLowerCase() && abs !== target) { - const tmp = abs + '_rename_tmp_' + Date.now() - await moveWithCreationMetadata(root, abs, tmp) - await moveWithCreationMetadata(root, tmp, target) - } else { - await moveWithCreationMetadata(root, abs, target) - } + // Comments and the creation date travel with the note, as they do on the + // desktop; a case-only rename is handled inside the shared move. + await relocateNote(root, toPosix(path.relative(root, abs)), target, async () => {}) } await syncTitleHeading(abs, target, trimmed) return await readMeta(root, target, folder) @@ -1063,7 +1056,7 @@ async function moveBetweenFolders( const baseTitle = path.basename(filename, path.extname(filename)) const finalTitle = await uniqueTitle(destDir, baseTitle) const destAbs = path.join(destDir, `${finalTitle}.md`) - await moveWithCreationMetadata(root, abs, destAbs) + await relocateNote(root, toPosix(path.relative(root, abs)), destAbs, async () => {}) return await readMeta(root, destAbs, target) } @@ -1095,7 +1088,7 @@ export async function moveNote( const baseTitle = path.basename(filename, ext) const finalTitle = await uniqueTitle(destDir, baseTitle) const destAbs = path.join(destDir, `${finalTitle}${ext}`) - await moveWithCreationMetadata(root, oldAbs, destAbs) + await relocateNote(root, toPosix(path.relative(root, oldAbs)), destAbs, async () => {}) return await readMeta(root, destAbs, targetFolder) } @@ -1116,8 +1109,12 @@ export async function duplicateNote(root: string, rel: string): Promise { const abs = resolveSafe(root, rel) + const notePath = toPosix(path.relative(root, abs)) await fs.rm(abs, { force: true }) - await removeNoteCreation(root, toPosix(path.relative(root, abs))) + // A note's discussion goes with it, as on the desktop: left behind, it would + // be taken over by the next note created under this name. + await fs.rm(noteCommentsPath(root, notePath), { force: true }) + await removeNoteCreation(root, notePath) } export async function emptyTrash(root: string): Promise { @@ -1125,7 +1122,9 @@ export async function emptyTrash(root: string): Promise { try { const entries = await fs.readdir(trashDir) await Promise.all(entries.map((e) => fs.rm(path.join(trashDir, e), { recursive: true, force: true }))) - await removeNoteCreation(root, toPosix(path.relative(root, trashDir)), true) + const trashRel = toPosix(path.relative(root, trashDir)) + await fs.rm(resolveSafe(noteCommentsRoot(root), trashRel), { recursive: true, force: true }) + await removeNoteCreation(root, trashRel, true) } catch { /* no trash dir */ } @@ -1159,8 +1158,18 @@ export async function renameFolder( if ((newAbs + path.sep).startsWith(oldAbs + path.sep)) { throw new Error('Cannot move a folder into itself') } - await fs.mkdir(path.dirname(newAbs), { recursive: true }) - await moveWithCreationMetadata(root, oldAbs, newAbs, true) + const oldRel = toPosix(path.relative(root, oldAbs)) + const newRel = toPosix(path.relative(root, newAbs)) + // The folder's comments and creation dates move as one with it, the way the + // desktop's renameFolderTrees does; a failure puts all three back. + await relocateFolderTrees( + [ + [oldAbs, newAbs], + [resolveSafe(noteCommentsRoot(root), oldRel), resolveSafe(noteCommentsRoot(root), newRel)], + [await noteMetadataPath(root, oldRel, true), await noteMetadataPath(root, newRel, true)] + ], + async () => {} + ) return newClean } @@ -1173,8 +1182,10 @@ export async function deleteFolder( if (!clean) throw new Error('Cannot delete the top-level folder') const folderAbs = await folderRoot(root, topFolder) const abs = resolveSafe(folderAbs, clean) + const rel = toPosix(path.relative(root, abs)) await fs.rm(abs, { recursive: true, force: true }) - await removeNoteCreation(root, toPosix(path.relative(root, abs)), true) + await fs.rm(resolveSafe(noteCommentsRoot(root), rel), { recursive: true, force: true }) + await removeNoteCreation(root, rel, true) } /* ---------- Text search ---------------------------------------------- */ @@ -1916,13 +1927,6 @@ export async function insertAtLine( /* ---------- Note comments (#738) --------------------------------------- */ -/** The sidecar beside a note: `.zennotes/comments/.comments.json`, the - * same path the desktop and the Go server use, validated against escapes. */ -function noteCommentsPath(root: string, rel: string): string { - const commentsRoot = path.join(root, INTERNAL_VAULT_DIR, NOTE_COMMENTS_DIR) - return resolveSafe(commentsRoot, `${toPosix(rel)}${NOTE_COMMENTS_SUFFIX}`) -} - export async function readNoteComments(root: string, rel: string): Promise { const notePath = toPosix(rel) try { diff --git a/apps/share-viewer/package.json b/apps/share-viewer/package.json index 0c5dfe45..5301ee0f 100644 --- a/apps/share-viewer/package.json +++ b/apps/share-viewer/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/share-viewer", "private": true, - "version": "2.54.1", + "version": "2.55.0", "type": "module", "description": "Read-only renderer for publicly shared ZenNotes, embedded by the zennotes.org website", "homepage": "https://zennotes.org", diff --git a/apps/web/package.json b/apps/web/package.json index 3ca5d4f9..e62d0858 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/web", "private": true, - "version": "2.54.1", + "version": "2.55.0", "type": "module", "description": "ZenNotes web client for self-hosted and hosted deployments", "homepage": "https://zennotes.org", diff --git a/docs/ideas/workflows.md b/docs/ideas/workflows.md index 2e81f2fd..1a1ffcdc 100644 --- a/docs/ideas/workflows.md +++ b/docs/ideas/workflows.md @@ -16,8 +16,12 @@ A visual, keyboard-drivable pipeline editor for the vault. > local vaults shipped first. Since 2.29, current self-hosted web servers also > store workflow files and apply the browser-prepared transaction under the Go > vault lock, with the same journalled Undo and crash recovery. Electron remote -> workspaces remain read-only. Not shipped yet, and described below as design: -> event and schedule triggers (they parse but do not fire), workflow MCP tools, +> workspaces remain read-only. Event triggers shipped in 2.55 in a narrower +> form than the executor model below: they fire for the edits made in the app +> itself, each run sees only the note that changed, and Settings carries a +> per-device kill switch (the in-app Help has the rules). Not shipped yet, and +> described below as design: schedule triggers (they parse but do not fire), +> the single executor per vault and sync-arrived changes, workflow MCP tools, > and anything labeled community. ## Problem Statement diff --git a/package-lock.json b/package-lock.json index 968cc4f2..a7ef01bd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "zennotes-monorepo", - "version": "2.54.1", + "version": "2.55.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "zennotes-monorepo", - "version": "2.54.1", + "version": "2.55.0", "hasInstallScript": true, "workspaces": [ "apps/*", @@ -23,7 +23,7 @@ }, "apps/desktop": { "name": "@zennotes/desktop", - "version": "2.54.1", + "version": "2.55.0", "license": "MIT", "dependencies": { "@codemirror/autocomplete": "^6.18.3", @@ -874,7 +874,7 @@ }, "apps/share-viewer": { "name": "@zennotes/share-viewer", - "version": "2.54.1", + "version": "2.55.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -945,7 +945,7 @@ }, "apps/web": { "name": "@zennotes/web", - "version": "2.54.1", + "version": "2.55.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -16382,7 +16382,7 @@ }, "packages/app-core": { "name": "@zennotes/app-core", - "version": "2.54.1", + "version": "2.55.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -16469,14 +16469,14 @@ }, "packages/bridge-contract": { "name": "@zennotes/bridge-contract", - "version": "2.54.1", + "version": "2.55.0", "devDependencies": { "typescript": "^5.7.2" } }, "packages/shared-domain": { "name": "@zennotes/shared-domain", - "version": "2.54.1", + "version": "2.55.0", "dependencies": { "@zennotes/bridge-contract": "*", "lz-string": "^1.5.0" @@ -16488,7 +16488,7 @@ }, "packages/shared-ui": { "name": "@zennotes/shared-ui", - "version": "2.54.1" + "version": "2.55.0" } } } diff --git a/package.json b/package.json index abf68ae8..d0227805 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "zennotes-monorepo", "private": true, - "version": "2.54.1", + "version": "2.55.0", "description": "ZenNotes monorepo for desktop, web, and self-hosted server builds", "packageManager": "npm@10.9.2", "engines": { diff --git a/packages/app-core/package.json b/packages/app-core/package.json index 8ad063f7..4934a5d8 100644 --- a/packages/app-core/package.json +++ b/packages/app-core/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/app-core", "private": true, - "version": "2.54.1", + "version": "2.55.0", "type": "module", "exports": { "./main": "./src/main.tsx", diff --git a/packages/app-core/src/browse-actions.test.ts b/packages/app-core/src/browse-actions.test.ts index 2df424fd..971d57ce 100644 --- a/packages/app-core/src/browse-actions.test.ts +++ b/packages/app-core/src/browse-actions.test.ts @@ -314,6 +314,15 @@ describe('public Browse actions', () => { ) }) + it('moves a nested folder to the notes root on an empty answer', async () => { + const s = await setup() + const result = s.requestMoveBrowseDirectory(s.host, 'Work/Nested') + expect(s.getPromptRequest()?.options.validate?.('')).toBeNull() + s.answer('') + expect(await result).toBe('completed') + expect(s.rename).toHaveBeenCalledWith('inbox', 'Work/Nested', 'Nested', expect.any(Function)) + }) + it('offers only real destinations: not itself, its children, databases, or archive', async () => { const s = await setup() s.useStore.setState({ @@ -325,10 +334,10 @@ describe('public Browse actions', () => { ] }) const result = s.requestMoveBrowseDirectory(s.host, 'Work') - expect(s.getPromptRequest()?.options.suggestions?.map((row) => row.value)).toEqual([ - 'inbox', - 'inbox/Home' - ]) + // The notes root is the empty path, labelled the way the sidebar labels it. + const suggestions = s.getPromptRequest()?.options.suggestions + expect(suggestions?.map((row) => row.value)).toEqual(['', 'Home']) + expect(suggestions?.[0].label).toBe('Inbox') s.answer(null) expect(await result).toBe('cancelled') }) @@ -337,9 +346,6 @@ describe('public Browse actions', () => { const s = await setup() for (const value of [ null, - '', - ' ', - 'Work', 'archive', 'inbox/Work/Nested', 'inbox/Work/Nested/Deeper', @@ -355,11 +361,14 @@ describe('public Browse actions', () => { s.answer(value) expect(await result).toBe('cancelled') } - // Its current parent is a valid answer that changes nothing. - const same = s.requestMoveBrowseDirectory(s.host, 'Work/Nested') - expect(s.getPromptRequest()?.options.validate?.('inbox/Work')).toBeNull() - s.answer('inbox/Work') - expect(await same).toBe('cancelled') + // Its current parent is a valid answer that changes nothing, in either + // spelling: the sidebar's, or the older inbox/ one. + for (const parent of ['Work', 'inbox/Work']) { + const same = s.requestMoveBrowseDirectory(s.host, 'Work/Nested') + expect(s.getPromptRequest()?.options.validate?.(parent)).toBeNull() + s.answer(parent) + expect(await same).toBe('cancelled') + } expect(s.rename).not.toHaveBeenCalled() }) diff --git a/packages/app-core/src/components/ArchiveView.tsx b/packages/app-core/src/components/ArchiveView.tsx index 39cae579..58cbf771 100644 --- a/packages/app-core/src/components/ArchiveView.tsx +++ b/packages/app-core/src/components/ArchiveView.tsx @@ -6,7 +6,7 @@ import { isArchiveViewActive, useStore } from '../store' import { ArchiveIcon, ArrowUpRightIcon, TrashIcon } from './icons' import { CollectionViewHeader } from './CollectionViewHeader' import { ContextMenu } from './ContextMenu' -import { buildMoveNotePrompt, parseMoveNoteTarget } from '../lib/move-note' +import { buildMoveNotePrompt, moveNoteVocabulary, parseMoveNoteTarget } from '../lib/move-note' import { promptApp } from '../lib/prompt-requests' import { advanceSequence, getKeymapBinding, matchesSequenceToken } from '../lib/keymaps' import { resolveSystemFolderLabels } from '../lib/system-folder-labels' @@ -44,6 +44,7 @@ export function ArchiveView(): JSX.Element { const vimMode = useStore((s) => s.vimMode) const setFocusedPanel = useStore((s) => s.setFocusedPanel) const systemFolderLabels = useStore((s) => s.systemFolderLabels) + const vaultSettings = useStore((s) => s.vaultSettings) const workspaceMode = useStore((s) => s.workspaceMode) const amActive = useStore(isArchiveViewActive) const folderLabels = useMemo( @@ -176,9 +177,11 @@ export function ArchiveView(): JSX.Element { items.push({ label: 'Move…', onSelect: async () => { - const target = await promptApp(buildMoveNotePrompt(note, folders)) - if (!target) return - const dest = parseMoveNoteTarget(target) + const vocabulary = moveNoteVocabulary(vaultSettings, systemFolderLabels, folders) + const target = await promptApp(buildMoveNotePrompt(note, folders, vocabulary)) + // Empty is an answer (the notes root); only null is the Cancel. + if (target === null) return + const dest = parseMoveNoteTarget(target, vocabulary) await moveNote(note.path, dest.folder, dest.subpath) } }) diff --git a/packages/app-core/src/components/CalendarPanel.tsx b/packages/app-core/src/components/CalendarPanel.tsx index e6fce104..580be172 100644 --- a/packages/app-core/src/components/CalendarPanel.tsx +++ b/packages/app-core/src/components/CalendarPanel.tsx @@ -122,6 +122,7 @@ export function CalendarPanel({ }): JSX.Element { const notes = useStore((s) => s.notes) const vaultSettings = useStore((s) => s.vaultSettings) + const vimMode = useStore((s) => s.vimMode) const openDailyNoteForDate = useStore((s) => s.openDailyNoteForDate) const openWeeklyNoteForDate = useStore((s) => s.openWeeklyNoteForDate) const vaultTasks = useStore((s) => s.vaultTasks) @@ -538,6 +539,12 @@ export function CalendarPanel({ const cur = tasks[Math.min(activeTaskIndex, Math.max(0, tasks.length - 1))] if (e.metaKey || e.ctrlKey || e.altKey) return + // With Vim off the single-character keys stay with the page, the rule + // every list in the app follows: arrows, Enter, Space, Tab and Escape + // are universal, the letters (and < > [ ]) are Vim's. Nothing below + // then names a key that is not live, and a note typed into by mistake + // is not toggled, moved or deleted by a stray letter. + if (!vimMode && e.key.length === 1 && e.key !== ' ') return if (grabbedTask) { if (e.key === 'Escape') { @@ -733,6 +740,7 @@ export function CalendarPanel({ return () => window.removeEventListener('keydown', handler, true) }, [ dailyEnabled, + vimMode, selectedDayTasks, activeTaskIndex, grabbedTask, diff --git a/packages/app-core/src/components/CommentsPanel.tsx b/packages/app-core/src/components/CommentsPanel.tsx index 5e945b79..f6334082 100644 --- a/packages/app-core/src/components/CommentsPanel.tsx +++ b/packages/app-core/src/components/CommentsPanel.tsx @@ -88,6 +88,11 @@ export function CommentsPanel({ const panelRef = useRef(null) const notePathRef = useRef(note.path) const commentsFocused = focusedPanel === 'comments' + // The strip and the badges name VimNav's keys, which stand down with Vim + // off, so neither exists then: a row of letters that do nothing reads as + // a terminal to someone who never asked for one, and a strip that can + // never fade in would only hold its blank row open under the header. + const vimMode = useStore((s) => s.vimMode) useEffect(() => { void loadNoteComments(note.path) @@ -255,22 +260,24 @@ export function CommentsPanel({ -
- - - - - - - - -
+ {vimMode && ( +
+ + + + + + + + +
+ )} {draft && (
@@ -475,7 +482,9 @@ function CommentCard({ // Render the comment body as Markdown (sanitized). Cached by renderMarkdown, // memoized per-body so card re-renders (hover/selection) don't re-parse. const bodyHtml = useMemo(() => renderMarkdown(comment.body), [comment.body]) - const showActionShortcuts = active && commentsFocused && !editing + // One subscription per card: the badges exist only for Vim's keys. + const vimMode = useStore((s) => s.vimMode) + const showActionShortcuts = vimMode && active && commentsFocused && !editing const handleCardClick = (event: MouseEvent): void => { const target = event.target as HTMLElement | null if (target?.closest('button, textarea, input, select, a, [data-comment-card-control]')) return @@ -680,7 +689,7 @@ function CommentCard({ @@ -690,7 +699,7 @@ function CommentCard({ @@ -701,7 +710,7 @@ function CommentCard({ @@ -711,7 +720,7 @@ function CommentCard({ @@ -720,7 +729,7 @@ function CommentCard({ s.vimMode) + // Every key on the strip is VimNav's, which stands down with Vim off, so + // the strip does too rather than naming keys that do nothing. + const showKeyboardHints = vimMode && (isConnectionsFocused || isHoverPreviewFocused) const cancelScheduledClose = (): void => { if (!closeTimerRef.current) return @@ -454,6 +457,9 @@ function ConnectionRow({ active: boolean rowIndex: number }): JSX.Element { + // The cursor row keeps its styling in both modes (it is the row you + // clicked); the chip on it names VimNav's key, so it exists only in Vim mode. + const vimMode = useStore((s) => s.vimMode) return (
- {active && ( + {active && vimMode && (
@@ -519,6 +525,7 @@ function AttachmentConnectionRow({ rowIndex: number }): JSX.Element { const name = link.assetPath.split('/').pop() ?? link.assetPath + const vimMode = useStore((s) => s.vimMode) return ( diff --git a/packages/app-core/src/components/NoteList.tsx b/packages/app-core/src/components/NoteList.tsx index 7c955f50..b11957b2 100644 --- a/packages/app-core/src/components/NoteList.tsx +++ b/packages/app-core/src/components/NoteList.tsx @@ -15,7 +15,7 @@ import { import { ContextMenu, type ContextMenuItem } from './ContextMenu' import { ResizeHandle } from './ResizeHandle' import { Button, IconButton } from './ui/Button' -import { buildMoveNotePrompt, parseMoveNoteTarget } from '../lib/move-note' +import { buildMoveNotePrompt, moveNoteVocabulary, parseMoveNoteTarget } from '../lib/move-note' import { naturalCompare } from '../lib/natural-sort' import { extractTags } from '../lib/tags' import { setDragPayload } from '../lib/dnd' @@ -95,6 +95,7 @@ export function NoteList(): JSX.Element { const openDatabase = useStore((s) => s.openDatabase) const prefetchNotes = useStore((s) => s.prefetchNotes) const focusedPanel = useStore((s) => s.focusedPanel) + const vimMode = useStore((s) => s.vimMode) const noteListCursorIndex = useStore((s) => s.noteListCursorIndex) const setFocusedPanel = useStore((s) => s.setFocusedPanel) const systemFolderLabels = useStore((s) => s.systemFolderLabels) @@ -186,9 +187,12 @@ export function NoteList(): JSX.Element { await runNoteLifecycleAction(n.path, 'trash') } const onMove = async (): Promise => { - const target = await promptApp(buildMoveNotePrompt(n, folders)) - if (!target) return - const dest = parseMoveNoteTarget(target) + const state = useStore.getState() + const vocabulary = moveNoteVocabulary(state.vaultSettings, state.systemFolderLabels, folders) + const target = await promptApp(buildMoveNotePrompt(n, folders, vocabulary)) + // Empty is an answer (the notes root); only null is the Cancel. + if (target === null) return + const dest = parseMoveNoteTarget(target, vocabulary) await moveNote(n.path, dest.folder, dest.subpath) } const onRestore = async (): Promise => { @@ -713,7 +717,9 @@ export function NoteList(): JSX.Element { return (
setFocusedPanel('notelist')} onFocusCapture={() => setFocusedPanel('notelist')} diff --git a/packages/app-core/src/components/QuickCaptureApp.tsx b/packages/app-core/src/components/QuickCaptureApp.tsx index 00214ef6..99ab7afb 100644 --- a/packages/app-core/src/components/QuickCaptureApp.tsx +++ b/packages/app-core/src/components/QuickCaptureApp.tsx @@ -727,6 +727,7 @@ export function QuickCaptureApp(): JSX.Element { { setOverlay('none') requestAnimationFrame(() => editorRef.current?.focus()) @@ -883,11 +884,14 @@ type CommandAction = 'save' | 'save-no-close' | 'new' | 'open' interface CommandOverlayProps { modKey: string mode: EditingMode + /** Whether the capture editor runs Vim: its ex line is the only way to + * `:w`, so that hint exists only then. */ + vimMode: boolean onAction: (action: CommandAction) => void onCancel: () => void } -function CommandOverlay({ modKey, mode, onAction, onCancel }: CommandOverlayProps): JSX.Element { +function CommandOverlay({ modKey, mode, vimMode, onAction, onCancel }: CommandOverlayProps): JSX.Element { const [query, setQuery] = useState('') const [active, setActive] = useState(0) const inputRef = useRef(null) @@ -907,7 +911,7 @@ function CommandOverlay({ modKey, mode, onAction, onCancel }: CommandOverlayProp { id: 'save-no-close' as CommandAction, label: 'Save without hiding', - hint: ':w', + hint: vimMode ? ':w' : '', keywords: 'save write keep open' }, { @@ -923,7 +927,7 @@ function CommandOverlay({ modKey, mode, onAction, onCancel }: CommandOverlayProp keywords: 'open switch picker find search note' } ], - [mode.kind, modKey] + [mode.kind, modKey, vimMode] ) const results = useMemo(() => { @@ -988,9 +992,11 @@ function CommandOverlay({ modKey, mode, onAction, onCancel }: CommandOverlayProp ].join(' ')} > {cmd.label} - - {cmd.hint} - + {cmd.hint && ( + + {cmd.hint} + + )} ) }) diff --git a/packages/app-core/src/components/SearchCreateForm.tsx b/packages/app-core/src/components/SearchCreateForm.tsx index 8abdd83c..8ad4c1d7 100644 --- a/packages/app-core/src/components/SearchCreateForm.tsx +++ b/packages/app-core/src/components/SearchCreateForm.tsx @@ -402,7 +402,10 @@ export function SearchCreateForm({ onMouseDown={(e) => e.preventDefault()} onClick={() => onOpenExisting(collision.note)} > - Open it Shift+↵ + Open it{' '} + + Shift+↵ + ) : ( @@ -476,7 +479,12 @@ export function SearchCreateForm({ )}
-
+ {/* `data-keyboard-hints` marks the hardware-keyboard hints here and on + the Open it chip above: the phone shells hide them by that hook, + because this footer also holds Back and Create, so the rule that + hides every other palette footer cannot be allowed to match it + (#842). A tablet with a keyboard keeps them. */} +
↑↓ pick diff --git a/packages/app-core/src/components/SettingsModal.tsx b/packages/app-core/src/components/SettingsModal.tsx index 4f2f5477..b2535be1 100644 --- a/packages/app-core/src/components/SettingsModal.tsx +++ b/packages/app-core/src/components/SettingsModal.tsx @@ -560,6 +560,8 @@ export function SettingsModal(): JSX.Element { const setTabsEnabled = useStore((s) => s.setTabsEnabled); const workflowsEnabled = useStore((s) => s.workflowsEnabled); const setWorkflowsEnabled = useStore((s) => s.setWorkflowsEnabled); + const workflowEventTriggers = useStore((s) => s.workflowEventTriggers); + const setWorkflowEventTriggers = useStore((s) => s.setWorkflowEventTriggers); const atlasEnabled = useStore((s) => s.atlasEnabled); const setAtlasEnabled = useStore((s) => s.setAtlasEnabled); const hiddenWorkflowPresets = useStore((s) => s.hiddenWorkflowPresets); @@ -2915,6 +2917,7 @@ export function SettingsModal(): JSX.Element { "The Workflows canvas, and whether it appears in the app at all.", searchIds: [ "workflows-enabled", + "workflow-event-triggers", "workflow-hidden-recipes", "workflow-tutorial", ], @@ -2931,6 +2934,13 @@ export function SettingsModal(): JSX.Element { settingId="workflows-enabled" onChange={setWorkflowsEnabled} /> +
s.folders); const hasAssetsDir = useStore((s) => s.hasAssetsDir); const focusedPanel = useStore((s) => s.focusedPanel); + const vimMode = useStore((s) => s.vimMode); const sidebarCursorIndex = useStore((s) => s.sidebarCursorIndex); const activeNote = useStore((s) => s.activeNote); const activeDirty = useStore((s) => s.activeDirty); @@ -2213,9 +2214,12 @@ export function Sidebar(): JSX.Element { items.push({ label: "Move…", onSelect: async () => { - const target = await promptApp(buildMoveNotePrompt(n, allFolders)); - if (!target) return; - const dest = parseMoveNoteTarget(target); + const state = useStore.getState(); + const vocabulary = moveNoteVocabulary(state.vaultSettings, state.systemFolderLabels, allFolders); + const target = await promptApp(buildMoveNotePrompt(n, allFolders, vocabulary)); + // Empty is an answer (the notes root); only null is the Cancel. + if (target === null) return; + const dest = parseMoveNoteTarget(target, vocabulary); await moveNoteAction(n.path, dest.folder, dest.subpath); }, }); @@ -2918,7 +2922,11 @@ export function Sidebar(): JSX.Element { return (
; } +/** + * The key chip on the cursor row (`m` opens the row's menu). The key is + * Vim's: VimNav owns every single-letter shortcut and stands down entirely + * with Vim mode off, so with Vim off the chip named a key that did nothing. + * Read from the store here rather than threaded through six row components, + * which is one subscription, since only the cursor row mounts a chip. + */ function RowKeyHint({ active, keyLabel, @@ -6295,7 +6310,9 @@ function RowKeyHint({ keyLabel: string; label?: string; compact?: boolean; -}): JSX.Element { +}): JSX.Element | null { + const vimMode = useStore((s) => s.vimMode); + if (!vimMode) return null; return ( setSelectedTags([])} - title="Clear all selected tags (c)" + title={vimMode ? 'Clear all selected tags (c)' : 'Clear all selected tags'} className="rounded-md border border-paper-300/60 px-2 py-1 text-xs text-current/60 transition-colors hover:bg-paper-200/70 hover:text-current/90" > Clear all @@ -660,7 +660,9 @@ export function TagView(): JSX.Element { ) : (
- j/k move · Enter/o open · click chips to toggle · c clear tags · / filter · : command · :q close + {vimMode + ? 'j/k move · Enter/o open · click chips to toggle · c clear tags · / filter · : command · :q close' + : '↑/↓ move · Enter open · click chips to toggle'}
)} {tagMenu && ( diff --git a/packages/app-core/src/components/TaskStateBox.tsx b/packages/app-core/src/components/TaskStateBox.tsx index 816a8961..13779eb2 100644 --- a/packages/app-core/src/components/TaskStateBox.tsx +++ b/packages/app-core/src/components/TaskStateBox.tsx @@ -22,6 +22,10 @@ interface Props { className?: string /** Stop the pointer events that would otherwise start a card drag. */ stopPointerEvents?: boolean + /** The key that toggles the task from the keyboard, named in the tooltip. + * Left out with Vim mode off, where no such key exists: the tooltip then + * promises only what the click does. */ + toggleKey?: string | null } function stateLabel(task: VaultTask): string { @@ -36,7 +40,8 @@ export function TaskStateBox({ onToggle, idleClassName = 'border border-current/40 hover:bg-current/10', className = 'mt-0.5', - stopPointerEvents = false + stopPointerEvents = false, + toggleKey = null }: Props): JSX.Element { const stopper = stopPointerEvents ? { @@ -50,7 +55,7 @@ export function TaskStateBox({ role="checkbox" aria-checked={task.checked} draggable={false} - title={`${stateLabel(task)}Toggle task (x)`} + title={`${stateLabel(task)}Toggle task${toggleKey ? ` (${toggleKey})` : ''}`} {...stopper} onClick={(e) => { e.stopPropagation() diff --git a/packages/app-core/src/components/TasksCalendar.tsx b/packages/app-core/src/components/TasksCalendar.tsx index ba699b1e..68181c70 100644 --- a/packages/app-core/src/components/TasksCalendar.tsx +++ b/packages/app-core/src/components/TasksCalendar.tsx @@ -368,6 +368,12 @@ export function TasksCalendar({ } if (e.metaKey || e.ctrlKey || e.altKey) return + // With Vim off the single-character keys stay with the page, the rule + // every list in the app follows: arrows, Enter, Space, Tab and Escape + // are universal, the letters (and < > [ ]) are Vim's. A stray letter + // then never edits, deletes, moves or reschedules the task under the + // cursor, and the hint line names nothing that is not live. + if (!vimMode && e.key.length === 1 && e.key !== ' ') return // Grab & place: while a task is picked up, the grid navigation chooses a // target day; Enter places it (move / set-due choice), Esc cancels. @@ -591,7 +597,8 @@ export function TasksCalendar({ onToggleTask, onRescheduleTask, onMoveTask, - deleteTaskFromList + deleteTaskFromList, + vimMode ]) const focusedTaskRef = useRef(null) @@ -634,7 +641,9 @@ export function TasksCalendar({
{grabbedTask ? `Moving “${grabbedTask.content || 'task'}” — h/j/k/l pick a day · Enter place · Esc cancel` - : 'h/j/k/l day · Tab pick · x toggle · e edit · dd del · m move · < > / T due · a add'} + : vimMode + ? 'h/j/k/l day · Tab pick · x toggle · e edit · dd del · m move · < > / T due · a add' + : '←/→/↑/↓ day · Tab pick · Space toggle · drag to move · right-click actions'}
diff --git a/packages/app-core/src/components/TasksKanban.tsx b/packages/app-core/src/components/TasksKanban.tsx index 249a21f9..492c57f2 100644 --- a/packages/app-core/src/components/TasksKanban.tsx +++ b/packages/app-core/src/components/TasksKanban.tsx @@ -47,6 +47,7 @@ import { InlineMarkdown } from '../lib/inline-markdown' import { CloudTaskConflictIndicator } from './CloudTaskConflictIndicator' import { TaskStateBox } from './TaskStateBox' import { isImeComposing } from '../lib/ime' +import { createDragAutoScroller } from '../lib/drag-autoscroll' import { getSequenceTokens, sequenceTokenFromEvent, @@ -600,6 +601,9 @@ interface ActiveColumnDrag { pointerId: number startX: number startY: number + /** Latest pointer position, to re-aim when the board scrolls under a still pointer. */ + pointerX: number + pointerY: number dragging: boolean /** Column the pointer is currently over, and which side to insert on. */ targetId: string | null @@ -612,6 +616,9 @@ interface ActivePointerDrag { sourceColumnId: string | null startX: number startY: number + /** Latest pointer position, to re-aim when the board scrolls under a still pointer. */ + pointerX: number + pointerY: number offsetX: number offsetY: number width: number @@ -703,6 +710,23 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: // #573: a pressed `g` waiting out the gt/gT window before cycling group-by. const groupByPendingRef = useRef(false) const groupByTimerRef = useRef(null) + // Edge auto-scroll for both drags (#838): near the board's left or right edge + // the board scrolls sideways, and a card held near the top or bottom of a + // long column scrolls that column. + const [cardAutoScroll] = useState(() => + createDragAutoScroller({ + horizontal: () => boardRef.current, + vertical: (clientX, clientY) => { + const body = ( + document.elementFromPoint(clientX, clientY) as HTMLElement | null + )?.closest('[data-kanban-column-body]') + return body && boardRef.current?.contains(body) ? body : null + } + }) + ) + const [columnAutoScroll] = useState(() => + createDragAutoScroller({ horizontal: () => boardRef.current }) + ) const openTaskMenu = useCallback( (e: React.MouseEvent, task: VaultTask): void => { @@ -910,6 +934,8 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: pointerId: e.pointerId, startX: e.clientX, startY: e.clientY, + pointerX: e.clientX, + pointerY: e.clientY, dragging: false, targetId: null, insertAfter: false @@ -928,12 +954,36 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: const targetIdx = ids.indexOf(drag.targetId) if (targetIdx < 0) return ids.splice(drag.insertAfter ? targetIdx + 1 : targetIdx, 0, drag.columnId) + // The cursor moves with the dropped column, as it does for `<` / `>`, so + // the focus-follow scroll stays on the column that was just placed + // instead of chasing whichever column slid under the old cursor (#838). + setColIdx(ids.indexOf(drag.columnId)) setKanbanColumnOrder(groupBy, ids) }, [groupBy, setKanbanColumnOrder] ) useEffect(() => { + const aimColumnDrag = (drag: ActiveColumnDrag): void => { + const columnEl = ( + document.elementFromPoint(drag.pointerX, drag.pointerY) as HTMLElement | null + )?.closest('[data-kanban-column-id]') + const targetId = columnEl?.dataset.kanbanColumnId ?? null + if (!columnEl || !targetId || targetId === NO_VALUE_COLUMN_ID || targetId === drag.columnId) { + drag.targetId = null + setColumnDropTarget(null) + return + } + const rect = columnEl.getBoundingClientRect() + const after = drag.pointerX > rect.left + rect.width / 2 + drag.targetId = targetId + drag.insertAfter = after + // Kept referentially stable while the aim holds: a scrolling board + // re-aims every frame, and a fresh object would re-render every frame. + setColumnDropTarget((current) => + current?.id === targetId && current.after === after ? current : { id: targetId, after } + ) + } const handleMove = (e: PointerEvent): void => { const drag = columnDragRef.current if (!drag || drag.pointerId !== e.pointerId) return @@ -950,25 +1000,22 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: document.body.style.userSelect = 'none' } e.preventDefault() - const columnEl = (document.elementFromPoint(e.clientX, e.clientY) as HTMLElement | null)?.closest( - '[data-kanban-column-id]' - ) - const targetId = columnEl?.dataset.kanbanColumnId ?? null - if (!targetId || targetId === NO_VALUE_COLUMN_ID || targetId === drag.columnId) { - drag.targetId = null - setColumnDropTarget(null) - return - } - const rect = columnEl!.getBoundingClientRect() - const after = e.clientX > rect.left + rect.width / 2 - drag.targetId = targetId - drag.insertAfter = after - setColumnDropTarget({ id: targetId, after }) + drag.pointerX = e.clientX + drag.pointerY = e.clientY + aimColumnDrag(drag) + columnAutoScroll.update(e.clientX, e.clientY) + } + // The board scrolling under a still pointer (the edge auto-scroll, a + // wheel) moves a different column beneath it without any pointermove. + const handleScroll = (): void => { + const drag = columnDragRef.current + if (drag?.dragging) aimColumnDrag(drag) } const handleUp = (e: PointerEvent): void => { const drag = columnDragRef.current if (!drag || drag.pointerId !== e.pointerId) return columnDragRef.current = null + columnAutoScroll.stop() if (drag.dragging) { e.preventDefault() finishColumnDrag(drag) @@ -982,19 +1029,24 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: const drag = columnDragRef.current if (!drag || drag.pointerId !== e.pointerId) return columnDragRef.current = null + columnAutoScroll.stop() setDraggingColumnId(null) setColumnDropTarget(null) document.body.style.userSelect = '' } + const board = boardRef.current window.addEventListener('pointermove', handleMove, { passive: false }) window.addEventListener('pointerup', handleUp, { passive: false }) window.addEventListener('pointercancel', handleCancel) + board?.addEventListener('scroll', handleScroll, { capture: true, passive: true }) return () => { window.removeEventListener('pointermove', handleMove) window.removeEventListener('pointerup', handleUp) window.removeEventListener('pointercancel', handleCancel) + board?.removeEventListener('scroll', handleScroll, { capture: true }) + columnAutoScroll.stop() } - }, [finishColumnDrag]) + }, [columnAutoScroll, finishColumnDrag]) useEffect(() => { if (!editingColumnId) return @@ -1347,8 +1399,28 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: columnId === drag.sourceColumnId ? [] : dropMutationsFor(groupBy, columnId, drag.task, today) - if (mutations) { - moveTaskOnBoard(drag.task, mutations, columnId, insertionIndex) + if (!mutations) return + // The cursor lands on the dropped card, the way a click, a right-click + // and Shift+H/L already put it on theirs. It moves BEFORE the card does: + // the move flushes the board rebuild synchronously, and a rebuild under + // the old cursor scrolls the board back to it, a whole board away once + // an edge auto-scroll carried the card there (#838). The insertion index + // is exact for the visible column; a drop without one resolves below. + const movedKey = taskIdentityKey(drag.task) + const targetColIdx = columnsRef.current.findIndex((column) => column.id === columnId) + if (targetColIdx >= 0) { + const currentIdx = columnsRef.current[targetColIdx].tasks.findIndex( + (task) => taskIdentityKey(task) === movedKey + ) + setColIdx(targetColIdx) + setCardIdx(insertionIndex ?? Math.max(0, currentIdx)) + } + moveTaskOnBoard(drag.task, mutations, columnId, insertionIndex) + if (mutations.length > 0) { + // That flushed rebuild already ran, so the board knows where the card landed. + const next = cursorAfterCardMove(columnsRef.current, columnId, movedKey) + setColIdx(next.colIdx) + setCardIdx(next.cardIdx) } }, [columnAtPoint, dndEnabled, groupBy, moveTaskOnBoard, today] @@ -1368,6 +1440,8 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: sourceColumnId, startX: e.clientX, startY: e.clientY, + pointerX: e.clientX, + pointerY: e.clientY, offsetX: e.clientX - rect.left, offsetY: e.clientY - rect.top, width: rect.width, @@ -1381,6 +1455,20 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: ) useEffect(() => { + const aimPointerDrag = (drag: ActivePointerDrag): void => { + const target = columnAtPoint(drag.pointerX, drag.pointerY) + if (target?.id && dndEnabled) { + drag.lastColumnId = target.id + markDropTarget(target.id, target.element) + drag.lastInsertionIndex = updateDropIndicator(drag, target, drag.pointerY) + } else { + drag.lastColumnId = null + drag.lastInsertionIndex = null + clearDropTarget() + hideDropIndicator() + } + } + const handlePointerMove = (e: PointerEvent): void => { const drag = pointerDragRef.current if (!drag || drag.pointerId !== e.pointerId) return @@ -1406,17 +1494,18 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: e.preventDefault() scheduleDragPreviewPosition(drag, e) - const target = columnAtPoint(e.clientX, e.clientY) - if (target?.id && dndEnabled) { - drag.lastColumnId = target.id - markDropTarget(target.id, target.element) - drag.lastInsertionIndex = updateDropIndicator(drag, target, e.clientY) - } else { - drag.lastColumnId = null - drag.lastInsertionIndex = null - clearDropTarget() - hideDropIndicator() - } + drag.pointerX = e.clientX + drag.pointerY = e.clientY + aimPointerDrag(drag) + cardAutoScroll.update(e.clientX, e.clientY) + } + + // The board or a column scrolling under a still pointer (the edge + // auto-scroll, a wheel) slides other cards and columns beneath it with no + // pointermove, so the highlight and the insertion line would go stale. + const handleScroll = (): void => { + const drag = pointerDragRef.current + if (drag?.dragging) aimPointerDrag(drag) } const handlePointerUp = (e: PointerEvent): void => { @@ -1424,6 +1513,7 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: if (!drag || drag.pointerId !== e.pointerId) return pointerDragRef.current = null + cardAutoScroll.stop() if (drag.dragging) { e.preventDefault() setDraggingId(null) @@ -1442,6 +1532,7 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: const drag = pointerDragRef.current if (!drag || drag.pointerId !== e.pointerId) return pointerDragRef.current = null + cardAutoScroll.stop() setDraggingId(null) document.body.style.userSelect = '' clearDropTarget() @@ -1449,18 +1540,23 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: clearDragPreview() } + const board = boardRef.current window.addEventListener('pointermove', handlePointerMove, { passive: false }) window.addEventListener('pointerup', handlePointerUp, { passive: false }) window.addEventListener('pointercancel', handlePointerCancel, { passive: false }) + board?.addEventListener('scroll', handleScroll, { capture: true, passive: true }) return () => { window.removeEventListener('pointermove', handlePointerMove) window.removeEventListener('pointerup', handlePointerUp) window.removeEventListener('pointercancel', handlePointerCancel) + board?.removeEventListener('scroll', handleScroll, { capture: true }) + cardAutoScroll.stop() document.body.style.userSelect = '' hideDropIndicator() clearDragPreview() } }, [ + cardAutoScroll, clearDragPreview, clearDropTarget, columnAtPoint, @@ -1525,6 +1621,12 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: } if (e.metaKey || e.ctrlKey || e.altKey) return + // With Vim off the single-character keys stay with the page, the rule + // every list in the app follows: arrows, Enter, Space and Escape are + // universal, the letters (and < > H L) are Vim's. The board kept them + // live for a while after the lists were gated; its hint line named + // h/l, j/k and x to people who never asked for them. + if (!vimMode && e.key.length === 1 && e.key !== ' ') return const consume = (): void => { e.preventDefault() @@ -1674,9 +1776,13 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: )}
- {dndEnabled - ? 'Drag or Shift+H·L move card · drag header or reorder columns · h/l · j/k · g group-by · x · Enter · right-click actions' - : 'Drag header or reorder columns · h/l column · j/k card · g group-by · x · Enter · right-click actions'} + {vimMode + ? dndEnabled + ? 'Drag or Shift+H·L move card · drag header or reorder columns · h/l · j/k · g group-by · x · Enter · right-click actions' + : 'Drag header or reorder columns · h/l column · j/k card · g group-by · x · Enter · right-click actions' + : dndEnabled + ? 'Drag to move a card · drag a header to reorder columns · ←/→ · ↑/↓ · Space · Enter · right-click actions' + : 'Drag a header to reorder columns · ←/→ column · ↑/↓ card · Space · Enter · right-click actions'}
@@ -1831,6 +1937,7 @@ export function TasksKanban({ tasks, filter, today, onOpenTask, onToggleTask }: onToggle={() => onToggleTask(task)} onPointerDown={(e) => beginPointerDrag(task, e)} onContextMenu={openTaskMenu} + toggleKey={vimMode ? 'x' : null} /> ) })} @@ -1901,6 +2008,8 @@ interface CardProps { onPointerDown: (e: React.PointerEvent) => void /** Right-click actions. Omitted on the drag preview, which is not a real card. */ onContextMenu?: (e: React.MouseEvent, task: VaultTask) => void + /** The key the checkbox tooltip names; none with Vim mode off. */ + toggleKey?: string | null } function formatDue(iso: string | undefined): string { @@ -1923,7 +2032,8 @@ function TaskCard({ shouldSuppressClick, onToggle, onPointerDown, - onContextMenu + onContextMenu, + toggleKey = null }: CardProps): JSX.Element { return (
{/* The card body stays focusable so clicks open the note and drags move it. */}
(null) const draggable = !!onReorder @@ -131,7 +134,7 @@ export function TasksRow({ {dropPos === 'after' && ( )} - +
{ e.stopPropagation() onOpen() diff --git a/packages/app-core/src/components/TasksView.tsx b/packages/app-core/src/components/TasksView.tsx index 1a4518a8..61f7e967 100644 --- a/packages/app-core/src/components/TasksView.tsx +++ b/packages/app-core/src/components/TasksView.tsx @@ -782,7 +782,7 @@ export function TasksView(): JSX.Element { key={id} type="button" onClick={() => setViewMode(id)} - title={`${label} (${shortcut})`} + title={vimMode ? `${label} (${shortcut})` : label} className={[ 'flex items-center gap-1 rounded px-2 py-1 text-xs transition-colors', isActive @@ -825,7 +825,7 @@ export function TasksView(): JSX.Element { type="button" onClick={() => void newTaskFile()} className="rounded-md border border-accent/45 bg-accent/10 px-2 py-1 text-xs font-medium text-accent hover:bg-accent/20" - title="New task (a)" + title={vimMode ? 'New task (a)' : 'New task'} > + New task @@ -947,6 +947,7 @@ export function TasksView(): JSX.Element { onReorder={reorderTaskByDrag} onContextMenu={openTaskMenu} toggleKeyLabel={toggleKeyLabel} + vimMode={vimMode} /> ) })} @@ -1012,11 +1013,9 @@ export function TasksView(): JSX.Element { /> ) : ( - /* Each line names only keys that fire in the current mode. The list's - single keys are Vim-gated (arrows, Enter and the Shift+J/K chord - are the universal ones); the board and the calendar predate that - gating and keep single-key navigation with Vim off, so their lines - lose only i / c / :q, which are gated on every surface. */ + /* Each line names only keys that fire in the current mode. Single + keys are Vim's on every surface; arrows, Enter, Space, Tab and the + Shift+J/K chord are the universal ones. */
{viewMode === 'list' ? vimMode @@ -1025,10 +1024,10 @@ export function TasksView(): JSX.Element { : viewMode === 'calendar' ? vimMode ? 'h/j/k/l day · [ ] month · Tab pick · x toggle · i start · c cancel · F saved filters · drag to move · right-click actions · :q' - : 'h/j/k/l day · [ ] month · Tab pick · x toggle · drag to move · right-click actions' + : '←/→/↑/↓ day · Tab pick · Space toggle · drag to move · right-click actions' : vimMode ? 'h/l column · j/k card · x toggle · i start · c cancel · Enter open · F saved filters · right-click actions · :q close' - : 'h/l column · j/k card · x toggle · Enter open · right-click actions'} + : '←/→ column · ↑/↓ card · Space toggle · Enter open · right-click actions'}
)} diff --git a/packages/app-core/src/components/WorkflowsView.tsx b/packages/app-core/src/components/WorkflowsView.tsx index 671f7617..54a45970 100644 --- a/packages/app-core/src/components/WorkflowsView.tsx +++ b/packages/app-core/src/components/WorkflowsView.tsx @@ -86,6 +86,7 @@ import type { } from '@shared/workflows/types' import { useStore } from '../store' import type { WorkflowRunRecord } from '../store' +import { getSystemFolderLabel } from '../lib/system-folder-labels' import { useToastStore } from '../lib/toast' import { createVaultReader } from '../lib/workflow-vault-reader' import { canManageWorkflows } from '../lib/workflow-workspace' @@ -132,6 +133,7 @@ import { irreversibleNote, opsExcludingPaths, planWritePaths, + promisedMoves, receiptHeadline, resolveTemplateOps, runConfirmDescription, @@ -143,8 +145,8 @@ import { undoneHeadline, unknownTemplateDiagnostics, unsavedCollisionDescription, - unsavedCollisionTitle, unsavedCollisions, + unsavedCollisionTitle, unsavedSkipDescription, unsavedSkipTitle } from '../lib/workflow-run' @@ -161,7 +163,7 @@ import { ContextMenu } from './ContextMenu' import type { ContextMenuItem } from './ContextMenu' import { CloseIcon, PencilIcon, PlusIcon, TrashIcon, ZapIcon } from './icons' import { NodeInspector } from './workflows/NodeInspector' -import type { InspectorVocabulary } from './workflows/NodeInspector' +import type { ComboboxOption, InspectorVocabulary } from './workflows/NodeInspector' import { ImportReviewDialog } from './workflows/ImportReviewDialog' import { TutorialPanel } from './workflows/TutorialPanel' import { WorkflowListPane } from './workflows/WorkflowListPane' @@ -211,7 +213,7 @@ const MODEL_RULES: readonly string[] = [ const HEADER_KEY_HELP: Record = { name: 'What it is called, and the filename it saves under.', description: 'One line, shown in the list on the left.', - trigger: `manual, "on ", or "schedule ". Events: ${WORKFLOW_EVENTS.join(', ')}.`, + trigger: `manual, "on ", or "schedule ". Events: ${WORKFLOW_EVENTS.join(', ')}. An event fires for the change you make in this app, on this device, and the run sees only the note that changed; "on where " fires only when that note matches. Schedules parse but do not fire yet.`, // Stated here as well as shown as a badge, because the file is the state: // someone reading the `.md` has to be able to tell whether it can act. status: 'draft or active. A draft is saved but cannot run. Missing means active.', @@ -1122,6 +1124,8 @@ const CARET_KEYS: ReadonlySet = new Set([ */ export function WorkflowsView(): JSX.Element { const notes = useStore((s) => s.notes) + const primaryNotesAtRoot = useStore((s) => s.vaultSettings.primaryNotesLocation === 'root') + const systemFolderLabels = useStore((s) => s.systemFolderLabels) const selectedPath = useStore((s) => s.selectedPath) const vimMode = useStore((s) => s.vimMode) const keymapOverrides = useStore((s) => s.keymapOverrides) @@ -1749,27 +1753,43 @@ export function WorkflowsView(): JSX.Element { }, [plan]) /** - * Every folder a note lives in, ancestors included. + * The four system names, then every folder a note lives in, ancestors + * included. * - * Derived from the note paths rather than from the store's folder tree - * because that is exactly what the engine sees: `WorkflowNote.folder` is the - * vault-relative DIRECTORY, while `NoteMeta.folder` is the system bucket. A - * combobox offering the bucket would suggest folders that match nothing. + * Directories are derived from the note paths rather than from the store's + * folder tree because that is exactly what the engine sees: + * `WorkflowNote.folder` is the vault-relative DIRECTORY, while + * `NoteMeta.folder` is the system bucket. The system names lead because the + * engine reads them as the system folders wherever the vault keeps them, and + * each carries the name the sidebar shows for it. On a vault whose notes + * live at the root, `inbox` is the root itself: the one folder no directory + * name could offer, and the one a list of directories sent someone looking + * for in vain (#840). */ - const vaultFolders = useMemo(() => { - const seen = new Set() + const vaultFolders = useMemo(() => { + const buckets = (['inbox', 'quick', 'archive', 'trash'] as const).map((bucket) => ({ + value: bucket, + hint: + bucket === 'inbox' && primaryNotesAtRoot + ? 'Vault root' + : getSystemFolderLabel(bucket, systemFolderLabels) + })) + const seen = new Set(buckets.map((bucket) => bucket.value)) + const directories: string[] = [] for (const note of notes) { const cut = note.path.lastIndexOf('/') if (cut === -1) continue let directory = note.path.slice(0, cut) while (directory !== '' && !seen.has(directory)) { seen.add(directory) + directories.push(directory) const up = directory.lastIndexOf('/') directory = up === -1 ? '' : directory.slice(0, up) } } - return [...seen].sort((a, b) => a.localeCompare(b)) - }, [notes]) + directories.sort((a, b) => a.localeCompare(b)) + return [...buckets, ...directories.map((value) => ({ value }))] + }, [notes, primaryNotesAtRoot, systemFolderLabels]) // A diagnostic carries a line, and a whole pipeline lives on one line, so a // bad step marks every step of its statement. Line is the finest grain the @@ -3619,11 +3639,25 @@ export function WorkflowsView(): JSX.Element { // on disk, so the edits have to be on disk before the run reads them. await Promise.all(unsaved.paths.map((path) => persistNote(path))) } - const receipt = await window.zen.applyWorkflow({ - workflowId: item.workflow.id, - ops: withTemplates.ops - }) - setRecord({ workflowId: item.workflow.id, receipt, undone: null, undoError: null }) + // An editor open on a note the run moves follows it (see the store's + // `followWorkflowMoves`): shielded from the unlink echo during the run, + // carried to the new path after it. + const moves = promisedMoves( + withTemplates.ops, + useStore.getState().vaultSettings.systemFolderPaths + ) + const settle = useStore.getState().followWorkflowMoves(moves) + const receipt = await (async () => { + try { + return await window.zen.applyWorkflow({ + workflowId: item.workflow.id, + ops: withTemplates.ops + }) + } finally { + await settle() + } + })() + setRecord({ workflowId: item.workflow.id, receipt, undone: null, undoError: null, moves }) // The two op kinds the main process cannot perform (a toast and the // clipboard live here, not there) happen now, and only for a run that // stood: a rolled-back run announcing itself, or writing its output to @@ -3692,8 +3726,20 @@ export function WorkflowsView(): JSX.Element { } setUndoing(true) + // The notes the run moved go back, and an editor open on one goes back + // with it, the way it followed the run forward. + const settle = useStore.getState().followWorkflowMoves( + (current.moves ?? []).map(({ from, to }) => ({ from: to, to: from })), + { reverting: true } + ) try { - const result = await window.zen.undoWorkflowRun(current.receipt.runId) + const result = await (async () => { + try { + return await window.zen.undoWorkflowRun(current.receipt.runId) + } finally { + await settle() + } + })() setRecord((latest) => latest && latest.receipt.runId === current.receipt.runId ? { ...latest, undone: result, undoError: null } diff --git a/packages/app-core/src/components/workflows/NodeInspector.tsx b/packages/app-core/src/components/workflows/NodeInspector.tsx index 5f711e9f..044c4466 100644 --- a/packages/app-core/src/components/workflows/NodeInspector.tsx +++ b/packages/app-core/src/components/workflows/NodeInspector.tsx @@ -48,11 +48,23 @@ export interface InspectorVocabulary { tags: readonly string[] fields: readonly string[] paths: readonly string[] - folders: readonly string[] + folders: readonly ComboboxOption[] wires: readonly string[] workflows: readonly string[] } +/** + * An entry a combobox can offer: the value the argument takes, and a hint + * shown beside it when the value alone does not say what it means. `inbox` + * reads as "Vault root" on a vault whose notes live there, which is the one + * folder a list of directories could never carry (#840). Typing matches the + * hint too, so "root" finds it. + */ +export interface ComboboxOption { + value: string + hint?: string +} + /** * Fields every note has, whether or not anyone wrote frontmatter. * @@ -65,11 +77,21 @@ const BUILTIN_FIELDS: readonly string[] = ['title', 'path', 'folder', 'created', /** Enough options to recognize what exists, not a whole vault in a dropdown. */ const OPTION_LIMIT = 60 -function optionsFor(all: readonly string[], text: string): string[] { +function optionsFor( + all: readonly (string | ComboboxOption)[], + text: string +): ComboboxOption[] { const needle = text.trim().toLowerCase() - const out: string[] = [] - for (const option of all) { - if (needle !== '' && !option.toLowerCase().includes(needle)) continue + const out: ComboboxOption[] = [] + for (const entry of all) { + const option = typeof entry === 'string' ? { value: entry } : entry + if ( + needle !== '' && + !option.value.toLowerCase().includes(needle) && + !(option.hint ?? '').toLowerCase().includes(needle) + ) { + continue + } out.push(option) if (out.length >= OPTION_LIMIT) break } @@ -128,7 +150,7 @@ function ThemedCombobox({ onChange }: { value: string - options: readonly string[] + options: readonly ComboboxOption[] disabled: boolean label: string placeholder?: string @@ -192,7 +214,7 @@ function ThemedCombobox({ setCursor((c) => (c - 1 + visible.length) % visible.length) } else if (event.key === 'Enter') { event.preventDefault() - commit(visible[active]) + commit(visible[active].value) } else if (event.key === 'Escape') { // Swallowed so the inspector stays open: Escape here means "I am // done with this list", not "leave the workflow". @@ -209,7 +231,7 @@ function ThemedCombobox({ className="absolute left-0 right-0 z-dropdown mt-1 max-h-48 overflow-auto rounded-md border border-paper-300 bg-paper-100 py-1 shadow-float" > {visible.map((option, index) => ( -
  • +
    • ))} @@ -318,7 +343,10 @@ function ParamControl({ ) } - const combobox = (all: readonly string[], placeholder?: string): JSX.Element => ( + const combobox = ( + all: readonly (string | ComboboxOption)[], + placeholder?: string + ): JSX.Element => ( { + const state = useStore.getState() + return moveNoteVocabulary(state.vaultSettings, state.systemFolderLabels, state.folders) + } const validate = (value: string): string | null => - validateMoveDirectoryTarget(directory, value, useStore.getState().folders) + validateMoveDirectoryTarget(directory, value, useStore.getState().folders, vocabulary()) const target = await promptApp({ - ...buildMoveDirectoryPrompt(directory, useStore.getState().folders), + ...buildMoveDirectoryPrompt(directory, useStore.getState().folders, vocabulary()), validate }) - if (!target || validate(target)) return 'cancelled' - const parent = parseMoveNoteTarget(target).subpath + // Empty is an answer (the notes root); only null is the Cancel. + if (target === null || validate(target)) return 'cancelled' + const parent = parseMoveNoteTarget(target, vocabulary()).subpath if (parent === parentDirOf(directory)) return 'cancelled' if (!isCurrent() || !targetExists()) return 'stale' const leaf = directory.split('/').pop()! diff --git a/packages/app-core/src/lib/commands.test.ts b/packages/app-core/src/lib/commands.test.ts index b7d4876d..b30fa942 100644 --- a/packages/app-core/src/lib/commands.test.ts +++ b/packages/app-core/src/lib/commands.test.ts @@ -183,9 +183,17 @@ describe('Workflow run entries', () => { name: 'Reading log', description: 'Keep the table in sync', status: 'active' as const, + trigger: { type: 'manual' as const }, mutates: true }, - { id: 'half-idea', name: 'Half idea', description: '', status: 'draft' as const, mutates: false } + { + id: 'half-idea', + name: 'Half idea', + description: '', + status: 'draft' as const, + trigger: { type: 'manual' as const }, + mutates: false + } ] it('lists one Run entry per active workflow and none for drafts', async () => { diff --git a/packages/app-core/src/lib/commands.ts b/packages/app-core/src/lib/commands.ts index b3f6347a..03d01c42 100644 --- a/packages/app-core/src/lib/commands.ts +++ b/packages/app-core/src/lib/commands.ts @@ -10,7 +10,7 @@ import { isTagsViewActive, isTasksViewActive, isTrashViewActive, useStore } from import { confirmApp } from './confirm-requests' import { promptApp } from './prompt-requests' import { captureNavigationContext } from './navigation-context' -import { buildMoveNotePrompt, parseMoveNoteTarget } from './move-note' +import { buildMoveNotePrompt, moveNoteVocabulary, parseMoveNoteTarget } from './move-note' import { focusPaneInDirection } from './pane-nav' import { focusSidebarPanel } from './sidebar-focus' import { findLeaf } from './pane-layout' @@ -550,9 +550,11 @@ export function buildCommands(options?: { includeUnavailable?: boolean }): Comma const state = getState() const active = state.activeNote if (!active) return - const target = await promptApp(buildMoveNotePrompt(active, state.folders)) - if (!target || !isCurrent()) return - const dest = parseMoveNoteTarget(target) + const vocabulary = moveNoteVocabulary(state.vaultSettings, state.systemFolderLabels, state.folders) + const target = await promptApp(buildMoveNotePrompt(active, state.folders, vocabulary)) + // Empty is an answer (the notes root); only null is the Cancel. + if (target === null || !isCurrent()) return + const dest = parseMoveNoteTarget(target, vocabulary) await state.moveNote(active.path, dest.folder, dest.subpath, isCurrent) } } diff --git a/packages/app-core/src/lib/drag-autoscroll.test.ts b/packages/app-core/src/lib/drag-autoscroll.test.ts new file mode 100644 index 00000000..1f912021 --- /dev/null +++ b/packages/app-core/src/lib/drag-autoscroll.test.ts @@ -0,0 +1,187 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createDragAutoScroller, + DRAG_AUTOSCROLL_BAND, + DRAG_AUTOSCROLL_MAX_SPEED, + edgeScrollVelocity +} from './drag-autoscroll' + +describe('edgeScrollVelocity (#838)', () => { + it('is still between the bands', () => { + expect(edgeScrollVelocity(500, 0, 1000)).toBe(0) + expect(edgeScrollVelocity(DRAG_AUTOSCROLL_BAND, 0, 1000)).toBe(0) + expect(edgeScrollVelocity(1000 - DRAG_AUTOSCROLL_BAND, 0, 1000)).toBe(0) + }) + + it('scrolls toward the edge the pointer is near, faster the deeper it sits', () => { + const shallow = edgeScrollVelocity(1000 - DRAG_AUTOSCROLL_BAND / 4, 0, 1000) + const deep = edgeScrollVelocity(999, 0, 1000) + expect(shallow).toBeGreaterThan(0) + expect(deep).toBeGreaterThan(shallow) + expect(edgeScrollVelocity(DRAG_AUTOSCROLL_BAND / 4, 0, 1000)).toBe(-shallow) + }) + + it('holds the top speed at and past the edge, so an overshoot keeps going', () => { + expect(edgeScrollVelocity(1000, 0, 1000)).toBe(DRAG_AUTOSCROLL_MAX_SPEED) + expect(edgeScrollVelocity(1400, 0, 1000)).toBe(DRAG_AUTOSCROLL_MAX_SPEED) + expect(edgeScrollVelocity(-300, 0, 1000)).toBe(-DRAG_AUTOSCROLL_MAX_SPEED) + }) + + it('keeps a dead zone in the middle of a scroller narrower than two bands', () => { + // 90px wide: the bands shrink to a third each, so the middle third is still. + expect(edgeScrollVelocity(45, 0, 90)).toBe(0) + expect(edgeScrollVelocity(10, 0, 90)).toBeLessThan(0) + expect(edgeScrollVelocity(80, 0, 90)).toBeGreaterThan(0) + }) + + it('never scrolls a scroller with no size', () => { + expect(edgeScrollVelocity(0, 0, 0)).toBe(0) + expect(edgeScrollVelocity(5, 10, 0)).toBe(0) + }) +}) + +describe('createDragAutoScroller (#838)', () => { + let frames: Map + let nextFrame: number + let now: number + + beforeEach(() => { + frames = new Map() + nextFrame = 1 + now = 1000 + vi.spyOn(performance, 'now').mockImplementation(() => now) + vi.spyOn(window, 'requestAnimationFrame').mockImplementation((callback) => { + const id = nextFrame++ + frames.set(id, callback) + return id + }) + vi.spyOn(window, 'cancelAnimationFrame').mockImplementation((id) => { + frames.delete(id) + }) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + /** Advance one ~60 Hz frame; true if a frame was waiting. */ + function frame(ms = 16): boolean { + const [id, callback] = frames.entries().next().value ?? [] + if (id === undefined || !callback) return false + frames.delete(id) + now += ms + callback(now) + return true + } + + /** A scroller of `size` px showing `view` px, laid out at 0..view on both axes. */ + function scroller(size: number, view: number): HTMLElement { + const el = document.createElement('div') + let left = 0 + let top = 0 + const clamp = (value: number): number => Math.max(0, Math.min(size - view, Math.round(value))) + Object.defineProperties(el, { + scrollWidth: { value: size }, + clientWidth: { value: view }, + scrollHeight: { value: size }, + clientHeight: { value: view }, + scrollLeft: { get: () => left, set: (value: number) => (left = clamp(value)) }, + scrollTop: { get: () => top, set: (value: number) => (top = clamp(value)) } + }) + vi.spyOn(el, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, view, view)) + return el + } + + it('keeps scrolling while the pointer rests in a band, with no further moves', () => { + const board = scroller(3000, 1000) + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + autoScroll.update(995, 500) + for (let i = 0; i < 30; i++) frame() + expect(board.scrollLeft).toBeGreaterThan(300) + expect(frames.size).toBe(1) + }) + + it('scrolls back toward the start from the start band', () => { + const board = scroller(3000, 1000) + board.scrollLeft = 2000 + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + autoScroll.update(3, 500) + for (let i = 0; i < 10; i++) frame() + expect(board.scrollLeft).toBeLessThan(2000) + }) + + it('does nothing, and stops asking for frames, between the bands', () => { + const board = scroller(3000, 1000) + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + autoScroll.update(500, 500) + expect(frame()).toBe(true) + expect(board.scrollLeft).toBe(0) + expect(frames.size).toBe(0) + }) + + it('stops once the scroller runs out of room', () => { + const board = scroller(1200, 1000) + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + autoScroll.update(1000, 500) + for (let i = 0; i < 60 && frame(); i++); + expect(board.scrollLeft).toBe(200) + expect(frames.size).toBe(0) + }) + + it('carries sub-pixel steps, so the slow edge of the band still creeps', () => { + const board = scroller(3000, 1000) + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + // Barely inside the band: well under one pixel per frame. + autoScroll.update(1000 - DRAG_AUTOSCROLL_BAND + 2, 500) + frame() + expect(board.scrollLeft).toBe(0) + for (let i = 0; i < 120; i++) frame() + expect(board.scrollLeft).toBeGreaterThan(0) + }) + + it('caps a stalled frame instead of jumping by the whole gap', () => { + const board = scroller(30000, 1000) + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + autoScroll.update(1000, 500) + frame(5000) + expect(board.scrollLeft).toBeLessThanOrEqual(DRAG_AUTOSCROLL_MAX_SPEED * 0.05) + }) + + it('scrolls the vertical target resolved under the pointer', () => { + const board = scroller(3000, 1000) + const column = scroller(2000, 500) + const vertical = vi.fn(() => column) + const autoScroll = createDragAutoScroller({ horizontal: () => board, vertical }) + autoScroll.update(250, 495) + for (let i = 0; i < 10; i++) frame() + expect(column.scrollTop).toBeGreaterThan(0) + expect(board.scrollLeft).toBe(0) + expect(vertical).toHaveBeenLastCalledWith(250, 495) + }) + + it('follows the latest pointer position', () => { + const board = scroller(3000, 1000) + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + autoScroll.update(995, 500) + for (let i = 0; i < 5; i++) frame() + const scrolled = board.scrollLeft + expect(scrolled).toBeGreaterThan(0) + autoScroll.update(500, 500) + frame() + expect(board.scrollLeft).toBe(scrolled) + expect(frames.size).toBe(0) + }) + + it('stop() cancels the pending frame and ignores late frames', () => { + const board = scroller(3000, 1000) + const autoScroll = createDragAutoScroller({ horizontal: () => board }) + autoScroll.update(995, 500) + frame() + const scrolled = board.scrollLeft + autoScroll.stop() + expect(frames.size).toBe(0) + expect(frame()).toBe(false) + expect(board.scrollLeft).toBe(scrolled) + }) +}) diff --git a/packages/app-core/src/lib/drag-autoscroll.ts b/packages/app-core/src/lib/drag-autoscroll.ts new file mode 100644 index 00000000..c16d7798 --- /dev/null +++ b/packages/app-core/src/lib/drag-autoscroll.ts @@ -0,0 +1,127 @@ +/** + * Edge auto-scroll for pointer-driven drags (#838). + * + * The Kanban board moves cards and columns with pointer events rather than + * HTML5 drag-and-drop, so it gets none of the browser's built-in drag + * autoscroll: a board wider than its pane could never carry a card to an + * off-screen column in one drag. Holding the pointer in a band along a + * scroller's edge scrolls it, faster the deeper the pointer sits. A resting + * mouse sends no pointermove, so a frame loop keeps the scroll going until the + * pointer leaves the band, the scroller runs out of room, or the drag ends. + */ + +/** Depth of the hot band inside a scroller's edge, in CSS pixels. */ +export const DRAG_AUTOSCROLL_BAND = 64 +/** Scroll speed with the pointer at (or past) the edge, in CSS pixels per second. */ +export const DRAG_AUTOSCROLL_MAX_SPEED = 1200 + +/** + * Signed velocity, in px/s, along one axis for a pointer at `pointer` over a + * scroller spanning `start`..`end`: negative toward `start`, positive toward + * `end`, zero between the two bands. The speed eases in with depth, so skimming + * the band nudges and pushing into the edge races. Past the edge it holds the + * maximum, so overshooting into the sidebar or a neighbouring pane keeps going. + */ +export function edgeScrollVelocity( + pointer: number, + start: number, + end: number, + band = DRAG_AUTOSCROLL_BAND, + maxSpeed = DRAG_AUTOSCROLL_MAX_SPEED +): number { + // A narrow scroller keeps a dead zone in the middle, or every spot would scroll. + const depth = Math.min(band, (end - start) / 3) + if (!(depth > 0)) return 0 + const intoStart = start + depth - pointer + if (intoStart > 0) return -maxSpeed * Math.min(1, intoStart / depth) ** 2 + const intoEnd = pointer - (end - depth) + if (intoEnd > 0) return maxSpeed * Math.min(1, intoEnd / depth) ** 2 + return 0 +} + +export interface DragAutoScrollTargets { + /** Scrolled sideways by the pointer's x. The pointer may be past its edges. */ + horizontal?: () => HTMLElement | null + /** Scrolled up and down by the pointer's y, resolved under the pointer each frame. */ + vertical?: (clientX: number, clientY: number) => HTMLElement | null +} + +export interface DragAutoScroller { + /** The drag moved: remember where, and scroll while that spot is in a band. */ + update(clientX: number, clientY: number): void + /** The drag ended: stop scrolling and forget the pointer. */ + stop(): void +} + +type Axis = 'x' | 'y' + +export function createDragAutoScroller(targets: DragAutoScrollTargets): DragAutoScroller { + let point: { x: number; y: number } | null = null + let frame: number | null = null + let lastTime = 0 + // Sub-pixel remainders per axis. At the slow end of the ramp a frame asks + // for a fraction of a pixel, which the scroll offset would round away every + // single frame, so the band's inner edge would never move at all. + const carry: Record = { x: 0, y: 0 } + + /** Scrolls one frame's worth; true while this axis still wants more frames. */ + const scrollAxis = (el: HTMLElement, axis: Axis, velocity: number, seconds: number): boolean => { + const position = axis === 'x' ? el.scrollLeft : el.scrollTop + const limit = + axis === 'x' ? el.scrollWidth - el.clientWidth : el.scrollHeight - el.clientHeight + if (velocity === 0 || (velocity < 0 ? position <= 0 : position >= limit - 1)) { + carry[axis] = 0 + return false + } + if (Math.sign(carry[axis]) === -Math.sign(velocity)) carry[axis] = 0 + carry[axis] += velocity * seconds + const whole = Math.trunc(carry[axis]) + if (whole !== 0) { + carry[axis] -= whole + if (axis === 'x') el.scrollLeft = position + whole + else el.scrollTop = position + whole + } + return true + } + + const tick = (now: number): void => { + frame = null + if (!point) return + // A stalled frame (a busy main thread, a hidden window) must not land as + // one giant jump, so the step is capped at a short frame's worth. + const seconds = Math.min(0.05, Math.max(0, (now - lastTime) / 1000)) + lastTime = now + let active = false + const horizontal = targets.horizontal?.() ?? null + if (horizontal) { + const rect = horizontal.getBoundingClientRect() + const velocity = edgeScrollVelocity(point.x, rect.left, rect.right) + active = scrollAxis(horizontal, 'x', velocity, seconds) || active + } + const vertical = targets.vertical?.(point.x, point.y) ?? null + if (vertical) { + const rect = vertical.getBoundingClientRect() + const velocity = edgeScrollVelocity(point.y, rect.top, rect.bottom) + active = scrollAxis(vertical, 'y', velocity, seconds) || active + } else { + carry.y = 0 + } + if (active) frame = requestAnimationFrame(tick) + } + + return { + update(clientX, clientY) { + point = { x: clientX, y: clientY } + if (frame !== null) return + lastTime = performance.now() + frame = requestAnimationFrame(tick) + }, + stop() { + point = null + if (frame !== null) cancelAnimationFrame(frame) + frame = null + carry.x = 0 + carry.y = 0 + } + } +} diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index dd08d3c4..d86959b4 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -129,7 +129,7 @@ export const HELP_HOW_TO_GUIDES: HelpCard[] = [ { title: 'Move a note without dragging', body: - 'Use the note context menu, search for `move` or `mv` in the command palette, or run `:move` or `:mv`. With no argument, ZenNotes opens a folder picker; with a target like `archive/Reference` or `inbox/Work`, it moves the note directly.' + 'Use the note context menu, search for `move` or `mv` in the command palette, or run `:move` or `:mv`. With no argument, ZenNotes opens a folder picker that speaks the sidebar\'s language: folders of your notes area as you see them there (`Work/Research`, no `inbox/` in front, and empty for the notes root, which is Inbox or the vault root depending on your Primary notes location), plus `archive/…` for the Archive. With a target like `:mv Work/Research` or `:mv archive/Reference`, it moves the note directly; `inbox/Work` still works on an Inbox vault.' }, { title: 'Act on multiple sidebar items', @@ -287,7 +287,7 @@ export const HELP_CORE_CONCEPTS: HelpCard[] = [ { title: 'The Tasks Kanban board, custom statuses, and any field', body: - 'Switch Tasks to Kanban (button or `3`) for a column board. "Group by" offers Status (Today / Upcoming / In progress / Waiting / Done, derived from due dates, `[/]` and `@waiting`), Priority, Folder, Custom status, and one entry per inline `@field` you use. The Folder board gives every note folder its own column (`Projects/alpha`, `Areas`, the Inbox root, Quick Notes), so notes that live together group together with nothing to maintain; point it at one folder under Settings → Tasks → Folder board, with `:folderroot Projects` on the board, or with `kanban_folder_root = "Projects"` in config.toml, and that folder\'s children become the columns, deeper notes roll up to their child, and notes outside it share an Other folders column. Folder columns stay read-only: move a note to change its column. On the Status board a started task (`[/]`) sits in its own In progress column between Upcoming and Waiting: drop a card there (or send it with `Shift+L`) to mark it `[/]`, drop it back on Today or Upcoming to reopen it with that date, and a `@waiting` card keeps its `[/]` underneath so clearing the wait returns it to In progress. Any task field works: tag tasks with `@key:value` tokens like `@status:review`, `@sprint:24`, or `@area:backend`, and each key becomes its own board with a column per value (auto-discovered, so it appears the moment you use it). For the status field, define the columns under Settings → Tasks → Kanban statuses, or list them in order in `config.toml` under `[view]`, e.g. `kanban_statuses = ["backlog", "in_progress", "review", "done"]`; other fields sort their columns automatically. A note-level `status:` in frontmatter sets a default for that note’s tasks. A note tagged `task` is a card of its own: its frontmatter `status:` is its custom status, and without one it sits in the trailing No status column until you move it. Everything is keyboard-first: `h`/`l` move between columns, `j`/`k` between cards, `g` cycles the group-by, `Shift+H` / `Shift+L` send the focused card to the previous/next column (rewriting its `@field` token), `<` / `>` reorder the columns themselves (saved per board), and `Space`/`Enter` toggle/open. Drag does the same with the mouse, including dragging a column header to reorder, and dragging a card to a new spot inside its column to hand-prioritize it (that arrangement is saved per column and restored when you come back to the board). Renaming a column (click its title, or `[kanban_column_titles]` in `config.toml`) sets a display label only: the column still shows its underlying `@field:value` beneath the name, and moving a card in writes that value, not the label.' + 'Switch Tasks to Kanban (the button, or `3` in Vim mode) for a column board. "Group by" offers Status (Today / Upcoming / In progress / Waiting / Done, derived from due dates, `[/]` and `@waiting`), Priority, Folder, Custom status, and one entry per inline `@field` you use. The Folder board gives every note folder its own column (`Projects/alpha`, `Areas`, the Inbox root, Quick Notes), so notes that live together group together with nothing to maintain; point it at one folder under Settings → Tasks → Folder board, with `:folderroot Projects` on the board, or with `kanban_folder_root = "Projects"` in config.toml, and that folder\'s children become the columns, deeper notes roll up to their child, and notes outside it share an Other folders column. Folder columns stay read-only: move a note to change its column. On the Status board a started task (`[/]`) sits in its own In progress column between Upcoming and Waiting: drop a card there (or send it with `Shift+L`) to mark it `[/]`, drop it back on Today or Upcoming to reopen it with that date, and a `@waiting` card keeps its `[/]` underneath so clearing the wait returns it to In progress. Any task field works: tag tasks with `@key:value` tokens like `@status:review`, `@sprint:24`, or `@area:backend`, and each key becomes its own board with a column per value (auto-discovered, so it appears the moment you use it). For the status field, define the columns under Settings → Tasks → Kanban statuses, or list them in order in `config.toml` under `[view]`, e.g. `kanban_statuses = ["backlog", "in_progress", "review", "done"]`; other fields sort their columns automatically. A note-level `status:` in frontmatter sets a default for that note’s tasks. A note tagged `task` is a card of its own: its frontmatter `status:` is its custom status, and without one it sits in the trailing No status column until you move it. Everything is keyboard-first: `h`/`l` move between columns, `j`/`k` between cards, `g` cycles the group-by, `Shift+H` / `Shift+L` send the focused card to the previous/next column (rewriting its `@field` token), `<` / `>` reorder the columns themselves (saved per board), and `Space`/`Enter` toggle/open. Drag does the same with the mouse, including dragging a column header to reorder, and dragging a card to a new spot inside its column to hand-prioritize it (that arrangement is saved per column and restored when you come back to the board). On a board wider than the window, hold a dragged card or column header near the board\'s left or right edge and the board scrolls that way, faster the closer you get to the edge; a card held near the top or bottom of a long column scrolls the column. After a drop the cursor sits on the card or column you moved, so the keyboard picks up where the mouse left off. Renaming a column (click its title, or `[kanban_column_titles]` in `config.toml`) sets a display label only: the column still shows its underlying `@field:value` beneath the name, and moving a card in writes that value, not the label.' }, { title: 'Filter the Tasks views to one project', @@ -327,7 +327,7 @@ export const HELP_CORE_CONCEPTS: HelpCard[] = [ { title: 'Moving notes is path-first', body: - 'Use the note context menu, search `move` or `mv` in the command palette, or run `:move` / `:mv` from the ex line to move the active note into Inbox or Archive. With no argument, the command opens the folder picker; with a target like `:mv archive/Reference` or `:move inbox/Work`, it moves the note directly. The move prompt autocompletes folder paths, so you can type and Tab through existing destinations instead of dragging. “Duplicate” (palette or context menu) copies a note in place, appending “ (copy)” to the name.' + 'Use the note context menu, search `move` or `mv` in the command palette, or run `:move` / `:mv` from the ex line to move the active note into a folder of your notes area or the Archive. With no argument, the command opens the folder picker; with a target like `:mv Work/Research` or `:mv archive/Reference`, it moves the note directly (`:move inbox/Work` still works on an Inbox vault). The picker spells destinations the way the sidebar does: no `inbox/` in front, empty for the notes root (Inbox, or the vault root when your notes live there), `archive/…` for the Archive, and it autocompletes folder paths, so you can type and Tab through existing destinations instead of dragging. “Duplicate” (palette or context menu) copies a note in place, appending “ (copy)” to the name.' }, { title: 'Renaming a note fixes its links', @@ -467,12 +467,17 @@ export const HELP_CORE_CONCEPTS: HelpCard[] = [ { title: 'Workflows plan first and write second', body: - 'A workflow is a plain `.md` file under `.zennotes/workflows/`: frontmatter plus one pipeline per line, like `good = books | where rating >= 4`. Wires carry sets of notes, so every wire on the canvas shows the live count flowing through it, and the canvas and the text are lossless projections of the same file (layout is computed, so no coordinates ever land in your vault). The engine can only propose changes: running shows the full dry-run diff before anything is applied, applying journals every file\'s pre-run bytes so Undo restores them exactly, and a run that fails midway rolls the whole thing back on its own. There are no code steps, no shell, and no network, which is why a workflow you did not write is safe to read and run. In this release workflows run when you run them: an event or schedule `trigger:` in the frontmatter parses but does not fire yet. Local desktop vaults, self-hosted web servers, and desktop remote workspaces can all author and run workflows; a remote workspace needs a server on 2.29 or newer, and against an older server workflows stay read-only. The feature is off by default; enable it under Settings → Workflows.' + 'A workflow is a plain `.md` file under `.zennotes/workflows/`: frontmatter plus one pipeline per line, like `good = books | where rating >= 4`. Wires carry sets of notes, so every wire on the canvas shows the live count flowing through it, and the canvas and the text are lossless projections of the same file (layout is computed, so no coordinates ever land in your vault). The engine can only propose changes: running shows the full dry-run diff before anything is applied, applying journals every file\'s pre-run bytes so Undo restores them exactly, and a run that fails midway rolls the whole thing back on its own. There are no code steps, no shell, and no network, which is why a workflow you did not write is safe to read and run. A workflow runs when you run it, and since 2.55 also on its own when its `trigger:` names an event: `on note-created`, `on note-saved`, `on note-moved` or `on tag-added` fires for the note you change in this app, a moment after it settles, with no confirmation and a receipt toast that carries Undo (see the Event triggers card); a `schedule` trigger parses but does not fire yet. Local desktop vaults, self-hosted web servers, and desktop remote workspaces can all author and run workflows; a remote workspace needs a server on 2.29 or newer, and against an older server workflows stay read-only. The feature is off by default; enable it under Settings → Workflows.' }, { title: 'The workflow grammar in one card', body: - 'Six sources open a set: `all`, `folder inbox`, `tag #book`, `search reading list`, `current`, `selection` (the Trash and Archive stay out unless you name them, like `folder trash`). Twelve steps filter and shape it: `where rating >= 4`, `tagged` / `not-tagged #x`, `in inbox/projects`, `matching inbox/**/*.md`, `contains TODO`, `since 7d`, `sort finished desc`, `limit 25`, `dedupe`, and `union` / `subtract ` to combine named wires. Mutating steps change every note on the wire, always behind the dry-run confirmation: `set status done`, `add-tag` / `remove-tag`, `move`, `rename {{date}}-{{title}}`, `append` / `prepend`, `apply-template`, `archive`, `trash`. Outputs turn the wire into text: `render table title, rating` feeds `write "Log.md"`, `write-section "Log.md" "Finished"`, `create-each "inbox/{{title}}.md"`, `notify`, or `clipboard`, and `call ` folds another workflow into the same run. `{{title}}`, `{{date}}`, `{{count}}` and any frontmatter field expand per note. Press `?` in the Workflows view for the live reference, with a one-line description and a real example for every step.' + 'Six sources open a set: `all`, `folder inbox`, `tag #book`, `search reading list`, `current`, `selection` (the Trash and Archive stay out unless you name them, like `folder trash`; the four system names `inbox`, `quick`, `archive`, `trash` mean those folders wherever the vault keeps them, so on a vault whose notes live at the root `folder inbox` is the vault root). Twelve steps filter and shape it: `where rating >= 4`, `tagged` / `not-tagged #x`, `in inbox/projects`, `matching inbox/**/*.md`, `contains TODO`, `since 7d`, `sort finished desc`, `limit 25`, `dedupe`, and `union` / `subtract ` to combine named wires. Mutating steps change every note on the wire, always behind the dry-run confirmation: `set status done`, `add-tag` / `remove-tag`, `move`, `rename {{date}}-{{title}}`, `append` / `prepend`, `apply-template`, `archive`, `trash`. Outputs turn the wire into text: `render table title, rating` feeds `write "Log.md"`, `write-section "Log.md" "Finished"`, `create-each "inbox/{{title}}.md"`, `notify`, or `clipboard`, and `call ` folds another workflow into the same run. `{{title}}`, `{{date}}`, `{{count}}` and any frontmatter field expand per note. Press `?` in the Workflows view for the live reference, with a one-line description and a real example for every step.' + }, + { + title: 'Event triggers', + body: + 'Set `trigger: on note-saved` (or `on note-created`, `on note-moved`, `on tag-added`) on an active workflow and it runs by itself when you make that change in this app, about a second and a half after the note settles: a burst of autosaves while you type is one run, and a note you are still typing in waits for its next save. `note-moved` fires with the note\'s new path for a rename, a move, Archive, Trash and Restore; `tag-added` fires for a save that gave the note a tag it did not have. The run sees only the note that changed: every source (`all`, `folder`, `tag`, `search`, `current`) resolves over that one note, so `all | contains type: topic | move Topics` means "file this note under Topics when it says so", and nothing is read across the vault on every save. Add a condition to fire only when the note matches: `trigger: on note-saved where folder = inbox` takes the same `field op value` a `where` step takes. There is no confirmation; the receipt toast carries Undo, a run that changed nothing says nothing, and a note the run would write that has unsaved edits is left alone and named. Only edits made in this app fire: a change that arrives by sync or from another device does not, what a run itself writes never fires again, a folder rename does not fire for the notes inside it, and the phone apps do not fire at all. `status: draft` or `trigger: manual` silences one workflow; Settings → Workflows → Event triggers (`workflow_event_triggers` under `[view]` in config.toml) silences them all on this device. A `schedule` trigger still parses without firing.' } ] @@ -651,7 +656,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ { id: 'tasks-tags-trash', title: 'Tasks, tags, and trash views', - description: 'These virtual views each run their own keyboard loop in the main pane, and the Vim leader works here too (for example Space h for hint mode).', + description: 'These virtual views each run their own keyboard loop in the main pane, and the Vim leader works here too (for example Space h for hint mode). The single keys below belong to Vim mode: with it off only the arrows, Enter and Escape stay live everywhere, plus Shift+J/K to reorder in the list, Tab to pick a task on the calendar and Space to toggle one on the board or the calendar; everything else sits in the right-click menu.', items: [ { keys: 'j / k', action: 'Move row cursor', detail: 'Step through task rows, tagged notes, or trashed notes.' }, { keys: 'g g / G', action: 'Jump to top or bottom', detail: 'Move to the first or last visible result.' }, @@ -851,7 +856,7 @@ export const HELP_VIM_COMMANDS: HelpExCommand[] = [ { command: ':move [folder] / :mv [folder]', summary: 'Move the active note', - detail: 'Both names are supported explicitly. Without an argument they open the move prompt; with a path like `archive/Reference` or `inbox/Work` they move the active note there directly.' + detail: 'Both names are supported explicitly. Without an argument they open the move prompt; with a path like `Work/Research` (a folder of your notes area, as the sidebar shows it) or `archive/Reference` they move the active note there directly. `inbox/Work` still works on an Inbox vault.' }, { command: ':bn / :bp', diff --git a/packages/app-core/src/lib/move-note.test.ts b/packages/app-core/src/lib/move-note.test.ts new file mode 100644 index 00000000..303279d3 --- /dev/null +++ b/packages/app-core/src/lib/move-note.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it } from 'vitest' +import type { FolderEntry, NoteMeta, VaultSettings } from '@shared/ipc' +import { DEFAULT_VAULT_SETTINGS } from '@shared/ipc' +import { + buildMoveDirectoryPrompt, + buildMoveNotePrompt, + moveNoteVocabulary, + parseMoveNoteTarget, + validateMoveDirectoryTarget, + validateMoveNoteTarget +} from './move-note' + +function settingsFor( + primaryNotesLocation: 'inbox' | 'root', + systemFolderPaths: VaultSettings['systemFolderPaths'] = {} +): VaultSettings { + return { ...DEFAULT_VAULT_SETTINGS, primaryNotesLocation, systemFolderPaths } +} + +function folder(folder: FolderEntry['folder'], subpath: string): FolderEntry { + return { folder, subpath, siblingOrder: 0 } +} + +const FOLDERS = [ + folder('inbox', 'Work'), + folder('inbox', 'Work/Research'), + folder('inbox', 'Areas'), + folder('archive', 'Old'), + folder('quick', ''), + folder('trash', '') +] + +function note(path: string, kind: NoteMeta['folder'] = 'inbox'): Pick { + return { path, folder: kind, title: path.split('/').pop()!.replace(/\.md$/, '') } +} + +const inboxVault = moveNoteVocabulary(settingsFor('inbox'), null, FOLDERS) +const rootVault = moveNoteVocabulary(settingsFor('root'), null, FOLDERS) + +describe('moveNoteVocabulary', () => { + it('names the notes root the way the sidebar does', () => { + expect(inboxVault.rootLabel).toBe('Inbox') + expect(rootVault.rootLabel).toBe('Vault root') + expect(moveNoteVocabulary(settingsFor('inbox'), { inbox: 'Notes' }, FOLDERS).rootLabel).toBe('Notes') + expect(inboxVault.archiveLabel).toBe('Archive') + }) + + it('knows the Archive by its directory name too', () => { + const remapped = moveNoteVocabulary(settingsFor('root', { archive: 'Shelf' }), null, FOLDERS) + expect(parseMoveNoteTarget('Shelf/Old', remapped)).toEqual({ folder: 'archive', subpath: 'Old' }) + expect(parseMoveNoteTarget('archive/Old', remapped)).toEqual({ folder: 'archive', subpath: 'Old' }) + }) +}) + +describe('parseMoveNoteTarget', () => { + it('reads notes-area paths, with empty as the root', () => { + expect(parseMoveNoteTarget('', inboxVault)).toEqual({ folder: 'inbox', subpath: '' }) + expect(parseMoveNoteTarget(' Work/Research ', inboxVault)).toEqual({ folder: 'inbox', subpath: 'Work/Research' }) + expect(parseMoveNoteTarget('Areas', rootVault)).toEqual({ folder: 'inbox', subpath: 'Areas' }) + expect(parseMoveNoteTarget('Archive', rootVault)).toEqual({ folder: 'archive', subpath: '' }) + }) + + it('still takes the older inbox/ spelling of the notes area', () => { + expect(parseMoveNoteTarget('inbox', inboxVault)).toEqual({ folder: 'inbox', subpath: '' }) + expect(parseMoveNoteTarget('inbox/Work', inboxVault)).toEqual({ folder: 'inbox', subpath: 'Work' }) + // On a root vault as well, since that is what the old prompt offered there. + expect(parseMoveNoteTarget('inbox/Areas', rootVault)).toEqual({ folder: 'inbox', subpath: 'Areas' }) + }) + + it('means a real folder named inbox on a root vault that has one', () => { + const withInboxFolder = moveNoteVocabulary(settingsFor('root'), null, [...FOLDERS, folder('inbox', 'inbox')]) + expect(parseMoveNoteTarget('inbox/Sub', withInboxFolder)).toEqual({ folder: 'inbox', subpath: 'inbox/Sub' }) + }) +}) + +describe('validateMoveNoteTarget', () => { + it('accepts the root, folders and the Archive', () => { + for (const value of ['', 'Work', 'Work/Research', 'archive', 'archive/Old', 'inbox/Work']) { + expect(validateMoveNoteTarget(value, inboxVault)).toBeNull() + expect(validateMoveNoteTarget(value, rootVault)).toBeNull() + } + }) + + it('refuses hidden names, parent references and control characters', () => { + for (const value of ['.hidden', 'Work/../Elsewhere', 'inbox/.git', 'bad\u0000name']) { + expect(validateMoveNoteTarget(value, inboxVault)).toBe( + 'Choose a folder without hidden names or parent-directory segments.' + ) + } + }) + + it('refuses the Quick Notes and the Trash, which have their own actions', () => { + expect(validateMoveNoteTarget('quick', rootVault)).toMatch(/Quick Notes and the Trash have their own actions/) + expect(validateMoveNoteTarget('trash/Old', inboxVault)).toMatch(/^Notes move within Inbox/) + expect(validateMoveNoteTarget('Trash', rootVault)).toMatch(/^Notes move within the vault root/) + // Spelled out as a subfolder of the inbox, it is a folder that happens to + // carry that name, and it stays allowed. + expect(validateMoveNoteTarget('inbox/trash', inboxVault)).toBeNull() + const remapped = moveNoteVocabulary(settingsFor('root', { quick: 'Scratch' }), null, FOLDERS) + expect(validateMoveNoteTarget('Scratch/x', remapped)).toMatch(/own actions/) + }) +}) + +describe('buildMoveNotePrompt', () => { + it('offers the notes area without a prefix, the root first, then the Archive', () => { + const prompt = buildMoveNotePrompt(note('inbox/Work/One.md'), FOLDERS, inboxVault) + expect(prompt.suggestions?.map((row) => row.value)).toEqual([ + '', + 'Areas', + 'Work', + 'Work/Research', + 'archive', + 'archive/Old' + ]) + expect(prompt.suggestions?.[0].label).toBe('Inbox') + expect(prompt.suggestions?.find((row) => row.value === 'Work/Research')?.detail).toBe('Work') + expect(prompt.suggestions?.find((row) => row.value === 'Areas')?.detail).toBe('Inbox') + expect(prompt.suggestions?.find((row) => row.value === 'archive')?.detail).toBe('Archive') + expect(prompt.allowEmptySubmit).toBe(true) + expect(prompt.description).toContain('empty = Inbox') + }) + + it('reads "Vault root" on a root vault, where the old prompt said inbox/', () => { + const prompt = buildMoveNotePrompt(note('Areas/Gym/Plan.md'), FOLDERS, rootVault) + expect(prompt.suggestions?.[0]).toEqual({ value: '', label: 'Vault root' }) + expect(prompt.suggestions?.map((row) => row.value)).not.toContainEqual(expect.stringMatching(/^inbox/)) + expect(prompt.placeholder).toBe('Vault root (type a folder to change)') + }) + + it('opens on the note\'s current folder, spelled like the suggestions', () => { + expect(buildMoveNotePrompt(note('inbox/Work/One.md'), FOLDERS, inboxVault).initialValue).toBe('Work') + expect(buildMoveNotePrompt(note('inbox/One.md'), FOLDERS, inboxVault).initialValue).toBe('') + expect(buildMoveNotePrompt(note('Areas/Gym/Plan.md'), FOLDERS, rootVault).initialValue).toBe('Areas/Gym') + expect(buildMoveNotePrompt(note('Plan.md'), FOLDERS, rootVault).initialValue).toBe('') + expect(buildMoveNotePrompt(note('archive/Old/X.md', 'archive'), FOLDERS, inboxVault).initialValue).toBe('archive/Old') + expect(buildMoveNotePrompt(note('archive/X.md', 'archive'), FOLDERS, rootVault).initialValue).toBe('archive') + }) + + it('validates through the same rules the prompt shows', () => { + const prompt = buildMoveNotePrompt(note('inbox/One.md'), FOLDERS, inboxVault) + expect(prompt.validate?.('')).toBeNull() + expect(prompt.validate?.('quick')).toMatch(/own actions/) + }) +}) + +describe('directory moves', () => { + it('offers only the notes area, root first', () => { + const prompt = buildMoveDirectoryPrompt('Work/Research', FOLDERS, rootVault) + expect(prompt.suggestions?.map((row) => row.value)).toEqual(['', 'Areas', 'Work']) + expect(prompt.suggestions?.[0].label).toBe('Vault root') + expect(prompt.allowEmptySubmit).toBe(true) + }) + + it('keeps folders out of the Archive and names the root the vault uses', () => { + expect(validateMoveDirectoryTarget('Work/Research', 'archive', FOLDERS, inboxVault)).toBe( + 'Folders and databases move within Inbox, not the Archive.' + ) + expect(validateMoveDirectoryTarget('Work/Research', 'archive/Old', FOLDERS, rootVault)).toBe( + 'Folders and databases move within the vault root, not the Archive.' + ) + expect(validateMoveDirectoryTarget('Work/Research', '', FOLDERS, rootVault)).toBeNull() + expect(validateMoveDirectoryTarget('Work/Research', 'Areas', FOLDERS, rootVault)).toBeNull() + expect(validateMoveDirectoryTarget('Work/Research', 'inbox/Areas', FOLDERS, inboxVault)).toBeNull() + expect(validateMoveDirectoryTarget('Work/Research', 'Missing', FOLDERS, rootVault)).toBe('Choose an existing folder.') + expect(validateMoveDirectoryTarget('Work', 'Work/Research', FOLDERS, rootVault)).toBe('A folder cannot move into itself.') + }) +}) diff --git a/packages/app-core/src/lib/move-note.ts b/packages/app-core/src/lib/move-note.ts index b66f72f1..e9418fdb 100644 --- a/packages/app-core/src/lib/move-note.ts +++ b/packages/app-core/src/lib/move-note.ts @@ -1,12 +1,72 @@ -import type { FolderEntry, NoteMeta } from '@shared/ipc' +import type { FolderEntry, NoteMeta, VaultSettings } from '@shared/ipc' import { formDirContaining, formTitleFromDir } from '@shared/databases' +import { resolveFolderPath } from '@shared/system-folder-paths' import type { PromptOptions, PromptSuggestion } from '../components/PromptModal' +import { resolveSystemFolderLabels, type SystemFolderLabels } from './system-folder-labels' +import { isPrimaryNotesAtRoot, noteFolderSubpath } from './vault-layout' export type MoveNoteDestination = { folder: 'inbox' | 'archive' subpath: string } +/** + * How a destination is spelled on this vault: the sidebar's language, not the + * bucket's. + * + * A destination is a path inside the notes area, the way the NOTES tree shows + * it: `Work/Research`, or nothing at all for the notes root, which is the + * Inbox on an Inbox vault and the vault itself when the primary notes live at + * the root. The Archive is reached as `archive/…`, its bucket id, whatever the + * folder is called on disk or in the sidebar. Spelling `inbox/Work` for the + * notes area is the older form the manual taught; it still works on an Inbox + * vault, and on a root vault too unless a real folder named `inbox` sits at + * the root, in which case it means that folder (#844). + */ +export interface MoveNoteVocabulary { + settings: VaultSettings | null | undefined + /** The notes area is the vault root (Settings → Vault, "Vault root"). */ + atRoot: boolean + /** What the notes root is called on the surface: "Vault root", or the inbox's label. */ + rootLabel: string + /** The same, as it reads inside a sentence. */ + rootPhrase: string + archiveLabel: string + /** The Archive's directory name, lowercased, so a typed `Shelf/Old` on a + * vault that keeps its archive in `Shelf/` still means the Archive. */ + archiveDir: string + /** Top-level names that are system folders rather than places to move a + * note: Quick Notes and the Trash, by their bucket ids and their directory + * names. Reaching them is a different action, with its own confirmation. */ + reserved: Set + /** A real folder named `inbox` at the root of a root vault, so `inbox/…` + * means that folder rather than the notes area. */ + inboxIsFolder: boolean +} + +export function moveNoteVocabulary( + settings: VaultSettings | null | undefined, + labels: SystemFolderLabels | null | undefined, + folders: readonly FolderEntry[] +): MoveNoteVocabulary { + const atRoot = isPrimaryNotesAtRoot(settings) + const resolved = resolveSystemFolderLabels(labels) + const dir = (folder: 'quick' | 'trash' | 'archive'): string => + resolveFolderPath(folder, settings?.systemFolderPaths).toLowerCase() + return { + settings, + atRoot, + rootLabel: atRoot ? 'Vault root' : resolved.inbox, + rootPhrase: atRoot ? 'the vault root' : resolved.inbox, + archiveLabel: resolved.archive, + archiveDir: dir('archive'), + reserved: new Set(['quick', 'trash', dir('quick'), dir('trash')]), + inboxIsFolder: + atRoot && + folders.some((entry) => entry.folder === 'inbox' && entry.subpath.toLowerCase() === 'inbox') + } +} + function normalizeMoveTarget(value: string): string { return value .trim() @@ -15,56 +75,107 @@ function normalizeMoveTarget(value: string): string { .replace(/^\/+|\/+$/g, '') } -function initialTargetFromPath(path: string): string { - const parts = path.split('/').filter(Boolean) - const top = parts[0] - if (top === 'inbox' || top === 'archive') { - return parts.slice(0, -1).join('/') - } - return 'inbox' +interface ReadMoveTarget extends MoveNoteDestination { + /** Written as `inbox/…`, the older spelling of the notes area. */ + viaInboxPrefix: boolean } -function buildMoveNoteSuggestions( - folders: FolderEntry[], - roots: readonly MoveNoteDestination['folder'][] = ['inbox', 'archive'] -): PromptSuggestion[] { - const byValue = new Map() - const push = (value: string, detail?: string): void => { - if (!byValue.has(value)) byValue.set(value, { value, detail }) +function readMoveTarget(value: string, vocabulary: MoveNoteVocabulary): ReadMoveTarget { + const normalized = normalizeMoveTarget(value) + if (!normalized) return { folder: 'inbox', subpath: '', viaInboxPrefix: false } + const [top, ...rest] = normalized.split('/') + const lower = top.toLowerCase() + if (lower === 'archive' || lower === vocabulary.archiveDir) { + return { folder: 'archive', subpath: rest.join('/'), viaInboxPrefix: false } } - - for (const root of roots) push(root, 'Root') - - for (const folder of folders) { - if (!roots.some((root) => root === folder.folder)) continue - const value = folder.subpath ? `${folder.folder}/${folder.subpath}` : folder.folder - push(value, folder.subpath ? folder.folder : 'Root') + if (lower === 'inbox' && !vocabulary.inboxIsFolder) { + return { folder: 'inbox', subpath: rest.join('/'), viaInboxPrefix: true } } + return { folder: 'inbox', subpath: normalized, viaInboxPrefix: false } +} - return [...byValue.values()].sort((a, b) => { - const aDepth = a.value.split('/').length - const bDepth = b.value.split('/').length - return aDepth - bDepth || a.value.localeCompare(b.value) - }) +/** Where a typed or picked destination points. Empty means the notes root. */ +export function parseMoveNoteTarget( + value: string, + vocabulary: MoveNoteVocabulary +): MoveNoteDestination { + const { folder, subpath } = readMoveTarget(value, vocabulary) + return { folder, subpath } } -export function validateMoveNoteTarget(value: string): string | null { - const normalized = normalizeMoveTarget(value) - if (!normalized) return 'Folder path required' - const [top] = normalized.split('/') - if (top !== 'inbox' && top !== 'archive') { - return 'Top-level folder must be inbox or archive' +/** + * Why `value` is not a place a note can move to, or null. Empty is the notes + * root, so it is a destination; a segment that starts with a dot is a hidden + * name or a parent reference, neither of which a note belongs in; and the + * Quick Notes and Trash are reached by their own actions, so their names are + * refused rather than quietly turned into folders of that name. + */ +export function validateMoveNoteTarget( + value: string, + vocabulary: MoveNoteVocabulary +): string | null { + if (/[\u0000-\u001f]/.test(value)) { + return 'Choose a folder without hidden names or parent-directory segments.' + } + const target = readMoveTarget(value, vocabulary) + const segments = target.subpath.split('/').filter(Boolean) + if (segments.some((part) => part.startsWith('.'))) { + return 'Choose a folder without hidden names or parent-directory segments.' + } + if ( + target.folder === 'inbox' && + !target.viaInboxPrefix && + segments.length > 0 && + vocabulary.reserved.has(segments[0].toLowerCase()) + ) { + return `Notes move within ${vocabulary.rootPhrase} or into archive/…; Quick Notes and the Trash have their own actions.` } return null } -export function parseMoveNoteTarget(value: string): MoveNoteDestination { - const normalized = normalizeMoveTarget(value) - const [folder, ...rest] = normalized.split('/') - return { - folder: folder as MoveNoteDestination['folder'], - subpath: rest.join('/') +/** `Work/Research` reads as a child of `Work`; a top-level folder, of the root. */ +function parentDetail(subpath: string, root: string): string { + const parts = subpath.split('/') + return parts.length > 1 ? parts.slice(0, -1).join('/') : root +} + +function buildMoveNoteSuggestions( + folders: readonly FolderEntry[], + vocabulary: MoveNoteVocabulary, + roots: readonly MoveNoteDestination['folder'][] = ['inbox', 'archive'] +): PromptSuggestion[] { + const notesArea = new Map() + const archive = new Map() + if (roots.includes('inbox')) notesArea.set('', { value: '', label: vocabulary.rootLabel }) + if (roots.includes('archive')) archive.set('archive', { value: 'archive', detail: vocabulary.archiveLabel }) + for (const folder of folders) { + const sub = normalizeMoveTarget(folder.subpath) + if (!sub) continue + if (folder.folder === 'inbox' && roots.includes('inbox') && !notesArea.has(sub)) { + notesArea.set(sub, { value: sub, detail: parentDetail(sub, vocabulary.rootLabel) }) + } else if (folder.folder === 'archive' && roots.includes('archive')) { + const value = `archive/${sub}` + if (!archive.has(value)) { + archive.set(value, { value, detail: parentDetail(value, vocabulary.archiveLabel) }) + } + } + } + const byDepth = (a: PromptSuggestion, b: PromptSuggestion): number => { + const depth = (value: string): number => (value === '' ? -1 : value.split('/').length) + return depth(a.value) - depth(b.value) || a.value.localeCompare(b.value) } + return [...[...notesArea.values()].sort(byDepth), ...[...archive.values()].sort(byDepth)] +} + +/** Where the note is now, spelled the way the prompt spells destinations. */ +function currentMoveTarget( + note: Pick, + vocabulary: MoveNoteVocabulary +): string { + const subpath = noteFolderSubpath(note, vocabulary.settings) + if (note.folder === 'archive') return subpath ? `archive/${subpath}` : 'archive' + if (note.folder === 'inbox') return subpath + return '' } /** @@ -148,42 +259,50 @@ export function buildTemplateDestinationPrompt( } } +/** + * Prompt for where a note should move. It opens on the note's current folder, + * spelled the way the suggestions are: a folder of the notes area with no + * prefix, empty for the notes root, `archive/…` for the Archive. + */ export function buildMoveNotePrompt( - note: Pick, - folders: FolderEntry[] + note: Pick, + folders: FolderEntry[], + vocabulary: MoveNoteVocabulary ): PromptOptions { return { title: `Move "${note.title}" to…`, - description: 'Enter a folder path, e.g. inbox/Work/Research', - initialValue: initialTargetFromPath(note.path), - placeholder: 'inbox/Work', + description: `Pick a folder, or type a path like Work/Research (empty = ${vocabulary.rootLabel}) or archive/Reference.`, + initialValue: currentMoveTarget(note, vocabulary), + placeholder: `${vocabulary.rootLabel} (type a folder to change)`, okLabel: 'Move', - suggestions: buildMoveNoteSuggestions(folders), + allowEmptySubmit: true, + suggestions: buildMoveNoteSuggestions(folders, vocabulary), autoHighlightFirst: true, - suggestionsHint: '↑↓ or ⌃J/⌃K pick a folder · Enter to move', - validate: validateMoveNoteTarget + suggestionsHint: `Empty = ${vocabulary.rootLabel} · ↑↓ or ⌃J/⌃K pick a folder · Enter to move`, + validate: (value) => validateMoveNoteTarget(value, vocabulary) } } /** * Why `value` cannot receive the folder or database at `directory`, or null. * Directories move within the notes area only, so the destination is an - * existing `inbox[/sub]` folder, written the way the move-note prompt writes - * it. `folders` is the live list: the prompt validates against what exists - * when the user submits, not when it opened. + * existing folder of it, written the way the move-note prompt writes it. + * `folders` is the live list: the prompt validates against what exists when + * the user submits, not when it opened. */ export function validateMoveDirectoryTarget( directory: string, value: string, - folders: FolderEntry[] + folders: FolderEntry[], + vocabulary: MoveNoteVocabulary ): string | null { - const normalized = normalizeMoveTarget(value) - if (!normalized) return 'Folder path required' - const [top, ...rest] = normalized.split('/') - if (top !== 'inbox') return 'Folders and databases move within inbox' - const subpath = rest.join('/') - if (/[\u0000-\u001f]/.test(value) || rest.some((part) => part.startsWith('.'))) - return 'Choose a folder without hidden names or parent-directory segments.' + const problem = validateMoveNoteTarget(value, vocabulary) + if (problem) return problem + const target = readMoveTarget(value, vocabulary) + if (target.folder !== 'inbox') { + return `Folders and databases move within ${vocabulary.rootPhrase}, not the Archive.` + } + const subpath = target.subpath if (subpath === directory || subpath.startsWith(`${directory}/`)) return 'A folder cannot move into itself.' if (formDirContaining(subpath)) return 'Databases are not move destinations.' @@ -203,7 +322,11 @@ export function validateMoveDirectoryTarget( * prefilled path would filter that list down to the folder it already is in. * The folder itself, everything under it, and databases are never offered. */ -export function buildMoveDirectoryPrompt(directory: string, folders: FolderEntry[]): PromptOptions { +export function buildMoveDirectoryPrompt( + directory: string, + folders: FolderEntry[], + vocabulary: MoveNoteVocabulary +): PromptOptions { const destinations = folders.filter( (entry) => entry.subpath !== directory && @@ -212,11 +335,12 @@ export function buildMoveDirectoryPrompt(directory: string, folders: FolderEntry ) return { title: `Move "${formTitleFromDir(directory)}" to…`, - description: 'Pick a folder, or enter a path like inbox/Work/Research', - placeholder: 'inbox/Work', + description: `Pick a folder, or type a path like Work/Research (empty = ${vocabulary.rootLabel}).`, + placeholder: `${vocabulary.rootLabel} (type a folder to change)`, okLabel: 'Move', - suggestions: buildMoveNoteSuggestions(destinations, ['inbox']), + allowEmptySubmit: true, + suggestions: buildMoveNoteSuggestions(destinations, vocabulary, ['inbox']), autoHighlightFirst: true, - suggestionsHint: '↑↓ or ⌃J/⌃K pick a folder · Enter to move' + suggestionsHint: `Empty = ${vocabulary.rootLabel} · ↑↓ or ⌃J/⌃K pick a folder · Enter to move` } } diff --git a/packages/app-core/src/lib/note-events.ts b/packages/app-core/src/lib/note-events.ts new file mode 100644 index 00000000..0c959736 --- /dev/null +++ b/packages/app-core/src/lib/note-events.ts @@ -0,0 +1,39 @@ +// The store's one-line announcements that THIS app changed a note, for the +// workflow event triggers to hear (`lib/workflow-events`). +// +// Deliberately without an import of its own: the store calls `emitNoteEvent` +// from its save path, and the workflows code must not ride onto the boot path +// on the back of that. The listening side is installed lazily when a vault +// opens, and until then an event simply has nobody to tell. +// +// Only edits made in this app pass through here, which is the whole point: +// what a workflow run writes is applied by the host and never comes back as +// an event, and a change that arrives by sync never happened in this app. + +import type { WorkflowEvent } from '@shared/workflows/types' + +type NoteEventListener = (event: WorkflowEvent, path: string) => void + +const listeners = new Set() + +/** Subscribe; the returned function unsubscribes. */ +export function onNoteEvent(listener: NoteEventListener): () => void { + listeners.add(listener) + return () => { + listeners.delete(listener) + } +} + +/** + * Tell every listener. Never throws: a listener that fails must not fail the + * save it was told about, so the failure is logged and the save goes on. + */ +export function emitNoteEvent(event: WorkflowEvent, path: string): void { + for (const listener of listeners) { + try { + listener(event, path) + } catch (err) { + console.error('note event listener failed', err) + } + } +} diff --git a/packages/app-core/src/lib/workflow-events.test.ts b/packages/app-core/src/lib/workflow-events.test.ts new file mode 100644 index 00000000..66baaa0c --- /dev/null +++ b/packages/app-core/src/lib/workflow-events.test.ts @@ -0,0 +1,231 @@ +// @vitest-environment jsdom + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { WorkflowIndexEntry } from './workflow-index' + +// A hand-rolled store: the dispatcher reads four facts from it and nothing +// else, so the suite tests the timing and the routing rather than the app. +const storeState = { + workflowsEnabled: true, + workflowEventTriggers: true, + workflowIndex: [] as WorkflowIndexEntry[], + noteDirty: {} as Record +} +vi.mock('../store', () => ({ useStore: { getState: () => storeState } })) + +// The run itself is covered in `workflow-trigger-events.test.ts`; here it is a +// spy that answers whatever a case needs. +const runWorkflowForEvent = vi.fn() +vi.mock('./workflow-trigger', () => ({ + runWorkflowForEvent: (...args: unknown[]) => runWorkflowForEvent(...args) +})) + +const { emitNoteEvent } = await import('./note-events') +const { + EVENT_SETTLE_MS, + installWorkflowEventTriggers, + resetWorkflowEventTriggers, + workflowsListeningTo +} = await import('./workflow-events') + +function entry( + id: string, + trigger: WorkflowIndexEntry['trigger'], + status: WorkflowIndexEntry['status'] = 'active' +): WorkflowIndexEntry { + return { id, name: id, description: '', status, trigger, mutates: true } +} + +const ON_SAVED = entry('file-topics', { type: 'event', event: 'note-saved' }) +const ON_CREATED = entry('stamp-new', { type: 'event', event: 'note-created' }) + +/** Let the settle timer land and the run promise chain drain. */ +async function settle(ms = EVENT_SETTLE_MS): Promise { + await vi.advanceTimersByTimeAsync(ms) +} + +beforeEach(() => { + vi.useFakeTimers() + storeState.workflowsEnabled = true + storeState.workflowEventTriggers = true + storeState.workflowIndex = [ON_SAVED] + storeState.noteDirty = {} + runWorkflowForEvent.mockReset() + runWorkflowForEvent.mockResolvedValue('ran') + Object.defineProperty(window, 'zen', { + configurable: true, + value: { applyWorkflow: vi.fn() } + }) + installWorkflowEventTriggers() +}) + +afterEach(() => { + resetWorkflowEventTriggers() + vi.useRealTimers() +}) + +describe('workflowsListeningTo', () => { + it('finds the active workflows whose trigger names the event', () => { + const index = [ + ON_SAVED, + ON_CREATED, + entry('draft-saved', { type: 'event', event: 'note-saved' }, 'draft'), + entry('by-hand', { type: 'manual' }), + entry('nightly', { type: 'schedule', cron: '0 2 * * *' }) + ] + expect(workflowsListeningTo(index, 'note-saved').map((w) => w.id)).toEqual(['file-topics']) + expect(workflowsListeningTo(index, 'note-created').map((w) => w.id)).toEqual(['stamp-new']) + expect(workflowsListeningTo(index, 'tag-added')).toEqual([]) + }) +}) + +describe('an event on a note', () => { + it('runs the listening workflow once the note has settled', async () => { + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle(EVENT_SETTLE_MS - 1) + expect(runWorkflowForEvent).not.toHaveBeenCalled() + await settle(1) + expect(runWorkflowForEvent).toHaveBeenCalledTimes(1) + expect(runWorkflowForEvent).toHaveBeenCalledWith({ + id: 'file-topics', + event: 'note-saved', + path: 'inbox/Dune.md' + }) + }) + + it('folds a burst of autosaves into one run, made after the last one', async () => { + for (let i = 0; i < 5; i += 1) { + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle(EVENT_SETTLE_MS / 2) + } + expect(runWorkflowForEvent).not.toHaveBeenCalled() + await settle(EVENT_SETTLE_MS / 2) + expect(runWorkflowForEvent).toHaveBeenCalledTimes(1) + }) + + it('keeps the notes apart: each has its own timer', async () => { + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle(EVENT_SETTLE_MS / 2) + emitNoteEvent('note-saved', 'inbox/Arrakis.md') + await settle(EVENT_SETTLE_MS / 2) + expect(runWorkflowForEvent).toHaveBeenCalledTimes(1) + expect(runWorkflowForEvent.mock.calls[0][0]).toMatchObject({ path: 'inbox/Dune.md' }) + await settle(EVENT_SETTLE_MS / 2) + expect(runWorkflowForEvent).toHaveBeenCalledTimes(2) + expect(runWorkflowForEvent.mock.calls[1][0]).toMatchObject({ path: 'inbox/Arrakis.md' }) + }) + + it('fires the events a note collected together, in a fixed order', async () => { + storeState.workflowIndex = [ON_SAVED, ON_CREATED] + // Saved first, then created: a note written straight after it was made. + emitNoteEvent('note-saved', 'inbox/New.md') + emitNoteEvent('note-created', 'inbox/New.md') + await settle() + expect(runWorkflowForEvent.mock.calls.map((call) => call[0].event)).toEqual([ + 'note-created', + 'note-saved' + ]) + }) + + it('runs every listener of an event, in index order', async () => { + storeState.workflowIndex = [ + ON_SAVED, + entry('second', { type: 'event', event: 'note-saved' }) + ] + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent.mock.calls.map((call) => call[0].id)).toEqual([ + 'file-topics', + 'second' + ]) + }) + + it('does nothing when no active workflow listens for it', async () => { + storeState.workflowIndex = [ON_CREATED, entry('draft', { type: 'event', event: 'note-saved' }, 'draft')] + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent).not.toHaveBeenCalled() + }) + + it('stays quiet while the kill switch or the feature is off', async () => { + storeState.workflowEventTriggers = false + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + storeState.workflowEventTriggers = true + storeState.workflowsEnabled = false + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent).not.toHaveBeenCalled() + }) + + it('honours a kill switch flipped while a note was settling', async () => { + emitNoteEvent('note-saved', 'inbox/Dune.md') + storeState.workflowEventTriggers = false + await settle() + expect(runWorkflowForEvent).not.toHaveBeenCalled() + }) + + it('stays quiet on a host that cannot apply a run', async () => { + Object.defineProperty(window, 'zen', { configurable: true, value: {} }) + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent).not.toHaveBeenCalled() + }) + + it('waits for the next save of a note that is being typed in again', async () => { + emitNoteEvent('note-saved', 'inbox/Dune.md') + storeState.noteDirty = { 'inbox/Dune.md': true } + await settle() + expect(runWorkflowForEvent).not.toHaveBeenCalled() + // The next save lands with a clean note, and that one runs. + storeState.noteDirty = {} + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent).toHaveBeenCalledTimes(1) + }) + + it('tries again after a palette run that was at its confirmation', async () => { + runWorkflowForEvent.mockResolvedValueOnce('busy') + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent).toHaveBeenCalledTimes(1) + await settle() + expect(runWorkflowForEvent).toHaveBeenCalledTimes(2) + expect(runWorkflowForEvent.mock.calls[1][0]).toMatchObject({ event: 'note-saved' }) + }) + + it('carries only the events still to fire across a retry', async () => { + storeState.workflowIndex = [ON_CREATED, ON_SAVED] + runWorkflowForEvent.mockResolvedValueOnce('ran').mockResolvedValueOnce('busy') + emitNoteEvent('note-created', 'inbox/New.md') + emitNoteEvent('note-saved', 'inbox/New.md') + await settle() + expect(runWorkflowForEvent.mock.calls.map((call) => call[0].event)).toEqual([ + 'note-created', + 'note-saved' + ]) + await settle() + // The created run is not repeated; the saved one is. + expect(runWorkflowForEvent.mock.calls.map((call) => call[0].event)).toEqual([ + 'note-created', + 'note-saved', + 'note-saved' + ]) + }) + + it('stops the chain when a run found the note being typed in again', async () => { + storeState.workflowIndex = [ON_SAVED, entry('second', { type: 'event', event: 'note-saved' })] + runWorkflowForEvent.mockResolvedValueOnce('unsaved') + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent).toHaveBeenCalledTimes(1) + }) + + it('listens once however many times a vault is opened', async () => { + installWorkflowEventTriggers() + installWorkflowEventTriggers() + emitNoteEvent('note-saved', 'inbox/Dune.md') + await settle() + expect(runWorkflowForEvent).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/app-core/src/lib/workflow-events.ts b/packages/app-core/src/lib/workflow-events.ts new file mode 100644 index 00000000..b3032560 --- /dev/null +++ b/packages/app-core/src/lib/workflow-events.ts @@ -0,0 +1,114 @@ +// Event triggers: the store says "this app just saved / created / moved / +// tagged a note", and an active workflow whose `trigger:` names that event +// runs over that one note a moment later. +// +// Only edits made in this app fire. A change that arrives by sync, or one that +// another device made, does not: two synced desktops both firing on each +// other's writes would run every non-idempotent step once per device and +// bounce an `append` between them forever. Firing where the human edited means +// each edit fires exactly once, on exactly one machine, and the single-executor +// model in `docs/ideas/workflows.md` can arrive later without changing what a +// workflow file means. What a run writes never fires either: the host applies +// it, and nothing about it passes through the store's save path. +// +// A note settles before it fires (`EVENT_SETTLE_MS` after its last event), so +// a burst of autosaves while someone types is one run, made once the typing +// paused, and a note still being typed in when the timer lands waits for its +// next save instead. The events one note collected fire together, in a fixed +// order, and every run goes through the same one-at-a-time funnel as a +// palette run (`lib/workflow-trigger`). + +import type { WorkflowEvent } from '@shared/workflows/types' +import { useStore } from '../store' +import { onNoteEvent } from './note-events' +import type { WorkflowIndexEntry } from './workflow-index' + +/** How long a note has to be quiet before its events fire. */ +export const EVENT_SETTLE_MS = 1500 + +/** The order the events one note collected fire in: what exists, where it is, + * what it says, what it is tagged. */ +const EVENT_ORDER: readonly WorkflowEvent[] = ['note-created', 'note-moved', 'note-saved', 'tag-added'] + +interface Pending { + events: Set + timer: ReturnType +} + +const pending = new Map() +let detach: (() => void) | null = null + +/** The active workflows whose trigger names `event`, in index order. */ +export function workflowsListeningTo( + index: readonly WorkflowIndexEntry[], + event: WorkflowEvent +): WorkflowIndexEntry[] { + return index.filter( + (entry) => + entry.status === 'active' && entry.trigger.type === 'event' && entry.trigger.event === event + ) +} + +/** Start listening. Idempotent: the store calls this on every vault open. */ +export function installWorkflowEventTriggers(): void { + if (detach) return + detach = onNoteEvent(noteChanged) +} + +/** Forget every pending firing and stop listening. For tests. */ +export function resetWorkflowEventTriggers(): void { + for (const entry of pending.values()) clearTimeout(entry.timer) + pending.clear() + detach?.() + detach = null +} + +/** + * A note changed in this app. Cheap by design, because it runs on every save: + * the index says whether anyone is listening at all, and only then does the + * note get a timer. + */ +export function noteChanged(event: WorkflowEvent, path: string): void { + const state = useStore.getState() + if (!state.workflowsEnabled || !state.workflowEventTriggers) return + if (typeof window.zen?.applyWorkflow !== 'function') return + if (workflowsListeningTo(state.workflowIndex, event).length === 0) return + arm(path, [event]) +} + +function arm(path: string, events: Iterable): void { + const existing = pending.get(path) + if (existing) clearTimeout(existing.timer) + const merged = new Set(existing?.events ?? []) + for (const event of events) merged.add(event) + pending.set(path, { + events: merged, + timer: setTimeout(() => void fire(path), EVENT_SETTLE_MS) + }) +} + +async function fire(path: string): Promise { + const entry = pending.get(path) + if (!entry) return + pending.delete(path) + const state = useStore.getState() + if (!state.workflowsEnabled || !state.workflowEventTriggers) return + // Being typed in again. Its next save arms it again, with the text that save + // lands, which is the text a run should see. + if (state.noteDirty[path]) return + const { runWorkflowForEvent } = await import('./workflow-trigger') + const order = EVENT_ORDER.filter((event) => entry.events.has(event)) + for (const [index, event] of order.entries()) { + for (const workflow of workflowsListeningTo(useStore.getState().workflowIndex, event)) { + const outcome = await runWorkflowForEvent({ id: workflow.id, event, path }) + // A palette run is at its confirmation: come back once it is done, with + // this event and the ones after it still to fire. + if (outcome === 'busy') { + arm(path, order.slice(index)) + return + } + // Typed in again while a run was planning: the next save re-arms it. + if (outcome === 'unsaved') return + } + } +} diff --git a/packages/app-core/src/lib/workflow-index.test.ts b/packages/app-core/src/lib/workflow-index.test.ts index 910137b0..dbe24f52 100644 --- a/packages/app-core/src/lib/workflow-index.test.ts +++ b/packages/app-core/src/lib/workflow-index.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { buildWorkflowIndex } from './workflow-index' describe('buildWorkflowIndex', () => { - it('summarizes name, status, and whether any step writes', () => { + it('summarizes name, status, trigger, and whether any step writes', () => { const files = [ { id: 'reading-log', @@ -13,6 +13,11 @@ describe('buildWorkflowIndex', () => { id: 'peek', sourcePath: '.zennotes/workflows/peek.md', raw: '---\nstatus: draft\n---\n\nnotes = all\n' + }, + { + id: 'file-topics', + sourcePath: '.zennotes/workflows/file-topics.md', + raw: '---\nname: File topics\ntrigger: on note-saved where folder = inbox\n---\n\nall | move Topics\n' } ] expect(buildWorkflowIndex(files)).toEqual([ @@ -21,11 +26,29 @@ describe('buildWorkflowIndex', () => { name: 'Reading log', description: 'Sync it', status: 'active', + trigger: { type: 'manual' }, mutates: true }, // The name falls back to the id and a missing status reads as the // parser's default, so the index answers for every file the view lists. - { id: 'peek', name: 'peek', description: '', status: 'draft', mutates: false } + { + id: 'peek', + name: 'peek', + description: '', + status: 'draft', + trigger: { type: 'manual' }, + mutates: false + }, + // The trigger rides along as parsed, so the event triggers can find + // their listeners without reading a file on every save. + { + id: 'file-topics', + name: 'File topics', + description: '', + status: 'active', + trigger: { type: 'event', event: 'note-saved', where: 'folder = inbox' }, + mutates: true + } ]) }) @@ -39,4 +62,15 @@ describe('buildWorkflowIndex', () => { ] expect(buildWorkflowIndex(files)[0]).toMatchObject({ id: 'wonky', name: 'Wonky' }) }) + + it('degrades a trigger the engine does not know to manual, as the parser does', () => { + const files = [ + { + id: 'future', + sourcePath: '.zennotes/workflows/future.md', + raw: '---\ntrigger: on note-starred\n---\n\nnotes = all\n' + } + ] + expect(buildWorkflowIndex(files)[0].trigger).toEqual({ type: 'manual' }) + }) }) diff --git a/packages/app-core/src/lib/workflow-index.ts b/packages/app-core/src/lib/workflow-index.ts index d28cfd12..db57be97 100644 --- a/packages/app-core/src/lib/workflow-index.ts +++ b/packages/app-core/src/lib/workflow-index.ts @@ -1,12 +1,13 @@ // The vault's workflows, reduced to what surfaces OUTSIDE the workflows view // need to know: the command palette lists them, the trigger flow checks they -// may act. Everything here derives from the same files the view reads, through -// the same parser, so the palette can never disagree with the editor about -// what exists or what may run. +// may act, and the event triggers ask which of them listen for an event. +// Everything here derives from the same files the view reads, through the +// same parser, so the palette can never disagree with the editor about what +// exists or what may run. import type { WorkflowFile } from '@bridge-contract/workflows' import { parseWorkflow } from '@shared/workflows/parse' -import type { WorkflowStatus } from '@shared/workflows/types' +import type { WorkflowStatus, WorkflowTrigger } from '@shared/workflows/types' import { stepIsMutating } from '@shared/workflows/nodes' export interface WorkflowIndexEntry { @@ -15,6 +16,9 @@ export interface WorkflowIndexEntry { name: string description: string status: WorkflowStatus + /** As parsed: an unknown trigger has already degraded to manual here, so an + * event trigger in the index is one the engine knows how to fire. */ + trigger: WorkflowTrigger /** True when any step writes, i.e. running it will ask for confirmation. */ mutates: boolean } @@ -34,6 +38,7 @@ export function buildWorkflowIndex(files: readonly WorkflowFile[]): WorkflowInde name: workflow.name, description: workflow.description, status: workflow.status, + trigger: workflow.trigger, mutates: workflow.statements.some((statement) => statement.steps.some((step) => stepIsMutating(step.kind)) ) diff --git a/packages/app-core/src/lib/workflow-run.test.ts b/packages/app-core/src/lib/workflow-run.test.ts index 1ce639ea..826f315e 100644 --- a/packages/app-core/src/lib/workflow-run.test.ts +++ b/packages/app-core/src/lib/workflow-run.test.ts @@ -29,7 +29,8 @@ import { undoneHeadline, unknownTemplateDiagnostics, unsavedCollisionDescription, - unsavedCollisions + unsavedCollisions, + promisedMoves } from './workflow-run' const OPS: WorkflowOp[] = [ @@ -510,3 +511,38 @@ describe('interruptedRunToOffer', () => { ) }) }) + +describe('promisedMoves', () => { + it('names where a move, a rename, an archive and a trash put a note', () => { + expect( + promisedMoves([ + { kind: 'move', path: 'inbox/Dune.md', to: 'Topics' }, + { kind: 'rename', path: 'inbox/Old.md', to: 'New name' }, + { kind: 'archive', path: 'inbox/Done.md' }, + { kind: 'trash', path: 'inbox/Gone.md' }, + { kind: 'add-tag', path: 'inbox/Stay.md', tag: 'x' } + ]) + ).toEqual([ + { from: 'inbox/Dune.md', to: 'Topics/Dune.md' }, + { from: 'inbox/Old.md', to: 'inbox/New name.md' }, + { from: 'inbox/Done.md', to: 'archive/Done.md' }, + { from: 'inbox/Gone.md', to: 'trash/Gone.md' } + ]) + }) + + it('folds a note moved twice to its last stop, and respects remapped system folders', () => { + expect( + promisedMoves( + [ + { kind: 'move', path: 'inbox/Dune.md', to: 'Topics' }, + { kind: 'rename', path: 'Topics/Dune.md', to: 'Arrakis' }, + { kind: 'archive', path: 'inbox/Done.md' } + ], + { archive: 'Shelf' } + ) + ).toEqual([ + { from: 'inbox/Dune.md', to: 'Topics/Arrakis.md' }, + { from: 'inbox/Done.md', to: 'Shelf/Done.md' } + ]) + }) +}) diff --git a/packages/app-core/src/lib/workflow-run.ts b/packages/app-core/src/lib/workflow-run.ts index a23b270d..bcde94d1 100644 --- a/packages/app-core/src/lib/workflow-run.ts +++ b/packages/app-core/src/lib/workflow-run.ts @@ -10,6 +10,8 @@ // after their vault changes, so each one is worth testing without mounting // React. Nothing in this file writes, and nothing in it touches the bridge. +import { folderTarget, moveTarget, renameTarget } from '@shared/workflows/paths' +import type { SystemFolderDirs } from '@shared/workflows/paths' import type { NoteTemplate } from '@bridge-contract/templates' import type { WorkflowRunReceipt, @@ -566,3 +568,41 @@ export function interruptedRunHeadline(run: WorkflowRunSummary): string { 'notes' )} on disk still carry it.` } + +/* -------------------------------------------------------------------------- */ +/* Where a run promises to put the notes it moves */ +/* -------------------------------------------------------------------------- */ + +/** + * Each note a run moves, with the path the plan promises it ends up at, the + * chain of a note moved twice folded to its last stop. Read by the store so + * an open editor follows its note (`followWorkflowMoves`). The applier can + * still land a note at a suffixed name when the promised one is taken, which + * is why the store checks the file before it trusts a promise. + */ +export function promisedMoves( + ops: readonly WorkflowOp[], + systemFolderDirs?: SystemFolderDirs +): { from: string; to: string }[] { + const finalOf = new Map() + for (const op of ops) { + let to: string + switch (op.kind) { + case 'move': + to = moveTarget(op.path, op.to) + break + case 'rename': + to = renameTarget(op.path, op.to) + break + case 'archive': + case 'trash': + to = folderTarget(op.kind, op.path, systemFolderDirs) + break + default: + continue + } + const origin = [...finalOf.entries()].find(([, current]) => current === op.path)?.[0] + finalOf.set(origin ?? op.path, to) + } + return [...finalOf.entries()].map(([from, to]) => ({ from, to })) +} diff --git a/packages/app-core/src/lib/workflow-trigger-events.test.ts b/packages/app-core/src/lib/workflow-trigger-events.test.ts new file mode 100644 index 00000000..befde7c6 --- /dev/null +++ b/packages/app-core/src/lib/workflow-trigger-events.test.ts @@ -0,0 +1,386 @@ +// @vitest-environment jsdom + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { NoteMeta } from '@shared/ipc' +import type { WorkflowRunRecord } from '../store' + +// The same hand-rolled store `workflow-trigger.test.ts` uses, plus the kill +// switch an event run reads. +const storeState = { + workflowsEnabled: true, + workflowEventTriggers: true, + notes: [] as NoteMeta[], + noteDirty: {} as Record, + selectedPath: null as string | null, + customTemplates: [], + vaultSettings: { systemFolderPaths: {} }, + workflowRunRecord: null as WorkflowRunRecord | null, + setWorkflowRunRecord: ( + next: + | WorkflowRunRecord + | null + | ((prev: WorkflowRunRecord | null) => WorkflowRunRecord | null) + ) => { + storeState.workflowRunRecord = + typeof next === 'function' ? next(storeState.workflowRunRecord) : next + }, + persistNote: vi.fn().mockResolvedValue(undefined), + refreshNotes: vi.fn().mockResolvedValue(undefined), + // The editor-follows-its-note half lives in the real store; here it is a + // no-op that records what the run promised to move. + followWorkflowMoves: vi.fn((moves: readonly { from: string; to: string }[]) => { + storeState.promised = [...moves] + return async () => {} + }), + promised: [] as { from: string; to: string }[] +} +vi.mock('../store', () => ({ useStore: { getState: () => storeState } })) + +// A confirmation that can be left hanging, to hold a palette run at its dialog. +let confirmImpl: () => Promise = async () => true +vi.mock('./confirm-requests', () => ({ confirmApp: () => confirmImpl() })) + +const { runWorkflowForEvent, runWorkflowById, forgetTriggerNotices } = await import( + './workflow-trigger' +) +const { useToastStore } = await import('./toast') + +const FILE_TOPICS = `--- +name: File topics +status: active +trigger: on note-saved +--- +all | contains type: topic | move Topics +` + +const STAMP_ALL = `--- +name: Stamp all +status: active +trigger: on note-saved +--- +all | add-tag #seen +` + +function file(id: string, raw: string) { + return { id, sourcePath: `.zennotes/workflows/${id}.md`, raw } +} + +function note(path: string, tags: string[] = []): NoteMeta { + return { + path, + title: path.split('/').pop()?.replace(/\.md$/i, '') ?? path, + folder: 'inbox', + siblingOrder: 0, + createdAt: 1, + updatedAt: 2, + size: 0, + tags, + wikilinks: [], + assetEmbeds: [], + hasAttachments: false, + excerpt: '' + } +} + +const BODIES: Record = { + 'inbox/Dune.md': '# Dune\n\ntype: topic\n', + 'inbox/Arrakis.md': '# Arrakis\n\nplain prose\n', + 'Areas/Plan.md': '# Plan\n\ntype: topic\n' +} + +let zen: Record> + +function installZen( + files: ReturnType[], + overrides: Record = {} +): void { + zen = { + listWorkflows: vi.fn().mockResolvedValue(files), + applyWorkflow: vi.fn().mockImplementation(async ({ ops }: { ops: { path?: string }[] }) => ({ + runId: 'run-1', + workflowId: 'file-topics', + startedAt: 0, + applied: ops.length, + paths: [...new Set(ops.flatMap((op) => (op.path ? [op.path] : [])))], + irreversible: 0 + })), + undoWorkflowRun: vi.fn().mockResolvedValue({ runId: 'run-1', restored: 1 }), + readNote: vi.fn().mockImplementation(async (path: string) => ({ body: BODIES[path] ?? '' })), + ...overrides + } + Object.defineProperty(window, 'zen', { configurable: true, value: zen }) +} + +const toasts = () => useToastStore.getState().toasts + +beforeEach(() => { + storeState.workflowsEnabled = true + storeState.workflowEventTriggers = true + storeState.notes = [note('inbox/Dune.md'), note('inbox/Arrakis.md'), note('Areas/Plan.md')] + storeState.noteDirty = {} + storeState.workflowRunRecord = null + confirmImpl = async () => true + useToastStore.setState({ toasts: [] }) + forgetTriggerNotices() + installZen([file('file-topics', FILE_TOPICS)]) +}) + +describe('runWorkflowForEvent', () => { + it('runs the pipeline over the note that fired and leaves a receipt with Undo', async () => { + const outcome = await runWorkflowForEvent({ + id: 'file-topics', + event: 'note-saved', + path: 'inbox/Dune.md' + }) + expect(outcome).toBe('ran') + expect(zen.applyWorkflow).toHaveBeenCalledWith({ + workflowId: 'file-topics', + ops: [{ kind: 'move', path: 'inbox/Dune.md', to: 'Topics' }] + }) + const receipt = toasts().at(-1) + expect(receipt?.message).toBe('File topics: Applied 1 change across 1 note.') + expect(receipt?.action?.label).toBe('Undo 1 note') + expect(storeState.workflowRunRecord?.receipt.runId).toBe('run-1') + expect(storeState.refreshNotes).toHaveBeenCalled() + // The store was told where the note is going before the run, so an editor + // open on it can follow. + expect(storeState.promised).toEqual([{ from: 'inbox/Dune.md', to: 'Topics/Dune.md' }]) + // And the record remembers them, so the Undo on the toast can carry the + // editor back the same way. + expect(storeState.workflowRunRecord?.moves).toEqual([{ from: 'inbox/Dune.md', to: 'Topics/Dune.md' }]) + }) + + it('undoing from the toast carries the editor back with the note', async () => { + await runWorkflowForEvent({ id: 'file-topics', event: 'note-saved', path: 'inbox/Dune.md' }) + storeState.followWorkflowMoves.mockClear() + // The toast's action fires and forgets, as a click handler does. + toasts().at(-1)?.action?.onClick() + await vi.waitFor(() => + expect(storeState.workflowRunRecord?.undone).toEqual({ runId: 'run-1', restored: 1 }) + ) + expect(zen.undoWorkflowRun).toHaveBeenCalledWith('run-1') + expect(storeState.followWorkflowMoves).toHaveBeenCalledWith( + [{ from: 'Topics/Dune.md', to: 'inbox/Dune.md' }], + { reverting: true } + ) + }) + + it('finds nothing to do when the note does not match, and says nothing', async () => { + const outcome = await runWorkflowForEvent({ + id: 'file-topics', + event: 'note-saved', + path: 'inbox/Arrakis.md' + }) + expect(outcome).toBe('nothing') + expect(zen.applyWorkflow).not.toHaveBeenCalled() + expect(toasts()).toEqual([]) + }) + + it('scopes `all` to the note that fired, never the rest of the vault', async () => { + installZen([file('stamp-all', STAMP_ALL)]) + await runWorkflowForEvent({ id: 'stamp-all', event: 'note-saved', path: 'inbox/Arrakis.md' }) + expect(zen.applyWorkflow).toHaveBeenCalledWith({ + workflowId: 'stamp-all', + ops: [{ kind: 'add-tag', path: 'inbox/Arrakis.md', tag: 'seen' }] + }) + }) + + it('says nothing about a run that wrote no file', async () => { + installZen([file('stamp-all', STAMP_ALL)], { + applyWorkflow: vi.fn().mockResolvedValue({ + runId: 'run-2', + workflowId: 'stamp-all', + startedAt: 0, + applied: 1, + paths: [], + irreversible: 0 + }) + }) + const outcome = await runWorkflowForEvent({ + id: 'stamp-all', + event: 'note-saved', + path: 'inbox/Dune.md' + }) + expect(outcome).toBe('ran') + expect(toasts()).toEqual([]) + }) + + it('reports a run the host rolled back, naming the workflow', async () => { + installZen([file('file-topics', FILE_TOPICS)], { + applyWorkflow: vi.fn().mockResolvedValue({ + runId: 'run-3', + workflowId: 'file-topics', + startedAt: 0, + applied: 0, + paths: [], + irreversible: 0, + rolledBack: { reason: 'Cannot move inbox/Dune.md: the note is missing' } + }) + }) + const outcome = await runWorkflowForEvent({ + id: 'file-topics', + event: 'note-saved', + path: 'inbox/Dune.md' + }) + expect(outcome).toBe('skipped') + expect(toasts().at(-1)).toMatchObject({ + type: 'error', + message: '"File topics": Cannot move inbox/Dune.md: the note is missing' + }) + }) + + describe('the trigger condition', () => { + const GATED = `--- +name: Inbox topics +status: active +trigger: on note-saved where folder = inbox +--- +all | contains type: topic | move Topics +` + + it('lets the run through when the note matches', async () => { + installZen([file('inbox-topics', GATED)]) + const outcome = await runWorkflowForEvent({ + id: 'inbox-topics', + event: 'note-saved', + path: 'inbox/Dune.md' + }) + expect(outcome).toBe('ran') + expect(zen.applyWorkflow).toHaveBeenCalledTimes(1) + }) + + it('holds the run when the note does not match', async () => { + installZen([file('inbox-topics', GATED)]) + const outcome = await runWorkflowForEvent({ + id: 'inbox-topics', + event: 'note-saved', + path: 'Areas/Plan.md' + }) + expect(outcome).toBe('nothing') + expect(zen.applyWorkflow).not.toHaveBeenCalled() + }) + + it('reads frontmatter from the body, like a where step does', async () => { + installZen([ + file( + 'rated', + '---\nname: Rated\ntrigger: on note-saved where rating >= 4\n---\nall | add-tag #good\n' + ) + ]) + BODIES['inbox/Rated.md'] = '---\nrating: 5\n---\n# Rated\n' + storeState.notes = [...storeState.notes, note('inbox/Rated.md')] + const outcome = await runWorkflowForEvent({ + id: 'rated', + event: 'note-saved', + path: 'inbox/Rated.md' + }) + expect(outcome).toBe('ran') + delete BODIES['inbox/Rated.md'] + }) + + it('refuses a condition the engine cannot read, once', async () => { + installZen([ + file('broken', '---\nname: Broken\ntrigger: on note-saved where rating\n---\nall | add-tag #x\n') + ]) + const first = await runWorkflowForEvent({ id: 'broken', event: 'note-saved', path: 'inbox/Dune.md' }) + const second = await runWorkflowForEvent({ id: 'broken', event: 'note-saved', path: 'inbox/Dune.md' }) + expect([first, second]).toEqual(['skipped', 'skipped']) + expect(zen.applyWorkflow).not.toHaveBeenCalled() + expect(toasts()).toHaveLength(1) + expect(toasts()[0]).toMatchObject({ type: 'error' }) + expect(toasts()[0].message).toContain('"Broken" has a trigger condition the engine cannot read') + }) + }) + + describe('unsaved edits', () => { + it('waits for the next save when the note that fired is dirty again', async () => { + storeState.noteDirty = { 'inbox/Dune.md': true } + const outcome = await runWorkflowForEvent({ + id: 'file-topics', + event: 'note-saved', + path: 'inbox/Dune.md' + }) + expect(outcome).toBe('unsaved') + expect(zen.applyWorkflow).not.toHaveBeenCalled() + expect(toasts()).toEqual([]) + }) + + it('leaves another dirty note alone, says so, and runs the rest', async () => { + installZen([ + file( + 'log', + '---\nname: Log\ntrigger: on note-saved\n---\nhit = all | contains type: topic\nhit | add-tag #topic\nhit | render list | write "inbox/Log.md"\n' + ) + ]) + storeState.notes = [...storeState.notes, note('inbox/Log.md')] + storeState.noteDirty = { 'inbox/Log.md': true } + const outcome = await runWorkflowForEvent({ id: 'log', event: 'note-saved', path: 'inbox/Dune.md' }) + expect(outcome).toBe('ran') + const ops = zen.applyWorkflow.mock.calls[0][0].ops as { kind: string; path?: string }[] + expect(ops.map((op) => op.kind)).toEqual(['add-tag']) + expect(toasts().map((t) => t.message)).toEqual([ + '"Log" left inbox/Log.md alone: unsaved edits there.', + 'Log: Applied 1 change across 1 note.' + ]) + }) + }) + + describe('what does not run', () => { + it('a workflow whose file no longer names this event', async () => { + const outcome = await runWorkflowForEvent({ + id: 'file-topics', + event: 'note-created', + path: 'inbox/Dune.md' + }) + expect(outcome).toBe('skipped') + expect(zen.applyWorkflow).not.toHaveBeenCalled() + }) + + it('a draft', async () => { + installZen([file('file-topics', FILE_TOPICS.replace('status: active', 'status: draft'))]) + expect( + await runWorkflowForEvent({ id: 'file-topics', event: 'note-saved', path: 'inbox/Dune.md' }) + ).toBe('skipped') + expect(zen.applyWorkflow).not.toHaveBeenCalled() + }) + + it('anything while the kill switch is off', async () => { + storeState.workflowEventTriggers = false + expect( + await runWorkflowForEvent({ id: 'file-topics', event: 'note-saved', path: 'inbox/Dune.md' }) + ).toBe('skipped') + expect(zen.listWorkflows).not.toHaveBeenCalled() + }) + + it('a note the store no longer lists', async () => { + expect( + await runWorkflowForEvent({ id: 'file-topics', event: 'note-saved', path: 'inbox/Gone.md' }) + ).toBe('skipped') + expect(zen.applyWorkflow).not.toHaveBeenCalled() + }) + + it('on a host with no run support', async () => { + installZen([file('file-topics', FILE_TOPICS)], { applyWorkflow: undefined }) + expect( + await runWorkflowForEvent({ id: 'file-topics', event: 'note-saved', path: 'inbox/Dune.md' }) + ).toBe('skipped') + }) + }) + + it('answers busy while a palette run is at its confirmation', async () => { + installZen([file('file-topics', FILE_TOPICS.replace('trigger: on note-saved', 'trigger: manual'))]) + let release: (ok: boolean) => void = () => {} + confirmImpl = () => new Promise((resolve) => (release = resolve)) + const manual = runWorkflowById('file-topics') + await vi.waitFor(() => expect(zen.readNote).toHaveBeenCalled()) + const outcome = await runWorkflowForEvent({ + id: 'file-topics', + event: 'note-saved', + path: 'inbox/Dune.md' + }) + expect(outcome).toBe('busy') + release(false) + await manual + expect(zen.applyWorkflow).not.toHaveBeenCalled() + }) +}) diff --git a/packages/app-core/src/lib/workflow-trigger.test.ts b/packages/app-core/src/lib/workflow-trigger.test.ts index 270bef96..2171143e 100644 --- a/packages/app-core/src/lib/workflow-trigger.test.ts +++ b/packages/app-core/src/lib/workflow-trigger.test.ts @@ -25,7 +25,14 @@ const storeState = { typeof next === 'function' ? next(storeState.workflowRunRecord) : next }, persistNote: vi.fn().mockResolvedValue(undefined), - refreshNotes: vi.fn().mockResolvedValue(undefined) + refreshNotes: vi.fn().mockResolvedValue(undefined), + // The editor-follows-its-note half lives in the real store; here it is a + // no-op that records what the run promised to move. + followWorkflowMoves: vi.fn((moves: readonly { from: string; to: string }[]) => { + storeState.promised = [...moves] + return async () => {} + }), + promised: [] as { from: string; to: string }[] } vi.mock('../store', () => ({ useStore: { getState: () => storeState } })) @@ -116,7 +123,9 @@ describe('running a workflow from the palette', () => { workflowId: 'star-books', receipt: receipt('run-1'), undone: null, - undoError: null + undoError: null, + // Nothing moved, so there is nothing for an undo to carry an editor back along. + moves: [] }) }) diff --git a/packages/app-core/src/lib/workflow-trigger.ts b/packages/app-core/src/lib/workflow-trigger.ts index 1ab90629..990bb49e 100644 --- a/packages/app-core/src/lib/workflow-trigger.ts +++ b/packages/app-core/src/lib/workflow-trigger.ts @@ -1,5 +1,6 @@ -// Run a workflow from anywhere that is not the workflows view: today the -// command palette, later the event and schedule triggers' manual cousins. +// Run a workflow from anywhere that is not the workflows view: the command +// palette, and the event triggers (`lib/workflow-events`), which are the +// background cousins of a palette run. // // The flow is the SAME trust ladder the view walks, built from the same pure // pieces (`workflow-run`, `workflow-op-summary`): plan fresh, name the notes @@ -9,6 +10,12 @@ // lives: the view has a card, this flow has a toast carrying the Undo action, // because the user who ran from the palette never left whatever they were // doing and a toast is the only surface that follows them there. +// +// An event run walks the same ladder with the questions taken out, because +// nobody is there to answer them: it sees only the note that fired, skips +// rather than asks when a note has unsaved edits, applies without a +// confirmation, and says nothing at all when it changed nothing. What it does +// write it reports the same way, on a toast with the Undo. import { BUILTIN_TEMPLATES } from '@shared/builtin-templates' import { mergeTemplates } from '@shared/template-files' @@ -16,7 +23,14 @@ import { parseWorkflow } from '@shared/workflows/parse' import { planWorkflow } from '@shared/workflows/engine' import { validateWorkflow } from '@shared/workflows/validate' import { isRunnable } from '@shared/workflows/types' -import type { Diagnostic, Workflow, WorkflowOp } from '@shared/workflows/types' +import type { + Diagnostic, + PlanContext, + Workflow, + WorkflowEvent, + WorkflowOp +} from '@shared/workflows/types' +import type { NoteMeta } from '@shared/ipc' import type { WorkflowRunReceipt, WorkflowRunSummary } from '@bridge-contract/workflows' import { useStore } from '../store' import { useToastStore } from './toast' @@ -26,10 +40,12 @@ import { IRREVERSIBLE_KINDS, summarizeOps } from './workflow-op-summary' import { collectRunSideEffects, driftedPathsNote, + formatPathList, interruptedRunHeadline, interruptedRunToOffer, opsExcludingPaths, planWritePaths, + promisedMoves, receiptHeadline, resolveTemplateOps, runConfirmDescription, @@ -56,10 +72,19 @@ const RECEIPT_TOAST_MS = 12_000 /** * One run at a time, across every surface that calls this. A palette entry * picked twice while the first confirm is still open must not stack a second - * confirm behind it. + * confirm behind it, and an event that lands during that confirm waits its + * turn rather than writing underneath a dialog that is still describing the + * vault as it was. */ let inFlight = false +/** + * Things said once per session rather than once per save: an event fires on + * every save, and a workflow whose trigger condition cannot be read would + * otherwise say so every time someone pauses typing. + */ +const noticed = new Set() + function toast( message: string, type: 'success' | 'error' | 'info' = 'info', @@ -69,6 +94,17 @@ function toast( useToastStore.getState().addToast(message, type, action, durationMs) } +function toastOnce(key: string, message: string, type: 'error' | 'info'): void { + if (noticed.has(key)) return + noticed.add(key) + toast(message, type) +} + +/** Forget what has already been said. For tests. */ +export function forgetTriggerNotices(): void { + noticed.clear() +} + function errorText(err: unknown): string { return err instanceof Error ? err.message : String(err) } @@ -117,8 +153,21 @@ async function undoFromToast(receipt: WorkflowRunReceipt): Promise { }) if (!ok) return } + // The notes the run moved go back where they were, and an editor open on + // one goes back with it, the way it followed the run forward. + const moves = state.workflowRunRecord?.receipt.runId === receipt.runId ? (state.workflowRunRecord.moves ?? []) : [] + const settle = state.followWorkflowMoves( + moves.map(({ from, to }) => ({ from: to, to: from })), + { reverting: true } + ) try { - const result = await window.zen.undoWorkflowRun(receipt.runId) + const result = await (async () => { + try { + return await window.zen.undoWorkflowRun(receipt.runId) + } finally { + await settle() + } + })() // The record follows the undo wherever it was pressed, so the view's card // stops offering an Undo this toast already spent. useStore @@ -213,6 +262,126 @@ export async function runWorkflowById(id: string): Promise { } } +/* -------------------------------------------------------------------------- */ +/* Shared rungs */ +/* -------------------------------------------------------------------------- */ + +interface LoadedWorkflows { + byId: Map + /** Parse diagnostics per workflow id. */ + diagnostics: Map +} + +/** + * Read the vault's workflows fresh from disk rather than trusting the index a + * palette row or a trigger timer was built from: the file may have changed + * since, and what runs must be what is on disk. + */ +async function loadWorkflows(): Promise { + const files = await window.zen.listWorkflows() + const byId = new Map() + const diagnostics = new Map() + for (const file of files) { + const parsed = parseWorkflow(file.raw, file.id) + byId.set(parsed.workflow.id, parsed.workflow) + diagnostics.set(parsed.workflow.id, parsed.diagnostics) + } + return { byId, diagnostics } +} + +function titleForPathIn(notes: readonly NoteMeta[]): (path: string) => string { + const titleByPath = new Map(notes.map((note) => [note.path, note.title])) + return (path: string): string => { + const known = titleByPath.get(path) + if (known !== undefined) return known + const file = path.split('/').pop() ?? path + return file.replace(/\.md$/i, '') + } +} + +function missingTemplatesMessage(workflowName: string, missing: readonly string[]): string { + const names = missing.map((name) => `"${name}"`).join(', ') + return missing.length === 1 + ? `${names} is not a template in this vault, so "${workflowName}" cannot run.` + : `${names} are not templates in this vault, so "${workflowName}" cannot run.` +} + +/** + * Apply an op list and leave the receipt: the last rung, shared by both entry + * points from the moment there is nothing left to ask. Throws only when the + * host refused the run outright; a run that failed partway comes back as a + * rolled-back receipt and is reported here. + * + * A `background` run says nothing about a run that wrote no file. An event + * fires on every save, and a tag the note already carries or a move into the + * folder it already sits in is a run that changed nothing, which is not news. + * A manual run gets the receipt either way: someone pressed something and is + * waiting to hear. + */ +async function applyAndReport( + workflow: Workflow, + ops: WorkflowOp[], + background: boolean +): Promise { + const state = useStore.getState() + // An editor open on a note the run moves follows it, the way it follows a + // rename the app makes: shielded from the unlink echo during the run and + // carried to the new path after it, if the note landed where promised. + const moves = promisedMoves(ops, state.vaultSettings.systemFolderPaths) + const settle = state.followWorkflowMoves(moves) + let receipt: WorkflowRunReceipt + try { + receipt = await window.zen.applyWorkflow({ workflowId: workflow.id, ops }) + } catch (err) { + await settle() + throw err + } + await settle() + // The same store record the view's Undo card reads, written from here too. + // A palette run and a canvas run are one event with two doorways, and two + // records for one workflow is exactly how an older Undo ends up reverting a + // newer run: the newest write wins, on every surface at once. + useStore.getState().setWorkflowRunRecord({ + workflowId: workflow.id, + receipt, + undone: null, + undoError: null, + moves + }) + if (receipt.rolledBack !== undefined) { + toast( + background ? `"${workflow.name}": ${receipt.rolledBack.reason}` : receipt.rolledBack.reason, + 'error' + ) + await state.refreshNotes() + return false + } + const effects = collectRunSideEffects(ops) + for (const message of effects.notifications) toast(message) + if (effects.clipboard !== null) { + try { + await navigator.clipboard.writeText(effects.clipboard) + } catch { + toast('The run finished, but the clipboard write failed.', 'error') + } + } + const undoable = receipt.paths.length > 0 && typeof window.zen.undoWorkflowRun === 'function' + if (!background || receipt.paths.length > 0) { + toast( + background ? `${workflow.name}: ${receiptHeadline(receipt)}` : receiptHeadline(receipt), + 'success', + undoable ? { label: undoLabel(receipt), onClick: () => void undoFromToast(receipt) } : undefined, + undoable ? RECEIPT_TOAST_MS : undefined + ) + } + await state.refreshNotes() + return true +} + +/* -------------------------------------------------------------------------- */ +/* A palette run */ +/* -------------------------------------------------------------------------- */ + async function runWorkflow(id: string): Promise { const state = useStore.getState() if (typeof window.zen.listWorkflows !== 'function' || typeof window.zen.applyWorkflow !== 'function') { @@ -220,22 +389,15 @@ async function runWorkflow(id: string): Promise { return } - // Read fresh from disk rather than trusting the index the palette showed: - // the file may have changed since, and what runs must be what is on disk. - let byId: Map - let parseDiagnostics: Diagnostic[] = [] + let loaded: LoadedWorkflows try { - const files = await window.zen.listWorkflows() - byId = new Map() - for (const file of files) { - const parsed = parseWorkflow(file.raw, file.id) - byId.set(parsed.workflow.id, parsed.workflow) - if (file.id === id) parseDiagnostics = parsed.diagnostics - } + loaded = await loadWorkflows() } catch (err) { toast(`Could not read the vault's workflows: ${errorText(err)}`, 'error') return } + const { byId } = loaded + const parseDiagnostics = loaded.diagnostics.get(id) ?? [] const workflow = byId.get(id) if (!workflow) { @@ -303,22 +465,9 @@ async function runWorkflow(id: string): Promise { return } - const titleByPath = new Map(notes.map((note) => [note.path, note.title])) - const titleForPath = (path: string): string => { - const known = titleByPath.get(path) - if (known !== undefined) return known - const file = path.split('/').pop() ?? path - return file.replace(/\.md$/i, '') - } - const withTemplates = resolveTemplateOps(ops, templates, titleForPath, new Date()) + const withTemplates = resolveTemplateOps(ops, templates, titleForPathIn(notes), new Date()) if (withTemplates.missing.length > 0) { - const names = withTemplates.missing.map((name) => `"${name}"`).join(', ') - toast( - withTemplates.missing.length === 1 - ? `${names} is not a template in this vault, so "${workflow.name}" cannot run.` - : `${names} are not templates in this vault, so "${workflow.name}" cannot run.`, - 'error' - ) + toast(missingTemplatesMessage(workflow.name, withTemplates.missing), 'error') return } @@ -348,45 +497,153 @@ async function runWorkflow(id: string): Promise { if (unsaved?.resolution === 'save') { await Promise.all(unsaved.paths.map((path) => state.persistNote(path))) } - const receipt = await window.zen.applyWorkflow({ - workflowId: workflow.id, - ops: withTemplates.ops - }) - // The same store record the view's Undo card reads, written from here too. - // A palette run and a canvas run are one event with two doorways, and two - // records for one workflow is exactly how an older Undo ends up reverting a - // newer run: the newest write wins, on every surface at once. - useStore.getState().setWorkflowRunRecord({ - workflowId: workflow.id, - receipt, - undone: null, - undoError: null - }) - if (receipt.rolledBack === undefined) { - const effects = collectRunSideEffects(withTemplates.ops) - for (const message of effects.notifications) toast(message) - if (effects.clipboard !== null) { - try { - await navigator.clipboard.writeText(effects.clipboard) - } catch { - toast('The run finished, but the clipboard write failed.', 'error') - } - } - const undoable = - receipt.paths.length > 0 && typeof window.zen.undoWorkflowRun === 'function' - toast( - receiptHeadline(receipt), - 'success', - undoable - ? { label: undoLabel(receipt), onClick: () => void undoFromToast(receipt) } - : undefined, - undoable ? RECEIPT_TOAST_MS : undefined + await applyAndReport(workflow, withTemplates.ops, false) + } catch (err) { + toast(errorText(err), 'error') + } +} + +/* -------------------------------------------------------------------------- */ +/* An event run */ +/* -------------------------------------------------------------------------- */ + +/** + * What became of an event: `ran` wrote and left a receipt, `nothing` found + * nothing to do (which is silent), `unsaved` found the note being typed in + * again (its next save fires again), `busy` met a palette run at its + * confirmation (the caller tries again), and `skipped` covers everything + * that was said out loud or needed no saying. + */ +export type EventRunOutcome = 'ran' | 'nothing' | 'unsaved' | 'busy' | 'skipped' + +export interface WorkflowEventRun { + id: string + event: WorkflowEvent + /** The note the event is about, as the store knows it now. */ + path: string +} + +/** + * Run a workflow because one of its events fired for one note. + * + * The plan sees ONLY that note: every source, `all` and `folder` included, + * resolves over it, and `current` is it. That is what keeps an event run cheap + * (no whole-vault body reads on every autosave), what makes `all | contains + * type: topic | move Topics` mean "file this note when it says so", and what + * bounds what a run can touch to the note that changed plus whatever its + * sinks write. + */ +export async function runWorkflowForEvent(run: WorkflowEventRun): Promise { + const state = useStore.getState() + if (!state.workflowsEnabled || !state.workflowEventTriggers) return 'skipped' + if (typeof window.zen.listWorkflows !== 'function' || typeof window.zen.applyWorkflow !== 'function') { + return 'skipped' + } + if (inFlight) return 'busy' + inFlight = true + try { + return await runForEvent(run) + } finally { + inFlight = false + } +} + +async function runForEvent({ id, event, path }: WorkflowEventRun): Promise { + const state = useStore.getState() + const meta = state.notes.find((note) => note.path === path) + // Gone (trashed, moved on, or a list that has not caught up with it yet). + // Its next event arms the timer again, so skipping loses nothing. + if (!meta) return 'skipped' + + let loaded: LoadedWorkflows + try { + loaded = await loadWorkflows() + } catch (err) { + toastOnce('load', `Could not read the vault's workflows: ${errorText(err)}`, 'error') + return 'skipped' + } + const workflow = loaded.byId.get(id) + // The index that armed the timer can be older than the file: a workflow made + // manual, moved to another event, or set back to draft since is left alone. + if (!workflow || !isRunnable(workflow)) return 'skipped' + if (workflow.trigger.type !== 'event' || workflow.trigger.event !== event) return 'skipped' + + const reader = createVaultReader({ + notes: [meta], + readBody: async (target) => (await window.zen.readNote(target)).body, + current: () => meta + }) + const ctx: PlanContext = { + reader, + now: Date.now(), + resolve: (other) => loaded.byId.get(other) ?? null, + systemFolderDirs: state.vaultSettings.systemFolderPaths + } + + if (workflow.trigger.where !== undefined) { + const gate = await triggerConditionHolds(workflow.trigger.where, ctx) + if (gate === 'invalid') { + toastOnce( + `where:${id}:${workflow.trigger.where}`, + `"${workflow.name}" has a trigger condition the engine cannot read (where ${workflow.trigger.where}), so it did not run.`, + 'error' ) - } else { - toast(receipt.rolledBack.reason, 'error') + return 'skipped' } - await state.refreshNotes() + if (!gate) return 'nothing' + } + + const plan = await planWorkflow(workflow, ctx) + if (plan.ops.length === 0) return 'nothing' + + const dirty = unsavedCollisions(planWritePaths(plan.ops), state.noteDirty) + // The note that fired is being typed in again. Its next save fires again, + // with the text that save lands; anything planned now would be stale. + if (dirty.includes(path)) return 'unsaved' + const ops = dirty.length > 0 ? opsExcludingPaths(plan.ops, dirty) : plan.ops + // Another note the run would write is open with unsaved edits: the manual + // ladder asks, a background run cannot, so it leaves that note alone and + // says so, since a silent skip is a change the author cannot account for. + if (dirty.length > 0) { + toast(`"${workflow.name}" left ${formatPathList(dirty)} alone: unsaved edits there.`) + } + if (ops.length === 0) return 'nothing' + + const templates = mergeTemplates(BUILTIN_TEMPLATES, state.customTemplates) + const withTemplates = resolveTemplateOps(ops, templates, titleForPathIn(state.notes), new Date()) + if (withTemplates.missing.length > 0) { + toastOnce( + `template:${id}:${withTemplates.missing.join(',')}`, + missingTemplatesMessage(workflow.name, withTemplates.missing), + 'error' + ) + return 'skipped' + } + + try { + return (await applyAndReport(workflow, withTemplates.ops, true)) ? 'ran' : 'skipped' } catch (err) { - toast(errorText(err), 'error') + toast(`"${workflow.name}": ${errorText(err)}`, 'error') + return 'skipped' } } + +/** + * Does the trigger's `where` hold for the note that fired? + * + * Evaluated by the engine itself, as the one-step pipeline `current | where + * ` over the same scoped reader the run will use, so the condition + * means exactly what the same words mean on a `where` step, frontmatter read + * from the body included. `invalid` is a condition the parser or the planner + * refused, which is reported once and never silently treated as true. + */ +async function triggerConditionHolds( + condition: string, + ctx: PlanContext +): Promise { + const parsed = parseWorkflow(`---\nname: gate\n---\ngate = current | where ${condition}\n`, 'gate') + if (parsed.diagnostics.some((diagnostic) => diagnostic.severity === 'error')) return 'invalid' + const plan = await planWorkflow(parsed.workflow, ctx) + if (plan.diagnostics.some((diagnostic) => diagnostic.severity === 'error')) return 'invalid' + return (plan.wires.gate?.length ?? 0) > 0 +} diff --git a/packages/app-core/src/note-actions.test.ts b/packages/app-core/src/note-actions.test.ts index bd3ca3ed..0f85f29d 100644 --- a/packages/app-core/src/note-actions.test.ts +++ b/packages/app-core/src/note-actions.test.ts @@ -190,7 +190,9 @@ describe("public note move", () => { .getState() .updateNoteBody("inbox/One.md", "Saved via public action.\n"); const moving = s.requestMoveNote(s.host, "inbox/One.md"); - expect(s.getPromptRequest()?.options.initialValue).toBe("inbox"); + // The notes root is the empty path now; `inbox/Work` is the older spelling + // of the same folder and still lands in it. + expect(s.getPromptRequest()?.options.initialValue).toBe(""); s.answer("inbox/Work"); expect(await moving).toBe("completed"); expect(s.files.get("inbox/Work/One.md")).toBe("Saved via public action.\n"); @@ -417,7 +419,7 @@ it.each([false, true])( }, }); const moving = s.requestMoveNote(s.host, path); - expect(s.getPromptRequest()?.options.initialValue).toBe("inbox/Work"); + expect(s.getPromptRequest()?.options.initialValue).toBe("Work"); s.answer("inbox/Work"); expect(await moving).toBe("cancelled"); }, diff --git a/packages/app-core/src/notes.ts b/packages/app-core/src/notes.ts index cd79b0ea..4b6a044d 100644 --- a/packages/app-core/src/notes.ts +++ b/packages/app-core/src/notes.ts @@ -6,8 +6,10 @@ import { confirmApp, getConfirmRequest } from "./lib/confirm-requests"; import { getPromptRequest, promptApp } from "./lib/prompt-requests"; import { buildMoveNotePrompt, + moveNoteVocabulary, parseMoveNoteTarget, validateMoveNoteTarget, + type MoveNoteVocabulary, } from "./lib/move-note"; import { noteFolderSubpath } from "./lib/vault-layout"; import { @@ -29,20 +31,29 @@ export type NoteActionResult = let pending = false; -function validateDestination(value: string): string | null { - const error = validateMoveNoteTarget(value); +function validateDestination( + value: string, + vocabulary: MoveNoteVocabulary, +): string | null { + const error = validateMoveNoteTarget(value, vocabulary); if (error) return error; - const { subpath } = parseMoveNoteTarget(value); - if ( - /[\u0000-\u001f]/.test(value) || - subpath.split("/").some((part) => part.startsWith(".")) - ) - return "Choose a folder without hidden names or parent-directory segments."; + const { subpath } = parseMoveNoteTarget(value, vocabulary); if (formDirContaining(subpath)) return "Database record folders are not move destinations."; return null; } +/** The prompt speaks the sidebar's language for this vault (see move-note). */ +function moveVocabulary( + state: ReturnType, +): MoveNoteVocabulary { + return moveNoteVocabulary( + state.vaultSettings, + state.systemFolderLabels, + state.folders, + ); +} + function captureNoteActionContext(host: NoteActionHost): () => boolean { const state = useStore.getState(); const vault = state.vault; @@ -104,25 +115,25 @@ export async function requestMoveNote( ): Promise { return requestNoteAction(host, path, async (state, note, isCurrent) => { const subpath = noteFolderSubpath(note, state.vaultSettings); - const initialValue = - note.folder === "archive" || note.folder === "inbox" - ? [note.folder, subpath].filter(Boolean).join("/") - : "inbox"; + const vocabulary = moveVocabulary(state); + const validate = (value: string): string | null => + validateDestination(value, vocabulary); const target = await promptApp({ ...buildMoveNotePrompt( note, state.folders.filter((folder) => !formDirContaining(folder.subpath)), + vocabulary, ), - initialValue, - validate: validateDestination, + validate, }); - if (!target || validateDestination(target)) return "cancelled"; + // Empty is an answer here (the notes root); only null is the Cancel. + if (target === null || validate(target)) return "cancelled"; if ( !isCurrent() || !useStore.getState().notes.some((note) => note.path === path) ) return "stale"; - const destination = parseMoveNoteTarget(target); + const destination = parseMoveNoteTarget(target, vocabulary); if (destination.folder === note.folder && destination.subpath === subpath) return "cancelled"; await useStore @@ -298,13 +309,14 @@ export async function requestNoteBatch( if (!valid()) return 'unavailable' let destination: ReturnType | null = null if (action === 'move') { + const vocabulary = moveVocabulary(state) + const validate = (value: string): string | null => validateDestination(value, vocabulary) const target = await promptApp({ - ...buildMoveNotePrompt({ ...first, title: `${paths.length} notes` }, state.folders.filter(folder => !formDirContaining(folder.subpath))), - initialValue: [first.folder === 'archive' ? 'archive' : 'inbox', noteFolderSubpath(first, state.vaultSettings)].filter(Boolean).join('/'), - validate: validateDestination + ...buildMoveNotePrompt({ ...first, title: `${paths.length} notes` }, state.folders.filter(folder => !formDirContaining(folder.subpath)), vocabulary), + validate }) - if (!target || validateDestination(target)) return 'cancelled' - destination = parseMoveNoteTarget(target) + if (target === null || validate(target)) return 'cancelled' + destination = parseMoveNoteTarget(target, vocabulary) } else if (action === 'archive') { if (!(await state.confirmArchiveNotes(paths))) return 'cancelled' } else if (action !== 'restore') { diff --git a/packages/app-core/src/store.test.ts b/packages/app-core/src/store.test.ts index 23fd7555..803d61b5 100644 --- a/packages/app-core/src/store.test.ts +++ b/packages/app-core/src/store.test.ts @@ -2011,6 +2011,41 @@ describe('renameNote heading sync (#455)', () => { expect(order.slice(0, 2)).toEqual(['save', 'rename']) }) + // Every rename in the UI comes through here without a host, so a refusal + // has to say why instead of leaving the old name in place silently (#839). + it('tells the user why a rename was refused', async () => { + installRename({ + renameNote: vi + .fn() + .mockRejectedValue( + new Error( + "Error invoking remote method 'vault:rename-note': Error: A note named “Groceries” already exists in this folder" + ) + ) + }) + const { useStore } = await loadStore() + const { useToastStore } = await import('./lib/toast') + useStore.setState({ notes: [metaOf('inbox/Untitled.md', 'Untitled')] }) + vi.spyOn(console, 'error').mockImplementation(() => {}) + + await useStore.getState().renameNote('inbox/Untitled.md', 'Groceries') + + expect(useToastStore.getState().toasts.map((toast) => [toast.type, toast.message])).toEqual([ + ['error', 'Could not rename “Untitled”: A note named “Groceries” already exists in this folder'] + ]) + }) + + it('leaves a refused rename to a host that asked to handle it', async () => { + installRename({ renameNote: vi.fn().mockRejectedValue(new Error('refused')) }) + const { useStore } = await loadStore() + const { useToastStore } = await import('./lib/toast') + + await expect( + useStore.getState().renameNote('inbox/Untitled.md', 'Groceries', () => true) + ).rejects.toThrow('refused') + expect(useToastStore.getState().toasts).toEqual([]) + }) + it('does not rename when a dirty linked note could not be saved', async () => { const dirtyNote = makeNote('See [[Untitled]]\n', 'inbox/Daily.md') const renameNote = vi.fn().mockResolvedValue(renamedMeta) diff --git a/packages/app-core/src/store.ts b/packages/app-core/src/store.ts index 9b70b5bb..87c739d3 100644 --- a/packages/app-core/src/store.ts +++ b/packages/app-core/src/store.ts @@ -194,6 +194,7 @@ import { } from '@shared/template-files' import { buildWorkflowIndex } from './lib/workflow-index' import type { WorkflowIndexEntry } from './lib/workflow-index' +import { emitNoteEvent } from './lib/note-events' import { INITIAL_VISIBLE_NOTE_PREFETCH_BATCH_SIZE, selectInitialVisibleNotePrefetchPaths @@ -457,6 +458,13 @@ async function refreshVaultIndexes(): Promise { .catch(() => { /* a message that cannot be raised is not a vault that failed to open */ }) + // The event triggers listen from here on: installed once (the module keeps + // its own listener across vaults), lazily for the same reason as above. + void import('./lib/workflow-events') + .then((mod) => mod.installWorkflowEventTriggers()) + .catch(() => { + /* nothing about opening a vault waits on the triggers either */ + }) } /** Find a template (built-in or custom) by id, or undefined if it's gone. */ @@ -709,6 +717,10 @@ interface Prefs { * closes any tab already showing it. OFF by default, deliberately: it can * rewrite notes in bulk, so it is a one-time opt-in under Settings. */ workflowsEnabled: boolean + /** Whether active workflows whose `trigger:` names an event run on their + * own for the edits made in this app. Kept apart from the master switch so + * the canvas and manual runs can stay while nothing fires by itself. */ + workflowEventTriggers: boolean atlasEnabled: boolean /** Built-in workflow recipes hidden from the gallery, by preset id. Unknown * ids are kept rather than pruned, so hiding a preset survives the preset @@ -914,6 +926,10 @@ export interface WorkflowRunRecord { undone: WorkflowUndoResult | null /** An undo that failed must not read as one that worked. */ undoError: string | null + /** Where the run promised to move each note it moved, so an undo can carry + * an open editor back the way the run carried it forward. Absent on a + * record written before this existed and on an interrupted run. */ + moves?: readonly { from: string; to: string }[] } /** Hidden gallery preset ids: strings, trimmed, deduped, order kept. Unknown @@ -1139,6 +1155,7 @@ export const DEFAULT_PREFS: Prefs = { // graph editor asks more of a new user than any other view. The feature is // opted into once in Settings -> Workflows, not stumbled into. workflowsEnabled: false, + workflowEventTriggers: true, atlasEnabled: true, hiddenWorkflowPresets: [], collapsedTagNodes: [], @@ -1466,6 +1483,10 @@ function normalizePrefs(p: Partial): Prefs { typeof p.workflowsEnabled === 'boolean' ? p.workflowsEnabled : DEFAULT_PREFS.workflowsEnabled, + workflowEventTriggers: + typeof p.workflowEventTriggers === 'boolean' + ? p.workflowEventTriggers + : DEFAULT_PREFS.workflowEventTriggers, atlasEnabled: typeof p.atlasEnabled === 'boolean' ? p.atlasEnabled : DEFAULT_PREFS.atlasEnabled, hiddenWorkflowPresets: normalizeHiddenWorkflowPresets(p.hiddenWorkflowPresets), @@ -2374,6 +2395,10 @@ function collectPrefs(s: { tagsCollapsed: boolean nestedTags: boolean workflowsEnabled: boolean + /** Whether active workflows whose `trigger:` names an event run on their + * own for the edits made in this app. Kept apart from the master switch so + * the canvas and manual runs can stay while nothing fires by itself. */ + workflowEventTriggers: boolean atlasEnabled: boolean hiddenWorkflowPresets: string[] collapsedTagNodes: string[] @@ -2480,6 +2505,7 @@ function collectPrefs(s: { tagsCollapsed: s.tagsCollapsed, nestedTags: s.nestedTags, workflowsEnabled: s.workflowsEnabled, + workflowEventTriggers: s.workflowEventTriggers, atlasEnabled: s.atlasEnabled, hiddenWorkflowPresets: s.hiddenWorkflowPresets, collapsedTagNodes: s.collapsedTagNodes, @@ -3077,6 +3103,10 @@ interface Store { * row, the `view.workflows` command, and the leader binding, so the canvas * has no way in at all. */ workflowsEnabled: boolean + /** Whether active workflows whose `trigger:` names an event run on their + * own for the edits made in this app. Kept apart from the master switch so + * the canvas and manual runs can stay while nothing fires by itself. */ + workflowEventTriggers: boolean atlasEnabled: boolean /** Built-in recipes hidden from the New-workflow gallery, by preset id. * Persisted (portable). Hiding is per taste, not per vault. */ @@ -3444,6 +3474,22 @@ interface Store { deleteNotePermanently: (path: string) => Promise emptyTrash: (hostIsCurrent?: () => boolean) => Promise changeNoteLifecycle: (path: string, action: 'archive' | 'trash' | 'restore' | 'delete', hostIsCurrent?: () => boolean) => Promise + /** + * Keep the open editors on notes a workflow run is about to move. + * + * The host moves the file and the watcher reports an unlink of the old path, + * which closes its tab (`applyChange`); a move the app makes itself shields + * the path in `renamesInFlight` and carries the tab, the buffer and the undo + * history to the new path when the host answers (`mutateNoteImpl`). A run is + * applied by the host in one transaction, so the shield goes up for every + * promised move before the run and the carry happens after it, for the notes + * that landed where the plan promised, byte for byte. Returns the function + * that ends it, to call once the run is over, landed or not. + */ + followWorkflowMoves: ( + moves: readonly { from: string; to: string }[], + options?: { reverting?: boolean } + ) => () => Promise restoreActive: () => Promise archiveActive: () => Promise unarchiveActive: () => Promise @@ -3514,6 +3560,7 @@ interface Store { /** Turn the whole Workflows feature on or off. Switching it off also closes * any pane still showing the canvas. */ setWorkflowsEnabled: (on: boolean) => void + setWorkflowEventTriggers: (on: boolean) => void setAtlasEnabled: (on: boolean) => void hideWorkflowPreset: (id: string) => void restoreWorkflowPreset: (id: string) => void @@ -5710,6 +5757,7 @@ export const useStore = create((set, get) => { tagsCollapsed: loadPrefs().tagsCollapsed, nestedTags: loadPrefs().nestedTags, workflowsEnabled: loadPrefs().workflowsEnabled, + workflowEventTriggers: loadPrefs().workflowEventTriggers, atlasEnabled: loadPrefs().atlasEnabled, hiddenWorkflowPresets: loadPrefs().hiddenWorkflowPresets, collapsedTagNodes: loadPrefs().collapsedTagNodes, @@ -6140,6 +6188,7 @@ export const useStore = create((set, get) => { try { const meta = await window.zen.createNote(folder, title, subpath) rememberEditModeForCreatedNote(meta.path) + emitNoteEvent('note-created', meta.path) // Overwrite the default `# title` body with the TaskNotes-style frontmatter // so the note is recognized as a task and shows up in the Tasks view. await window.zen.writeNote( @@ -7754,6 +7803,7 @@ export const useStore = create((set, get) => { // Snapshot only after earlier writes finish. A second caller sees the // newest buffer here, then becomes the last writer by construction. const writtenBody = content.body + const tagsBefore = s.notes.find((note) => note.path === path)?.tags noteContentVersions.set(path, (noteContentVersions.get(path) ?? 0) + 1) const meta = await window.zen.writeNote(path, writtenBody) if (!isCurrent()) return @@ -7782,6 +7832,12 @@ export const useStore = create((set, get) => { ...activeFieldsFrom(cur.paneLayout, cur.activePaneId, cur.noteContents, dirty) } }) + // This app saved the note, which is what the workflow event triggers + // listen for. A tag the note did not carry before is its own event. + emitNoteEvent('note-saved', path) + if (tagsBefore !== undefined && meta.tags.some((tag) => !tagsBefore.includes(tag))) { + emitNoteEvent('tag-added', path) + } } catch (err) { console.error('writeNote failed', err) } @@ -7926,10 +7982,21 @@ export const useStore = create((set, get) => { renameNote: async (oldPath, nextTitle, hostIsCurrent) => { if (!oldPath) return try { - await mutateNoteImpl(oldPath, () => window.zen.renameNote(oldPath, nextTitle), hostIsCurrent, true) + const moved = await mutateNoteImpl(oldPath, () => window.zen.renameNote(oldPath, nextTitle), hostIsCurrent, true) + if (moved && moved.path !== oldPath) emitNoteEvent('note-moved', moved.path) } catch (err) { if (hostIsCurrent) throw err console.error('renameNote failed', err) + // Every rename in the UI (title field, sidebar, note list, :rename) + // lands here, and a refusal used to leave the old name in place with + // nothing said about why (#839). + const title = + get().notes.find((note) => note.path === oldPath)?.title ?? + oldPath.split('/').pop()?.replace(/\.(md|excalidraw)$/i, '') ?? + oldPath + useToastStore + .getState() + .addToast(`Could not rename “${title}”: ${humanIpcError(err, 'the rename failed.')}`, 'error') } }, @@ -7943,6 +8010,7 @@ export const useStore = create((set, get) => { try { const meta = await window.zen.createNote(folder, options?.title, subpath) rememberEditModeForCreatedNote(meta.path) + emitNoteEvent('note-created', meta.path) // The heading uses the title the vault settled on, which may carry a // " 2" suffix the requested one did not. if (options?.tags && options.tags.length > 0) { @@ -8056,6 +8124,7 @@ export const useStore = create((set, get) => { const title = file.name.replace(/\.(md|markdown)$/i, '').trim() const meta = await window.zen.createNote('inbox', title || undefined) if (content) await window.zen.writeNote(meta.path, content) + emitNoteEvent('note-created', meta.path) createdPaths.push(meta.path) } catch (err) { console.error('importDroppedMarkdownFiles failed', file.name, err) @@ -8192,6 +8261,7 @@ export const useStore = create((set, get) => { if (action === 'trash') return bridge.moveToTrash(path) return source.folder === 'archive' ? bridge.unarchiveNote(path) : bridge.restoreFromTrash(path) }, isCurrent) + if (meta && meta.path !== path) emitNoteEvent('note-moved', meta.path) if (meta && canReconcile() && (action === 'archive' || action === 'trash')) { if (get().noteDirty[meta.path]) throw new Error('The moved note still has unsaved changes.') set(s => withoutNoteInWorkspace(s, meta.path)) @@ -8200,6 +8270,52 @@ export const useStore = create((set, get) => { return meta }, + followWorkflowMoves: (moves, options) => { + const openTabs = new Set(allLeaves(get().paneLayout).flatMap((leaf) => leaf.tabs)) + const open = moves.filter(({ from, to }) => from !== to && (openTabs.has(from) || from in get().noteContents)) + for (const { from } of open) renamesInFlight.add(from) + return async () => { + try { + if (open.length === 0) return + await get().refreshNotes() + const notes = get().notes + for (const { from, to } of open) { + // Asked of the disk, not of the list: a refresh that was already in + // flight when the run landed answers with the vault as it was. + const landed = await window.zen.readNote(to).then((content) => content.body, () => null) + if (landed === null) continue + if (await window.zen.readNote(from).then(() => true, () => false)) continue + // The promised path can be taken by another note, in which case the + // applier suffixed ours and this file is someone else's: only a file + // that reads exactly as the buffer did is the note that moved. An undo + // carries the note back to a path the ledger restored for it, so there + // the bytes may differ (the run edited the note after moving it) and + // a clean buffer takes the restored text instead. + const buffer = get().noteContents[from] + let restored: string | null = null + if (buffer && landed !== buffer.body) { + if (!options?.reverting || get().noteDirty[from]) continue + restored = landed + } + const meta = notes.find((note) => note.path === to) + set((s) => { + const rewritten = rewriteFolderWorkspace(s, from, to) + const contents = rewritten.noteContents! + if (meta && contents[to]) contents[to] = { ...contents[to], ...meta } + if (restored !== null && contents[to]) contents[to] = { ...contents[to], body: restored } + return { + ...rewritten, + ...activeFieldsFrom(rewritten.paneLayout!, rewritten.activePaneId!, contents, rewritten.noteDirty!) + } + }) + } + savePrefs(collectPrefs(get())) + } finally { + for (const { from } of open) renamesInFlight.delete(from) + } + } + }, + restoreActive: async () => { const path = get().selectedPath if (!path) return @@ -8661,6 +8777,10 @@ export const useStore = create((set, get) => { savePrefs(collectPrefs(get())) if (!on) closeWorkflowsTabsEverywhere() }, + setWorkflowEventTriggers: (on) => { + set({ workflowEventTriggers: on }) + savePrefs(collectPrefs(get())) + }, setAtlasEnabled: (on) => { set({ atlasEnabled: on }) savePrefs(collectPrefs(get())) @@ -9116,6 +9236,7 @@ export const useStore = create((set, get) => { const meta = await window.zen.createNote('inbox', title, subpath) rememberEditModeForCreatedNote(meta.path) if (body) await window.zen.writeNote(meta.path, body) + emitNoteEvent('note-created', meta.path) await get().refreshNotes() return get().notes.find((n) => n.path === meta.path) ?? meta } catch (err) { @@ -9447,6 +9568,7 @@ export const useStore = create((set, get) => { const { body, cursorOffset } = renderTemplate(template.body, { title, now: opts?.date }) const meta = await window.zen.createNote(folder, title, subpath) rememberEditModeForCreatedNote(meta.path) + emitNoteEvent('note-created', meta.path) // Write the rendered body before opening so the editor never flashes the // default `# Title` scaffold (mirrors importDroppedMarkdownFiles). await window.zen.writeNote(meta.path, body) @@ -10261,7 +10383,8 @@ export const useStore = create((set, get) => { moveNote: async (relPath, targetFolder, targetSubpath, hostIsCurrent) => { try { - await mutateNoteImpl(relPath, () => window.zen.moveNote(relPath, targetFolder, targetSubpath), hostIsCurrent) + const moved = await mutateNoteImpl(relPath, () => window.zen.moveNote(relPath, targetFolder, targetSubpath), hostIsCurrent) + if (moved && moved.path !== relPath) emitNoteEvent('note-moved', moved.path) } catch (err) { if (hostIsCurrent) throw err console.error('moveNote failed', err) diff --git a/packages/bridge-contract/package.json b/packages/bridge-contract/package.json index dfe4ca24..65590682 100644 --- a/packages/bridge-contract/package.json +++ b/packages/bridge-contract/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/bridge-contract", "private": true, - "version": "2.54.1", + "version": "2.55.0", "type": "module", "exports": { "./bridge": "./src/bridge.ts", diff --git a/packages/bridge-contract/src/app-config.ts b/packages/bridge-contract/src/app-config.ts index 2d495da3..689e515c 100644 --- a/packages/bridge-contract/src/app-config.ts +++ b/packages/bridge-contract/src/app-config.ts @@ -78,6 +78,7 @@ export const PORTABLE_PREF_KEYS = [ 'monoFont', // features 'workflowsEnabled', + 'workflowEventTriggers', 'hiddenWorkflowPresets', 'atlasEnabled', // view diff --git a/packages/shared-domain/package.json b/packages/shared-domain/package.json index 7eef0049..317803ab 100644 --- a/packages/shared-domain/package.json +++ b/packages/shared-domain/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/shared-domain", "private": true, - "version": "2.54.1", + "version": "2.55.0", "type": "module", "exports": { "./*": "./src/*.ts" diff --git a/packages/shared-domain/src/app-config.ts b/packages/shared-domain/src/app-config.ts index 9b06ca1c..198c6ff1 100644 --- a/packages/shared-domain/src/app-config.ts +++ b/packages/shared-domain/src/app-config.ts @@ -139,6 +139,7 @@ export const PORTABLE_DEFAULTS: Record = { textFont: null, monoFont: null, workflowsEnabled: false, + workflowEventTriggers: true, hiddenWorkflowPresets: [], atlasEnabled: true, systemFolderLabels: {}, diff --git a/packages/shared-domain/src/workflows/engine.test.ts b/packages/shared-domain/src/workflows/engine.test.ts index c8208f97..382b6344 100644 --- a/packages/shared-domain/src/workflows/engine.test.ts +++ b/packages/shared-domain/src/workflows/engine.test.ts @@ -1361,6 +1361,19 @@ describe('remapped system folders', () => { expect(plan.wires.out.map((n) => n.title)).toEqual(['Gone']) }) + it('`folder inbox` and `in inbox` still mean THE inbox after a remap', async () => { + const source = await planWorkflow( + workflow([stmt('out', null, [step('folder', ['inbox'])])]), + makeCtx(remapReader, { systemFolderDirs: dirs }) + ) + expect(source.wires.out.map((n) => n.title)).toEqual(['Idea']) + const filter = await planWorkflow( + workflow([stmt('out', null, [step('all'), step('in', ['inbox'])])]), + makeCtx(remapReader, { systemFolderDirs: dirs }) + ) + expect(filter.wires.out.map((n) => n.title)).toEqual(['Idea']) + }) + it('the `trash` step projects into the remapped directory', async () => { const plan = await planWorkflow( workflow([stmt('out', null, [step('all'), step('trash')])]), @@ -1376,3 +1389,43 @@ describe('remapped system folders', () => { expect(folderTarget('trash', 'inbox/demo/X.md')).toBe('trash/demo/X.md') }) }) + +/* -------------------------------------------------------------------------- */ +/* Notes at the vault root (#840) */ +/* -------------------------------------------------------------------------- */ + +// With vault.json `primaryNotesLocation: root` the primary notes area is the +// root itself, so its directory is the empty string, which `folder` refuses on +// purpose, and no note has an `inbox` directory. The system name is the only +// way to spell it, and it must reach the subfolders too. +describe('notes at the vault root', () => { + const rootNotes: WorkflowNote[] = [ + { ...note('Dune.md', 'Dune', '', ['book'], {}, DAY), system: 'inbox' }, + { ...note('Areas/Gym/Plan.md', 'Plan', 'Areas/Gym', [], {}, DAY), system: 'inbox' }, + { ...note('quick/Scratch.md', 'Scratch', 'quick', [], {}, DAY), system: 'quick' }, + { ...note('archive/Old.md', 'Old', 'archive', ['book'], {}, DAY), system: 'archive' } + ] + const rootReader: VaultReader = { + listNotes: async () => rootNotes, + readBody: async () => '' + } + const out = async (steps: WorkflowStep[]): Promise => { + const plan = await planWorkflow(workflow([stmt('out', null, steps)]), makeCtx(rootReader)) + return plan.wires.out.map((n) => n.title) + } + + it('`folder inbox` is the root and everything under it', async () => { + expect(await out([step('folder', ['inbox'])])).toEqual(['Dune', 'Plan']) + }) + + it('a directory name still narrows to that directory', async () => { + expect(await out([step('folder', ['Areas'])])).toEqual(['Plan']) + expect(await out([step('folder', ['Areas/Gym'])])).toEqual(['Plan']) + }) + + it('the other system names keep their meaning', async () => { + expect(await out([step('folder', ['quick'])])).toEqual(['Scratch']) + expect(await out([step('folder', ['archive'])])).toEqual(['Old']) + expect(await out([step('tag', ['#book']), step('in', ['inbox'])])).toEqual(['Dune']) + }) +}) diff --git a/packages/shared-domain/src/workflows/engine.ts b/packages/shared-domain/src/workflows/engine.ts index 6120a212..bb6b18bf 100644 --- a/packages/shared-domain/src/workflows/engine.ts +++ b/packages/shared-domain/src/workflows/engine.ts @@ -750,6 +750,29 @@ function inFolder(note: WorkflowNote, folder: string): boolean { return own === target || own.startsWith(`${target}/`) } +/** The system folder a `folder`/`in` argument names, or null for a directory. */ +function bucketNamed(folder: string): NonNullable | null { + const name = normalizeFolder(folder).trim().toLowerCase() + return name === 'inbox' || name === 'quick' || name === 'archive' || name === 'trash' + ? name + : null +} + +/** + * `inFolder`, plus the four system names read as THE system folders. `folder + * trash` means the Trash rather than a directory that happens to carry that + * name, so on a vault with remapped system folders it matches by the reader's + * classification too, and `folder inbox` means the primary notes area + * wherever the vault keeps it. On a vault whose notes live at the root that + * is the root itself, which no directory name can spell: `folder ""` is + * refused above, and the root's own directory is that empty string (#840). + */ +function inFolderOrBucket(note: WorkflowNote, folder: string): boolean { + if (inFolder(note, folder)) return true + const bucket = bucketNamed(folder) + return bucket !== null && note.system === bucket +} + /** Whether a `folder`/`in` argument actually names something. */ function hasFolderName(folder: string): boolean { return normalizeFolder(folder).trim() !== '' @@ -947,18 +970,8 @@ async function runStep( const folder = argString(step, 'folder') if (folder === null) return missingArg(state, step, 'folder') if (!hasFolderName(folder)) return fail(state, '`folder` needs a name', step.line) - // `folder trash` / `folder archive` mean THE Trash / THE Archive, not a - // directory that happens to carry that name, so on a vault with remapped - // system folders they match by the reader's classification too. - const bucket = folder.trim().toLowerCase() - const matchesBucket = - bucket === 'trash' || bucket === 'archive' - ? (note: WorkflowNote): boolean => note.system === bucket - : (): boolean => false return keep( - (await allNotes(state, step.line)).filter( - (note) => inFolder(note, folder) || matchesBucket(note) - ) + (await allNotes(state, step.line)).filter((note) => inFolderOrBucket(note, folder)) ) } @@ -1035,7 +1048,7 @@ async function runStep( const folder = argString(step, 'folder') if (folder === null) return missingArg(state, step, 'folder') if (!hasFolderName(folder)) return fail(state, '`in` needs a folder name', step.line) - return keep(current.filter((note) => inFolder(note, folder))) + return keep(current.filter((note) => inFolderOrBucket(note, folder))) } case 'matching': { diff --git a/packages/shared-domain/src/workflows/nodes.ts b/packages/shared-domain/src/workflows/nodes.ts index 76edc7a0..fd4c6be6 100644 --- a/packages/shared-domain/src/workflows/nodes.ts +++ b/packages/shared-domain/src/workflows/nodes.ts @@ -153,7 +153,7 @@ export const NODE_DEFS: readonly NodeDef[] = [ category: 'source', title: 'Notes in folder', description: - 'Starts a pipeline with the notes in a folder, subfolders included.', + 'Starts a pipeline with the notes in a folder, subfolders included. The four system names (inbox, quick, archive, trash) mean those folders wherever the vault keeps them: on a vault whose notes live at the root, `folder inbox` is the vault root.', example: 'inbox = folder inbox', params: [p('folder', 'folder')], source: true, @@ -259,7 +259,7 @@ export const NODE_DEFS: readonly NodeDef[] = [ category: 'filter', title: 'In folder', description: - 'Keeps only the notes that live inside a folder, subfolders included.', + 'Keeps only the notes that live inside a folder, subfolders included. The four system names mean the system folders wherever the vault keeps them, as with `folder`.', example: 'in inbox/projects', params: [p('folder', 'folder')], source: false, diff --git a/packages/shared-domain/src/workflows/prepare-run.test.ts b/packages/shared-domain/src/workflows/prepare-run.test.ts index 49670246..26bd0855 100644 --- a/packages/shared-domain/src/workflows/prepare-run.test.ts +++ b/packages/shared-domain/src/workflows/prepare-run.test.ts @@ -27,10 +27,36 @@ describe('prepareWorkflowRun', () => { changes: [ { path: 'inbox/A.md', before: '# A\n', after: null }, { path: 'archive/A.md', before: null, after: '# A\ndone\n' } - ] + ], + moves: [{ from: 'inbox/A.md', to: 'archive/A.md' }] }) }) + it('lists the moves as they land, so the server can carry each note\'s comments', async () => { + const files = new Map([['inbox/A.md', '# A\n']]) + + const prepared = await prepareWorkflowRun( + { + workflowId: 'chain', + ops: [ + { kind: 'move', path: 'inbox/A.md', to: 'inbox/Work' }, + { kind: 'rename', path: 'inbox/Work/A.md', to: 'Final' }, + { kind: 'append', path: 'inbox/Work/Final.md', text: 'done' } + ] + }, + { + read: async (path) => files.get(path) ?? null, + systemFolderDirs: {} + } + ) + + // In order, each from where the note really is: a text op adds no move. + expect(prepared.moves).toEqual([ + { from: 'inbox/A.md', to: 'inbox/Work/A.md' }, + { from: 'inbox/Work/A.md', to: 'inbox/Work/Final.md' } + ]) + }) + it('refuses a create that would replace an existing note', async () => { await expect( prepareWorkflowRun( @@ -70,6 +96,8 @@ describe('prepareWorkflowRun', () => { { path: 'inbox/A.md', before: '# A\n', after: null }, { path: 'archive/A 2.md', before: null, after: '# A\ndone\n' } ]) + // The move names where the note landed, suffix and all. + expect(prepared.moves).toEqual([{ from: 'inbox/A.md', to: 'archive/A 2.md' }]) }) it('rejects malformed operations before reading or preparing files', async () => { diff --git a/packages/shared-domain/src/workflows/prepare-run.ts b/packages/shared-domain/src/workflows/prepare-run.ts index a1a74eb5..f1cb8430 100644 --- a/packages/shared-domain/src/workflows/prepare-run.ts +++ b/packages/shared-domain/src/workflows/prepare-run.ts @@ -21,12 +21,26 @@ export interface WorkflowRunFileChange { after: string | null } +/** One path op as it will land: the note's path before and after. */ +export interface WorkflowRunMove { + from: string + to: string +} + export interface PreparedWorkflowRun { workflowId: string ops: WorkflowOp[] applied: number irreversible: number changes: WorkflowRunFileChange[] + /** + * The path ops in the order they land. `changes` names only notes, and a + * note's comments live in `.zennotes`, which a change may not name; the + * server carries them along for each move and records them for undo. A + * server from before this field ignores it and moves the Markdown alone, as + * it always did. + */ + moves: WorkflowRunMove[] } export interface WorkflowRunSource { @@ -171,6 +185,7 @@ export async function prepareWorkflowRun( const live = new Map() const journal = new Map() const redirects = new Map() + const moves: WorkflowRunMove[] = [] const read = async (path: string): Promise => { const normalized = normalizeRel(path) @@ -240,6 +255,7 @@ export async function prepareWorkflowRun( await touch(destination) live.set(from, null) live.set(destination, body) + moves.push({ from, to: destination }) const promised = normalizeRel(promisedPath) if (destination !== promised) redirects.set(promised, destination) } @@ -301,6 +317,7 @@ export async function prepareWorkflowRun( ops, applied, irreversible: ops.filter((op) => IRREVERSIBLE_OP_KINDS.has(op.kind)).length, - changes + changes, + moves } } diff --git a/packages/shared-ui/package.json b/packages/shared-ui/package.json index a98b0d9b..4cdec708 100644 --- a/packages/shared-ui/package.json +++ b/packages/shared-ui/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/shared-ui", "private": true, - "version": "2.54.1", + "version": "2.55.0", "type": "module", "exports": { ".": "./src/index.ts"