From 9d69478aed78aa507ccc45dbf5801116ebb02a9e Mon Sep 17 00:00:00 2001 From: ther12k Date: Sun, 21 Jun 2026 17:28:18 +0000 Subject: [PATCH 01/22] fix(bases-filter-defaults): extract property from generated conjunction + map filter (closes #2043) The default-relationships Subtasks filter generated by formatProjectEntryLinkExpression in defaultBasesFiles.ts emits: file.hasLink(this.file) && list(note.PROP).map().asLink()).contains(this.file.asLink()) The currentFileContainsMatch regex captures the entire conjunction as the property expression. normalizeFilterProperty then sees: file.hasLink(this.file) && list(note.projects).map(...).asLink()) which fails every check (no leading 'list(' since the string starts with 'file.hasLink', no trailing match for the core field set, etc.) and returns null. Net effect: clicking the column '+' button on the default Subtasks tab creates a task with no 'projects' field, which then fails the view filter and disappears. Fix: strip the &&-joined left side and the generated .map(...) wrapper (balanced-paren walk, since .asLink() lives inside the .map argument) before the existing list()/note-prefix recognition runs. Tests: - 3 new regression tests in basesFilterDefaults.test.ts covering the generated core-field case, the user-defined-field case, and the missing-current-file-link fallback. - All 6/6 tests in basesFilterDefaults.test.ts pass. - All 13/13 tests across kanbanCreationDefaults, basesTaskCreation, KanbanView.manualOrderFastPath, and basesCreateFileForView pass. Refs: #2043, #1657, #1902. --- src/bases/basesFilterDefaults.ts | 38 ++++++++++- tests/unit/bases/basesFilterDefaults.test.ts | 68 ++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/src/bases/basesFilterDefaults.ts b/src/bases/basesFilterDefaults.ts index 7b1270185..d9debf425 100644 --- a/src/bases/basesFilterDefaults.ts +++ b/src/bases/basesFilterDefaults.ts @@ -139,11 +139,47 @@ function normalizeFilterProperty( options: BasesFilterDefaultOptions ): string | null { let property = propertyExpression.trim(); + + // Strip a leading `&&`-joined left side. The generated default-relationships + // filter (see `formatProjectEntryLinkExpression` in `defaultBasesFiles.ts`, + // issue #2043) concatenates `file.hasLink(this.file) &&` before the + // `list(note.PROP).map(...)` expression so the regex above matches the + // whole conjunction. Keep only the right-hand operand. + const conjunctionIndex = property.lastIndexOf("&&"); + if (conjunctionIndex !== -1) { + property = property.slice(conjunctionIndex + 2).trim(); + } + + // Strip generated `.map(...)` chains emitted by the relationship templates + // in `defaultBasesFiles.ts`. These wrap `list(note.PROP)` to normalize link + // formats (markdown links, `%20`, paths). The wrapper closes one balanced + // call, so find the last `.map(` and strip up to its matching `)`. + const mapStart = property.lastIndexOf(".map("); + if (mapStart !== -1) { + let depth = 0; + let endIndex = -1; + for (let i = mapStart + 5; i < property.length; i++) { + const ch = property[i]; + if (ch === "(") { + depth++; + } else if (ch === ")") { + if (depth === 0) { + endIndex = i; + break; + } + depth--; + } + } + if (endIndex !== -1) { + property = property.slice(0, mapStart).trim(); + } + } + const listMatch = property.match(/^list\((.+)\)$/); if (listMatch) { property = listMatch[1].trim(); } - property = property.replace(/^(note|task)\./, ""); + property = property.replace(/^(note|task|this\.note)\./, ""); if (property === "tags" || property === "file.tags") { return "tags"; diff --git a/tests/unit/bases/basesFilterDefaults.test.ts b/tests/unit/bases/basesFilterDefaults.test.ts index d4833dc6c..febc25070 100644 --- a/tests/unit/bases/basesFilterDefaults.test.ts +++ b/tests/unit/bases/basesFilterDefaults.test.ts @@ -124,4 +124,72 @@ describe("Bases filter defaults", () => { expect(defaults).toEqual({}); }); + + // Regression: issue #2043. The default relationships Subtasks filter + // generated by `formatProjectEntryLinkExpression` in `defaultBasesFiles.ts` + // emits `file.hasLink(this.file) && list(note.PROP).map().asLink()).contains(this.file.asLink())`. + // The `currentFileContainsMatch` regex captures the entire conjunction, so + // the parser must strip the `&&` left side and the generated `.map(...).asLink()` + // chain before extracting the property name. + it("extracts project default from generated `file.hasLink(this.file) &&` conjunction filter (#2043)", () => { + const generatedRule = + 'file.hasLink(this.file) && list(note.projects).map(file(value.replace(/^\\[[^\\]]+\\]\\((.*)\\)$/, "$1").replace(/%20/g, " ")).asLink()).contains(this.file.asLink())'; + + const defaults = extractBasesFilterDefaults({ + config: { + filters: { + rule: { text: generatedRule }, + }, + }, + fieldMapper: createFieldMapper(), + taskTag: "task", + currentFileLink: "[[Current]]", + }); + + expect(defaults).toEqual({ + projects: ["[[Current]]"], + }); + }); + + it("extracts user-defined field from generated conjunction + map filter (#2043)", () => { + const generatedRule = + 'file.hasLink(this.file) && list(note.customField).map(file(value.replace(/^\\[[^\\]]+\\]\\((.*)\\)$/, "$1").replace(/%20/g, " ")).asLink()).contains(this.file.asLink())'; + + const defaults = extractBasesFilterDefaults({ + config: { + filters: { + rule: { text: generatedRule }, + }, + }, + fieldMapper: createFieldMapper(), + taskTag: "task", + userFields: [{ key: "customField" }], + currentFileLink: "[[Current]]", + }); + + // Unknown user-defined fields fall through to scalar storage in + // `addFrontmatterDefault` (the list-merge path is reserved for the + // core list fields: tags, contexts, projects, blockedBy). + expect(defaults).toEqual({ + customField: "[[Current]]", + }); + }); + + it("falls back gracefully when current file link is missing for the generated filter (#2043)", () => { + const generatedRule = + 'file.hasLink(this.file) && list(note.projects).map(file(value.replace(/^\\[[^\\]]+\\]\\((.*)\\)$/, "$1").replace(/%20/g, " ")).asLink()).contains(this.file.asLink())'; + + const defaults = extractBasesFilterDefaults({ + config: { + filters: { + rule: { text: generatedRule }, + }, + }, + fieldMapper: createFieldMapper(), + taskTag: "task", + currentFileLink: null, + }); + + expect(defaults).toEqual({}); + }); }); From c251260c3d2e74f59cce2f1f53693801f613edca Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:52:47 +0100 Subject: [PATCH 02/22] fix: re-read frontmatter before modal save to prevent overwriting concurrent task edits --- docs/releases/unreleased.md | 3 + .../task-service/TaskUpdateService.ts | 39 +++++++++--- .../task-service/taskUpdatePlanning.ts | 13 ++-- ...TaskUpdateService.concurrent-edits.test.ts | 60 +++++++++++++++++++ ...sue-1696-gcal-recurring-reschedule.test.ts | 2 + tests/unit/services/TaskService.test.ts | 2 + .../unit/services/taskUpdatePlanning.test.ts | 2 + 7 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 tests/services/TaskUpdateService.concurrent-edits.test.ts diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index c29cde0dd..c0e770bec 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -34,6 +34,9 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l ## Fixed +- Preserve background task updates when saving an older task edit window, including recurring completion history and fields removed by another writer. + - Thanks to @martin-forge for the contribution. + - (#1849) Fixed context menus stacking on top of each other. Only one menu stays open at a time, and clicking the same indicator again closes its menu. This previously applied to date fields only, and now covers priority, status, recurrence, reminders, task, ICS event, and batch menus. - Thanks to @3zra47 for reporting and @YBKF for the contribution. diff --git a/src/services/task-service/TaskUpdateService.ts b/src/services/task-service/TaskUpdateService.ts index eee4a475e..6b00fd852 100644 --- a/src/services/task-service/TaskUpdateService.ts +++ b/src/services/task-service/TaskUpdateService.ts @@ -130,20 +130,43 @@ export class TaskUpdateService { newPath = parentPath ? `${parentPath}/${newFilename}.md` : `${newFilename}.md`; } - const recurrenceUpdates = buildTaskUpdateRecurrenceUpdates({ - originalTask, - updates: taskUpdates, - maintainDueDateOffsetInRecurring: runtime.settings.maintainDueDateOffsetInRecurring, - }); + let recurrenceUpdates: Partial = {}; const normalizedDetails = normalizeTaskUpdateDetails(taskUpdates); let finalTags: string[] | undefined; const dateModified = getCurrentTimestamp(); const currentDateString = - taskUpdates.status !== undefined && !originalTask.recurrence - ? getCurrentDateString() - : ""; + taskUpdates.status !== undefined ? getCurrentDateString() : ""; await processVaultFrontMatter(runtime.app, file, (frontmatter) => { + // Rebase the caller's named changes on the bytes Obsidian is about + // to modify. A modal's TaskInfo can predate an external completion. + const current = runtime.fieldMapper.mapFromFrontmatter( + frontmatter, + originalTask.path, + runtime.settings.storeTitleInFilename + ); + originalTask = { + // Retain derived view/body context; persisted fields come from the fresh mapping. + id: originalTask.id, + basesData: originalTask.basesData, + details: originalTask.details, + blocking: originalTask.blocking, + isBlocked: originalTask.isBlocked, + isBlocking: originalTask.isBlocking, + hasSubtasks: originalTask.hasSubtasks, + path: originalTask.path, + title: originalTask.title, + status: originalTask.status, + priority: originalTask.priority, + archived: false, + ...current, + }; + recurrenceUpdates = buildTaskUpdateRecurrenceUpdates({ + originalTask, + updates: taskUpdates, + maintainDueDateOffsetInRecurring: + runtime.settings.maintainDueDateOffsetInRecurring, + }); const frontmatterResult = applyTaskUpdateFrontmatterChange({ frontmatter, originalTask, diff --git a/src/services/task-service/taskUpdatePlanning.ts b/src/services/task-service/taskUpdatePlanning.ts index 460569e00..8bcc4c562 100644 --- a/src/services/task-service/taskUpdatePlanning.ts +++ b/src/services/task-service/taskUpdatePlanning.ts @@ -1,8 +1,5 @@ import type { FieldMappingKey, TaskInfo, TimeEntry } from "../../types"; -import { - addDTSTARTToRecurrenceRule, - updateToNextScheduledOccurrence, -} from "../../core/recurrence"; +import { addDTSTARTToRecurrenceRule, updateToNextScheduledOccurrence } from "../../core/recurrence"; import { applyGoogleCalendarRecurringExceptionCleanup, applyGoogleCalendarRecurringExceptionForScheduledChange, @@ -18,6 +15,11 @@ export type TaskUpdateInput = Partial & { }; export interface TaskUpdateFieldMapper { + mapFromFrontmatter: ( + frontmatter: unknown, + filePath: string, + storeTitleInFilename?: boolean + ) => Partial; mapToFrontmatter: ( taskData: Partial, taskTag?: string, @@ -196,8 +198,9 @@ export function applyTaskUpdateFrontmatterChange({ storeTitleInFilename, updateCompletedDateInFrontmatter, }: ApplyTaskUpdateFrontmatterChangeInput): ApplyTaskUpdateFrontmatterChangeResult { + // Publish only the named patch and its recurrence consequences. const completeTaskData: Partial = { - ...originalTask, + tags: getFrontmatterTags(frontmatter.tags), ...updates, ...recurrenceUpdates, dateModified, diff --git a/tests/services/TaskUpdateService.concurrent-edits.test.ts b/tests/services/TaskUpdateService.concurrent-edits.test.ts new file mode 100644 index 000000000..191290e5f --- /dev/null +++ b/tests/services/TaskUpdateService.concurrent-edits.test.ts @@ -0,0 +1,60 @@ +import { TFile } from "obsidian"; +import { TaskUpdateService } from "../../src/services/task-service/TaskUpdateService"; + +describe("concurrent task edits", () => { + it("rebases a stale modal patch on current recurrence fields and returns that state", async () => { + const file = Object.assign(Object.create(TFile.prototype), { path: "Tasks/example.md" }); + const fm: any = { + status: "ready", + priority: "normal", + scheduled: "2026-09-07", + recurrence: "DTSTART:20260905;FREQ=DAILY", + complete_instances: ["2026-09-05", "2026-09-06"], + timeEstimate: 5, + custom: "keep", + }; + const runtime: any = { + app: { + vault: { getAbstractFileByPath: () => file }, + fileManager: { processFrontMatter: async (_: unknown, fn: any) => fn(fm) }, + }, + settings: { + storeTitleInFilename: false, + maintainDueDateOffsetInRecurring: false, + taskIdentificationMethod: "property", + taskPropertyName: "type", + taskPropertyValue: "task", + }, + fieldMapper: { + mapFromFrontmatter: (value: any) => ({ ...value }), + mapToFrontmatter: (value: any) => ({ ...value }), + toUserField: (key: string) => key, + }, + cacheManager: { updateTaskInfoInCache: jest.fn() }, + emitter: { trigger: jest.fn() }, + statusManager: { isCompletedStatus: () => false }, + }; + const service = new TaskUpdateService({ + runtime, + updateCompletedDateInFrontmatter: () => {}, + }); + const stale: any = { + ...fm, + path: file.path, + title: "Example", + scheduled: "2026-09-06", + due: "2026-09-01", + complete_instances: ["2026-09-05"], + }; + const result = await service.updateTask(stale, { timeEstimate: 10 }); + expect(fm.scheduled).toBe("2026-09-07"); + expect(fm.complete_instances).toEqual(["2026-09-05", "2026-09-06"]); + expect(fm.custom).toBe("keep"); + expect(fm.due).toBeUndefined(); + expect(result.due).toBeUndefined(); + expect(result.scheduled).toBe("2026-09-07"); + expect(result.complete_instances).toEqual(fm.complete_instances); + expect(result.timeEstimate).toBe(10); + expect(stale.scheduled).toBe("2026-09-06"); + }); +}); diff --git a/tests/unit/issues/issue-1696-gcal-recurring-reschedule.test.ts b/tests/unit/issues/issue-1696-gcal-recurring-reschedule.test.ts index 1782362d0..23ee72931 100644 --- a/tests/unit/issues/issue-1696-gcal-recurring-reschedule.test.ts +++ b/tests/unit/issues/issue-1696-gcal-recurring-reschedule.test.ts @@ -64,6 +64,7 @@ function createGoogleSyncPlugin(frontmatter: Record = {}) { }, }, fieldMapper: { + mapFromFrontmatter: (fm: Record) => ({ ...fm }), toUserField: jest.fn((field: string) => field), mapToFrontmatter: jest.fn((taskData: Record) => { const mapped: Record = {}; @@ -131,6 +132,7 @@ describe("Issue #1696: Google Calendar recurring reschedule sync", () => { googleCalendarEventId: "master-event-id", } as TaskInfo; + Object.assign(frontmatter, task); const updatedTask = await taskService.updateTask(task, { scheduled: "2026-04-15", }); diff --git a/tests/unit/services/TaskService.test.ts b/tests/unit/services/TaskService.test.ts index 079e311a1..f5c7fea90 100644 --- a/tests/unit/services/TaskService.test.ts +++ b/tests/unit/services/TaskService.test.ts @@ -1425,6 +1425,7 @@ describe('TaskService', () => { complete_instances: ['2024-12-30', '2024-12-31'] }); + mockPlugin.app.fileManager.processFrontMatter.mockImplementation(async (_file, fn) => fn({...recurringTask})); const result = await taskService.updateTask(recurringTask, { priority: 'high' }); expect(result.complete_instances).toEqual(['2024-12-30', '2024-12-31']); @@ -1465,6 +1466,7 @@ describe('TaskService', () => { const taskWithTags = TaskFactory.createTask({ tags: ['task', 'important'] }); const updates = { priority: 'high' }; + mockPlugin.app.fileManager.processFrontMatter.mockImplementation(async (_file, fn) => fn({...taskWithTags})); const result = await taskService.updateTask(taskWithTags, updates); expect(result.tags).toEqual(['task', 'important']); diff --git a/tests/unit/services/taskUpdatePlanning.test.ts b/tests/unit/services/taskUpdatePlanning.test.ts index 5c83ca4fe..98fe80da2 100644 --- a/tests/unit/services/taskUpdatePlanning.test.ts +++ b/tests/unit/services/taskUpdatePlanning.test.ts @@ -40,6 +40,7 @@ function createFieldMapper(): TaskUpdateFieldMapper { }; return { + mapFromFrontmatter: (frontmatter) => frontmatter as Partial, mapToFrontmatter: (taskData, taskTag, storeTitleInFilename) => { const frontmatter: Record = { title: taskData.title, @@ -162,6 +163,7 @@ describe("taskUpdatePlanning", () => { it("applies mapped updates, custom frontmatter, removals, and task identification", () => { const frontmatter: Record = { + priority: "normal", title: "Old", status: "open", due: "2026-05-19", From 36cc531d972814f05fbd7baaeab7d4ff1689d7c7 Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:52:48 +0100 Subject: [PATCH 03/22] fix: stop startup when an existing settings file cannot be read --- docs/releases/unreleased.md | 3 +++ src/main.ts | 4 ++++ tests/unit/issues/issue-1591-settings-lost-on-update.test.ts | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index c29cde0dd..848c6059e 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -34,6 +34,9 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l ## Fixed +- Stop plugin startup when an existing settings file cannot be read, preserving it for recovery instead of starting services with default settings. + - Thanks to @martin-forge for the contribution. + - (#1849) Fixed context menus stacking on top of each other. Only one menu stays open at a time, and clicking the same indicator again closes its menu. This previously applied to date fields only, and now covers priority, status, recurrence, reminders, task, ICS event, and batch menus. - Thanks to @3zra47 for reporting and @YBKF for the contribution. diff --git a/src/main.ts b/src/main.ts index 4a967ff72..d1fe8ad43 100644 --- a/src/main.ts +++ b/src/main.ts @@ -744,6 +744,10 @@ export default class TaskNotesPlugin extends Plugin { }, }); } + if (result.compromised) + throw new Error( + "TaskNotes settings are unreadable; restore settings before loading the plugin." + ); return result.data; } diff --git a/tests/unit/issues/issue-1591-settings-lost-on-update.test.ts b/tests/unit/issues/issue-1591-settings-lost-on-update.test.ts index bcd377f6e..4f151779d 100644 --- a/tests/unit/issues/issue-1591-settings-lost-on-update.test.ts +++ b/tests/unit/issues/issue-1591-settings-lost-on-update.test.ts @@ -38,8 +38,8 @@ describe("issue #1591 settings reset on update", () => { const plugin = createPlugin({ dataFileExists: true }); plugin.loadData = jest.fn().mockResolvedValue(null); - await plugin.loadSettings(); - await plugin.checkForVersionUpdate(); + await expect(plugin.loadSettings()).rejects.toThrow("settings are unreadable"); + await plugin.saveSettingsDataOnly(); expect(plugin.loadData).toHaveBeenCalledTimes(4); expect(plugin.saveData).not.toHaveBeenCalled(); From bedab0fd9452c41c2227e173e02a100e4e49204c Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:22:35 +0100 Subject: [PATCH 04/22] Remove self-attribution from release note --- docs/releases/unreleased.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index c0e770bec..7262fcaa0 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -35,7 +35,6 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l ## Fixed - Preserve background task updates when saving an older task edit window, including recurring completion history and fields removed by another writer. - - Thanks to @martin-forge for the contribution. - (#1849) Fixed context menus stacking on top of each other. Only one menu stays open at a time, and clicking the same indicator again closes its menu. This previously applied to date fields only, and now covers priority, status, recurrence, reminders, task, ICS event, and batch menus. - Thanks to @3zra47 for reporting and @YBKF for the contribution. From bbf53a0bb0dd6168ad1b3e6d7a49f15d75d48bed Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:22:43 +0100 Subject: [PATCH 05/22] Remove self-attribution from release note --- docs/releases/unreleased.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index 848c6059e..6bcd3b76b 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -35,7 +35,6 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l ## Fixed - Stop plugin startup when an existing settings file cannot be read, preserving it for recovery instead of starting services with default settings. - - Thanks to @martin-forge for the contribution. - (#1849) Fixed context menus stacking on top of each other. Only one menu stays open at a time, and clicking the same indicator again closes its menu. This previously applied to date fields only, and now covers priority, status, recurrence, reminders, task, ICS event, and batch menus. - Thanks to @3zra47 for reporting and @YBKF for the contribution. From 4195cad91d9189c6d87e68d64cd99cc5c6032f07 Mon Sep 17 00:00:00 2001 From: Nelson Love Date: Tue, 8 Sep 2026 03:48:33 -0400 Subject: [PATCH 06/22] fix(reading-mode): nest the task card inside the header so virtualisation cannot delete it Reading mode is virtualised, and every render pass ends with `sizerEl.setChildrenInPlace([pusherEl, ...shownSections])`, which deletes any direct child of `.markdown-preview-sizer` that Obsidian did not put there. The task card was injected between sections, so it was deleted on every pass and re-injected on the next frame. During a scroll that cycle ran continuously and dragged the reader down the note until it hit the bottom (#2255). The scroll quiet period added in b29a29e6 defers re-injection but cannot stop this, because the problem is where the re-injection lands rather than how often it happens. Once the reader scrolls past the top of the note, Obsidian has detached `.mod-header.mod-ui` too, so `getMetadataOrHeaderInsertionReference` falls through to the preview pusher - whose next sibling is the first section that is *currently rendered*, not the first section of the note. The card is therefore inserted into the middle of the text being read, shoving the page down by its own height. Measured on two notes, each correction was exactly the card's occupied height (579px and 690px), and the corrections arrived every 218-262ms, which is the 200ms quiet period plus a frame. `setChildrenInPlace` only manages *direct* children, so nesting the card one level deeper leaves it untouched by the render pass. It is now placed inside the header, after the properties block. Two supporting changes make that safe: - The observer skips re-injection while the header is not rendered. Obsidian detaches the header with the card inside it when the reader scrolls past the top; that must be left alone rather than fought, and the card is restored when the header comes back. - Injection anchors on the properties block rather than the end of the header. Obsidian builds the header in stages, so appending during an early pass put the card above the properties, which then rendered underneath and shoved it down - a visible jolt on every note open. Nesting also corrects the height accounting noted in the issue: `measureSection` takes a section's height as the gap between its own `offsetTop` and its next sibling's, so a widget between sections is counted in neither. Inside the header it is counted in that section. Spacing is scoped to `.markdown-preview-view .mod-header` so Live Preview and canvas are unchanged. The gap below the card uses `--p-spacing`, the note's own gap between blocks. The `:has()` selector that trims the properties block's bottom margin needs Chromium 105+; where it does not apply the gap is simply larger, never broken. Note that requiring the properties block means a task note rendering with no `.metadata-container` at all would not receive a card. --- src/editor/MarkdownWidgetInsertion.ts | 57 +++++++ src/editor/ReadingModeWidgetObserver.ts | 11 ++ src/editor/TaskCardNoteDecorations.ts | 17 +- styles/task-card-note-widget.css | 19 +++ .../editor/MarkdownWidgetInsertion.test.ts | 149 ++++++++++++++++++ 5 files changed, 251 insertions(+), 2 deletions(-) create mode 100644 tests/unit/editor/MarkdownWidgetInsertion.test.ts diff --git a/src/editor/MarkdownWidgetInsertion.ts b/src/editor/MarkdownWidgetInsertion.ts index fe2e26fa0..fa4cca92f 100644 --- a/src/editor/MarkdownWidgetInsertion.ts +++ b/src/editor/MarkdownWidgetInsertion.ts @@ -58,6 +58,63 @@ export function getMetadataOrHeaderInsertionReference(container: HTMLElement): C return container.firstChild; } +/** + * Places a widget inside the note's header block, after the properties. + * + * Reading mode is virtualised: every render pass ends with + * `sizerEl.setChildrenInPlace([pusherEl, ...shownSections])`, which deletes any + * direct child of the sizer that Obsidian did not put there. A widget injected + * between sections is therefore removed on each pass and re-injected on the next + * frame, and that add/remove cycle drags the scroll position down the note (#2255). + * + * `setChildrenInPlace` only manages *direct* children, so nesting the widget one + * level deeper leaves it untouched by the render pass. Nesting also fixes the height + * accounting: `measureSection` takes a section's height as the gap between its own + * `offsetTop` and its next sibling's, so a widget inside the header is counted in + * that section instead of falling between two sections and being counted in neither. + * + * Obsidian still detaches the whole header when it scrolls out of the render window, + * taking the widget with it. That is fine and must not be fought: callers check + * `hasMetadataOrHeaderAnchor` first and skip injection while the header is gone, + * because the fallback placement would land the widget in the middle of the text + * being read. + * + * Returns false when there is no header to nest into. + */ +export function insertInsideHeaderAnchor(container: HTMLElement, widget: HTMLElement): boolean { + const header = findDirectHeader(container); + if (!header) { + return false; + } + + // Anchor on the properties block rather than the end of the header. Obsidian + // builds the header in stages, so appending to it during an early pass puts the + // widget above the properties, which then render underneath and shove it down - + // a visible jolt on every note open. Waiting for the properties block means the + // widget is placed once, in its final position. + const metadata = header.querySelector(".metadata-container"); + if (!metadata) { + return false; + } + + metadata.insertAdjacentElement("afterend", widget); + return true; +} + +/** + * Whether the header the widget is nested into is currently rendered. + * + * Obsidian detaches sections that scroll out of the render window, and the note's + * `.mod-header.mod-ui` is one of them. While it is gone there is nowhere correct to + * put the widget: `getMetadataOrHeaderInsertionReference` would fall through to the + * preview pusher, whose next sibling is the first section that is *currently + * rendered* rather than the first section of the note (#2255). + */ +export function hasMetadataOrHeaderAnchor(container: HTMLElement): boolean { + const header = findDirectHeader(container); + return header !== null && header.querySelector(".metadata-container") !== null; +} + export function insertAfterMetadataOrHeader(container: HTMLElement, widget: HTMLElement): void { container.insertBefore(widget, getMetadataOrHeaderInsertionReference(container)); } diff --git a/src/editor/ReadingModeWidgetObserver.ts b/src/editor/ReadingModeWidgetObserver.ts index 01f71fd0e..c7b54cecb 100644 --- a/src/editor/ReadingModeWidgetObserver.ts +++ b/src/editor/ReadingModeWidgetObserver.ts @@ -1,5 +1,6 @@ import { MarkdownView, WorkspaceLeaf } from "obsidian"; import { shouldSkipMarkdownWidgetLeaf } from "./MarkdownWidgetContext"; +import { hasMetadataOrHeaderAnchor } from "./MarkdownWidgetInsertion"; /** * How long after the last scroll event re-injection stays deferred. @@ -122,6 +123,16 @@ export function observeReadingModeWidgetMutations( return; } + // The header these widgets nest into is itself a virtualised section, so it + // is absent while the reader is scrolled past the top of the note. Injecting + // then falls back to the preview pusher, which lands the widget in the middle + // of the text being read and shoves the page down by its height, once per + // attempt (#2255). This observer runs again when the header is rendered, so + // skipping here defers the widget rather than dropping it. + if (!hasMetadataOrHeaderAnchor(sizer)) { + return; + } + // While scrolling, Obsidian's own virtualisation keeps removing the // widget. Re-injecting every frame only creates DOM churn and scroll // corrections, so wait until scrolling settles before restoring it. diff --git a/src/editor/TaskCardNoteDecorations.ts b/src/editor/TaskCardNoteDecorations.ts index 605644501..3c4cb45ed 100644 --- a/src/editor/TaskCardNoteDecorations.ts +++ b/src/editor/TaskCardNoteDecorations.ts @@ -60,7 +60,10 @@ import { shouldSkipMarkdownWidgetEditor, shouldSkipMarkdownWidgetLeaf, } from "./MarkdownWidgetContext"; -import { insertAfterMetadataOrHeader } from "./MarkdownWidgetInsertion"; +import { + insertAfterMetadataOrHeader, + insertInsideHeaderAnchor, +} from "./MarkdownWidgetInsertion"; import { createTaskNotesLogger } from "../utils/tasknotesLogger"; const tasknotesLogger = createTaskNotesLogger({ tag: "Editor/TaskCardNoteDecorations" }); @@ -643,7 +646,17 @@ async function injectReadingModeWidget( return; } - insertAfterMetadataOrHeader(sizer, widget); + // Nest inside the header rather than sitting between sections. Obsidian + // deletes unexpected *direct* children of the sizer on every render pass, so + // a widget placed between sections churns and drags the scroll position down + // the note (#2255). While the header is scrolled out of the render window + // there is nowhere correct to put the widget, so skip and let the observer + // re-run once it is back. + if (!insertInsideHeaderAnchor(sizer, widget)) { + widget.component?.unload(); + widget.remove(); + return; + } } catch (error) { tasknotesLogger.error("[TaskNotes] Error injecting task card widget in reading mode:", { category: "persistence", diff --git a/styles/task-card-note-widget.css b/styles/task-card-note-widget.css index 676c7363a..18fb764ff 100644 --- a/styles/task-card-note-widget.css +++ b/styles/task-card-note-widget.css @@ -17,6 +17,25 @@ margin-inline: auto; } +/* + * Reading mode nests the widget inside `.mod-header.mod-ui`, after the properties + * block, so that Obsidian's virtualisation cannot delete it (see + * MarkdownWidgetInsertion and #2255). That host spaces differently from sitting + * between sections: the properties block already contributes a large bottom margin + * above the widget, and the header ends flush with it, leaving nothing between the + * widget and the note body. Rebalance for that host only. + */ +.markdown-preview-view .mod-header .metadata-container:has(+ .task-card-note-widget) { + margin-bottom: var(--cs-spacing-md); +} + +.markdown-preview-view .mod-header .task-card-note-widget { + margin-block-start: 0; + /* The note's own gap between blocks, so the widget sits in the same vertical + rhythm as a paragraph rather than on a value of our choosing. */ + margin-block-end: var(--p-spacing); +} + .task-card-note-widget__card { /* Reset margins for the inner task card */ margin: 0; diff --git a/tests/unit/editor/MarkdownWidgetInsertion.test.ts b/tests/unit/editor/MarkdownWidgetInsertion.test.ts new file mode 100644 index 000000000..6a667e269 --- /dev/null +++ b/tests/unit/editor/MarkdownWidgetInsertion.test.ts @@ -0,0 +1,149 @@ +import { afterEach, describe, expect, it } from "@jest/globals"; + +import { + getMetadataOrHeaderInsertionReference, + hasMetadataOrHeaderAnchor, + insertInsideHeaderAnchor, +} from "../../../src/editor/MarkdownWidgetInsertion"; + +/** + * Builds a stand-in for the reading-mode DOM. + * + * `withHeader: false` models what Obsidian leaves behind once the reader has scrolled + * past the top of the note: the header section is detached, and the pusher is followed + * by whichever section happens to be rendered. + */ +function buildSizer({ withHeader }: { withHeader: boolean }): HTMLElement { + const scroller = document.createElement("div"); + scroller.className = "markdown-preview-view"; + + const sizer = document.createElement("div"); + sizer.className = "markdown-preview-sizer"; + + const pusher = document.createElement("div"); + pusher.className = "markdown-preview-pusher"; + sizer.appendChild(pusher); + + if (withHeader) { + const header = document.createElement("div"); + header.className = "mod-header mod-ui"; + const metadata = document.createElement("div"); + metadata.className = "metadata-container"; + header.appendChild(metadata); + sizer.appendChild(header); + } + + const section = document.createElement("div"); + section.className = "el-p"; + section.textContent = "a section the reader is currently looking at"; + sizer.appendChild(section); + + scroller.appendChild(sizer); + // Attach to the document so `isConnected` means what it says: without this it is + // false for everything and the survival assertions would pass vacuously. + document.body.appendChild(scroller); + return sizer; +} + +function widgetEl(): HTMLElement { + const widget = document.createElement("div"); + widget.className = "tasknotes-task-card-note-widget"; + return widget; +} + +/** + * Obsidian ends every render pass with + * `sizerEl.setChildrenInPlace([pusherEl, ...shownSections])`, which drops any direct + * child of the sizer it did not put there. Nested nodes are untouched. + */ +function simulateRenderPass(sizer: HTMLElement): void { + Array.from(sizer.children).forEach((child) => { + const keep = + child.classList.contains("markdown-preview-pusher") || + child.className.startsWith("el-") || + child.classList.contains("mod-header"); + if (!keep) { + child.remove(); + } + }); +} + +afterEach(() => { + document.body.innerHTML = ""; +}); + +describe("insertInsideHeaderAnchor", () => { + it("nests the widget in the header, after the properties block", () => { + const sizer = buildSizer({ withHeader: true }); + const widget = widgetEl(); + + expect(insertInsideHeaderAnchor(sizer, widget)).toBe(true); + + const header = sizer.querySelector(".mod-header") as HTMLElement; + expect(widget.parentElement).toBe(header); + expect(Array.from(header.children).map((child) => child.className)).toEqual([ + "metadata-container", + "tasknotes-task-card-note-widget", + ]); + }); + + it("survives a render pass, which is the whole point", () => { + const sizer = buildSizer({ withHeader: true }); + const widget = widgetEl(); + insertInsideHeaderAnchor(sizer, widget); + + simulateRenderPass(sizer); + + expect(widget.isConnected).toBe(true); + }); + + it("reports failure when the header has been virtualised away", () => { + const sizer = buildSizer({ withHeader: false }); + expect(insertInsideHeaderAnchor(sizer, widgetEl())).toBe(false); + }); + + it("waits for the properties block instead of landing above it", () => { + // Obsidian builds the header in stages. Injecting into a half-built header puts + // the widget above the properties, which then render underneath and shove it + // down - a visible jolt on every note open. + const sizer = buildSizer({ withHeader: true }); + const header = sizer.querySelector(".mod-header") as HTMLElement; + header.querySelector(".metadata-container")?.remove(); + + expect(hasMetadataOrHeaderAnchor(sizer)).toBe(false); + expect(insertInsideHeaderAnchor(sizer, widgetEl())).toBe(false); + }); +}); + +describe("hasMetadataOrHeaderAnchor", () => { + it("is true while the header is rendered", () => { + expect(hasMetadataOrHeaderAnchor(buildSizer({ withHeader: true }))).toBe(true); + }); + + it("is false once the header has been virtualised away", () => { + expect(hasMetadataOrHeaderAnchor(buildSizer({ withHeader: false }))).toBe(false); + }); +}); + +describe("the between-sections placement this replaces", () => { + it("is deleted by a render pass", () => { + const sizer = buildSizer({ withHeader: true }); + const widget = widgetEl(); + sizer.insertBefore(widget, getMetadataOrHeaderInsertionReference(sizer)); + + simulateRenderPass(sizer); + + expect(widget.isConnected).toBe(false); + }); + + it("lands in the middle of the reader's viewport once the header is gone", () => { + // The trap behind #2255: with the header detached the insertion reference is the + // first *rendered* section, so re-injecting drops the widget into the text being + // read and shoves the page down by the widget's own height. + const sizer = buildSizer({ withHeader: false }); + const reference = getMetadataOrHeaderInsertionReference(sizer); + + expect((reference as HTMLElement).className).toBe("el-p"); + expect(hasMetadataOrHeaderAnchor(sizer)).toBe(false); + }); +}); From 8a41f5b3e42a3c94f34c0944bfa6aa248e72120f Mon Sep 17 00:00:00 2001 From: Nelson Love Date: Tue, 8 Sep 2026 03:51:16 -0400 Subject: [PATCH 07/22] fix(reading-mode): make the header-anchor guard opt-in per widget `observeReadingModeWidgetMutations` is shared: TaskCardNoteDecorations and RelationshipsDecorations both use it. Requiring the header anchor unconditionally would have blocked the relationships widget from injecting whenever the header is detached - which is precisely where that widget lives, since it defaults to the bottom of the note. The guard is now `requireHeaderAnchor`, off by default, set only by the task card, which is the widget that nests inside the header. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SBWuKe71PRSJaN2SM4xUFJ --- src/editor/ReadingModeWidgetObserver.ts | 24 +++++++++++++++++------- src/editor/TaskCardNoteDecorations.ts | 5 ++++- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/editor/ReadingModeWidgetObserver.ts b/src/editor/ReadingModeWidgetObserver.ts index c7b54cecb..fdb25df00 100644 --- a/src/editor/ReadingModeWidgetObserver.ts +++ b/src/editor/ReadingModeWidgetObserver.ts @@ -18,6 +18,16 @@ export const DEFAULT_SCROLL_QUIET_PERIOD_MS = 200; export interface ReadingModeObserverOptions { /** Override the quiet period; 0 disables scroll deferral (used by tests). */ scrollQuietPeriodMs?: number; + /** + * Only inject while the note's header is rendered. + * + * For widgets nested inside `.mod-header.mod-ui`, which Obsidian detaches once + * the reader scrolls past the top of the note. Injecting then falls back to the + * preview pusher and lands the widget in the middle of the text being read + * (#2255). Off by default: widgets positioned elsewhere in the note, such as the + * relationships widget at the bottom, must still inject when the header is gone. + */ + requireHeaderAnchor?: boolean; } type FrameHandle = { @@ -94,6 +104,7 @@ export function observeReadingModeWidgetMutations( } const scrollQuietPeriodMs = options.scrollQuietPeriodMs ?? DEFAULT_SCROLL_QUIET_PERIOD_MS; + const requireHeaderAnchor = options.requireHeaderAnchor ?? false; let pendingFrame: FrameHandle | null = null; let lastScrollAt = Number.NEGATIVE_INFINITY; @@ -123,13 +134,12 @@ export function observeReadingModeWidgetMutations( return; } - // The header these widgets nest into is itself a virtualised section, so it - // is absent while the reader is scrolled past the top of the note. Injecting - // then falls back to the preview pusher, which lands the widget in the middle - // of the text being read and shoves the page down by its height, once per - // attempt (#2255). This observer runs again when the header is rendered, so - // skipping here defers the widget rather than dropping it. - if (!hasMetadataOrHeaderAnchor(sizer)) { + // A header-nested widget has nowhere correct to go while the header is + // scrolled out of the render window: injection would fall back to the preview + // pusher and land it in the middle of the text being read (#2255). This + // observer runs again when the header returns, so skipping defers the widget + // rather than dropping it. Only applies to callers that opted in. + if (requireHeaderAnchor && !hasMetadataOrHeaderAnchor(sizer)) { return; } diff --git a/src/editor/TaskCardNoteDecorations.ts b/src/editor/TaskCardNoteDecorations.ts index 3c4cb45ed..93afa5f49 100644 --- a/src/editor/TaskCardNoteDecorations.ts +++ b/src/editor/TaskCardNoteDecorations.ts @@ -750,7 +750,10 @@ export function setupReadingModeHandlers(plugin: TaskNotesPlugin): () => void { scheduleInjection, observedMarkdownContainers, markdownWidgetObserverCleanups, - shouldRefreshMarkdownLeaf + shouldRefreshMarkdownLeaf, + // The card nests inside the header, so it must not be re-injected while + // Obsidian has that section detached. + { requireHeaderAnchor: true } ); }; const observeMarkdownLeaves = () => { From a17ba2e118dae465e4943797360be0c8ecba44c1 Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:26:23 +0100 Subject: [PATCH 08/22] fix(calendar): keep event placement ahead of Bases rank --- docs/releases/unreleased.md | 2 ++ src/bases/CalendarView.ts | 4 ++- .../issue-1411-agenda-bases-sort.test.ts | 36 +++++++++++++++++-- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index 21617927f..2f8bde999 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -16,6 +16,8 @@ Example: ``` ## Fixed +- Keep agenda events chronological when a Base sort is configured; use Base order at the same calendar time. Follow-up to #1411, reported by @ky1ejs. + - (#768) Fixed calendar view appearing empty in week and day views due to invalid time configuration values - Added time validation in settings UI with proper error messages and debouncing - Prevents "Cannot read properties of null (reading 'years')" error from FullCalendar diff --git a/src/bases/CalendarView.ts b/src/bases/CalendarView.ts index 9ca0bf0b6..d5f33d8a0 100644 --- a/src/bases/CalendarView.ts +++ b/src/bases/CalendarView.ts @@ -179,7 +179,9 @@ export function getTaskNotesCalendarEventOrder(sortConfig: unknown): string { if (!hasBasesCalendarSortConfig(sortConfig)) { return DEFAULT_CALENDAR_EVENT_ORDER; } - return `${TASKNOTES_CALENDAR_SORT_INDEX},${DEFAULT_CALENDAR_EVENT_ORDER}`; + // Bases ranks break ties at the same placement. They must not put a timed + // task ahead of an earlier appointment that has no Bases result index. + return `start,allDay,${TASKNOTES_CALENDAR_SORT_INDEX},-duration,title`; } function getCalendarEventSortPath(event: EventInput): string | null { diff --git a/tests/unit/issues/issue-1411-agenda-bases-sort.test.ts b/tests/unit/issues/issue-1411-agenda-bases-sort.test.ts index e549a214b..e0643f4e0 100644 --- a/tests/unit/issues/issue-1411-agenda-bases-sort.test.ts +++ b/tests/unit/issues/issue-1411-agenda-bases-sort.test.ts @@ -1,4 +1,5 @@ import type { EventInput } from "@fullcalendar/core"; +import { execFileSync } from "node:child_process"; import { applyBasesSortIndexesToCalendarEvents, getTaskNotesCalendarEventOrder, @@ -13,15 +14,46 @@ describe("Issue #1411: Agenda Calendar Bases respect Bases sort order", () => { expect(getTaskNotesCalendarEventOrder(undefined)).toBe("start,-duration,allDay,title"); }); - it("puts the TaskNotes sort index first when the Base has a sort config", () => { + it("uses Bases order after event time, before duration and title", () => { const sortConfig = [{ column: "note.status", direction: "ASC" }]; expect(hasBasesCalendarSortConfig(sortConfig)).toBe(true); expect(getTaskNotesCalendarEventOrder(sortConfig)).toBe( - `${TASKNOTES_CALENDAR_SORT_INDEX},start,-duration,allDay,title` + `start,allDay,${TASKNOTES_CALENDAR_SORT_INDEX},-duration,title` ); }); + it("interleaves timed tasks with appointments while sorting untimed tasks by Bases rank", () => { + const order = getTaskNotesCalendarEventOrder([{ column: "status" }]); + const events = [ + { title: "Evening task", start: 20, tasknotesSortIndex: 0 }, + { title: "Morning appointment", start: 9 }, + { title: "Morning task", start: 8, tasknotesSortIndex: 3 }, + { title: "Ready", start: 0, tasknotesSortIndex: 2 }, + { title: "Doing", start: 0, tasknotesSortIndex: 1 }, + ]; + // The suite mocks FullCalendar. Exercise its real comparator in Node. + const sorted = JSON.parse( + execFileSync( + process.execPath, + [ + "-e", + "const {parseFieldSpecs,compareByFieldSpecs}=require('@fullcalendar/core/internal'); const events=JSON.parse(process.argv[2]); const specs=parseFieldSpecs(process.argv[1]); console.log(JSON.stringify(events.sort((a,b)=>compareByFieldSpecs(a,b,specs)).map(e=>e.title)));", + order, + JSON.stringify(events), + ], + { encoding: "utf8" } + ) + ); + expect(sorted).toEqual([ + "Doing", + "Ready", + "Morning task", + "Morning appointment", + "Evening task", + ]); + }); + it("adds Bases result indexes to task and property-based events", () => { const events: EventInput[] = [ { From a48e5632b9a7648c1f29aadc6f3ff125322d90a6 Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:28:04 +0100 Subject: [PATCH 09/22] fix(calendar): suppress recorded moved recurrence projections --- docs/releases/unreleased.md | 2 + src/bases/calendar-core.ts | 9 ++++- ...rring-calendar-instance-visibility.test.ts | 37 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index 21617927f..adcaf0e88 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -16,6 +16,8 @@ Example: ``` ## Fixed +- Avoid showing a recurring task twice when its original occurrence date is recorded by a calendar move. Keep requested completion and skip history visible. + - (#768) Fixed calendar view appearing empty in week and day views due to invalid time configuration values - Added time validation in settings UI with proper error messages and debouncing - Prevents "Cannot read properties of null (reading 'years')" error from FullCalendar diff --git a/src/bases/calendar-core.ts b/src/bases/calendar-core.ts index 0a25be2c2..8d5281b18 100644 --- a/src/bases/calendar-core.ts +++ b/src/bases/calendar-core.ts @@ -1141,6 +1141,13 @@ export function generateRecurringTaskInstances( const hasOriginalTime = hasTimeComponent(task.scheduled); const templateTime = getRecurringTime(task); const nextScheduledDate = getDatePart(task.scheduled); + // A moved occurrence is represented at its current scheduled placement. + // Its original rule date must not also become a projected task. Recorded + // completions/skips are handled separately below and remain available. + const movedOriginalDates = new Set(task.googleCalendarMovedOriginalDates || []); + if (task.googleCalendarExceptionOriginalScheduled) { + movedOriginalDates.add(getDatePart(task.googleCalendarExceptionOriginalScheduled)); + } const spanDayOffset = showScheduledToDueSpan ? getScheduledToDueSpanDayOffset(task) : null; const shouldCreateRecurringSpan = spanDayOffset !== null; const recurringSearchStartDate = shouldCreateRecurringSpan @@ -1227,7 +1234,7 @@ export function generateRecurringTaskInstances( } // Skip if conflicts with next scheduled occurrence - if (instanceDate === nextScheduledDate) { + if (instanceDate === nextScheduledDate || movedOriginalDates.has(instanceDate)) { continue; } diff --git a/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts b/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts index c444673fc..7a5b66c4e 100644 --- a/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts +++ b/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts @@ -35,6 +35,43 @@ describe("Issue #1603: recurring calendar instance visibility", () => { const start = new Date("2026-02-01T00:00:00.000Z"); const end = new Date("2026-02-06T00:00:00.000Z"); + it("renders a moved occurrence once and keeps later weekly occurrences", () => { + const task = TaskFactory.createRecurringTask("DTSTART:20260905;FREQ=WEEKLY;BYDAY=SA", { + scheduled: "2026-09-11", + googleCalendarExceptionOriginalScheduled: "2026-09-12", + googleCalendarMovedOriginalDates: ["2026-09-05"], + }); + const before = JSON.stringify(task); + const events = generateRecurringTaskInstances( + task, + new Date("2026-09-01T00:00:00Z"), + new Date("2026-09-21T00:00:00Z"), + plugin + ); + expect(getInstanceDates(events)).toEqual(["2026-09-11", "2026-09-19"]); + expect(JSON.stringify(task)).toBe(before); + }); + + it("keeps recorded history for moved dates when history is requested", () => { + const task = TaskFactory.createRecurringTask("DTSTART:20260905;FREQ=WEEKLY;BYDAY=SA", { + scheduled: "2026-09-19", + googleCalendarMovedOriginalDates: ["2026-09-12"], + complete_instances: ["2026-09-12"], + }); + const range = [new Date("2026-09-11T00:00:00Z"), new Date("2026-09-21T00:00:00Z")] as const; + expect(getInstanceDates(generateRecurringTaskInstances(task, ...range, plugin))).toEqual([ + "2026-09-12", + "2026-09-19", + ]); + expect( + getInstanceDates( + generateRecurringTaskInstances(task, ...range, plugin, { + showCompletedRecurringInstances: false, + }) + ) + ).toEqual(["2026-09-19"]); + }); + it("keeps completed and skipped recurring instances visible by default", () => { const task = TaskFactory.createRecurringTask("FREQ=DAILY;INTERVAL=1", { path: "tasks/recur.md", From f965d2450c77fec64173580aa3aa2f25a73d55c6 Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:31:21 +0100 Subject: [PATCH 10/22] fix(calendar): carry recurrence move metadata through Bases and refreshes --- src/bases/calendarDataSignature.ts | 2 ++ src/bases/helpers.ts | 6 ++++++ tests/unit/bases/calendarDataSignature.test.ts | 10 ++++++++++ ...curring-calendar-instance-visibility.test.ts | 17 +++++++++++++++++ 4 files changed, 35 insertions(+) diff --git a/src/bases/calendarDataSignature.ts b/src/bases/calendarDataSignature.ts index f1e3469e3..3ea7a1fa4 100644 --- a/src/bases/calendarDataSignature.ts +++ b/src/bases/calendarDataSignature.ts @@ -18,6 +18,8 @@ const CALENDAR_DATA_SIGNATURE_FIELDS: FieldMappingKey[] = [ "blockedBy", "icsEventId", "googleCalendarEventId", + "googleCalendarExceptionOriginalScheduled", + "googleCalendarMovedOriginalDates", "reminders", "sortOrder", ]; diff --git a/src/bases/helpers.ts b/src/bases/helpers.ts index 88b2ff3df..99fdfc47d 100644 --- a/src/bases/helpers.ts +++ b/src/bases/helpers.ts @@ -198,6 +198,8 @@ function createTaskInfoFromProperties( "icsEventId", "complete_instances", "skipped_instances", + "googleCalendarExceptionOriginalScheduled", + "googleCalendarMovedOriginalDates", "blockedBy", "blocking", "sortOrder", @@ -267,6 +269,10 @@ function createTaskInfoFromProperties( icsEventId: toStringArray(props.icsEventId), complete_instances: toStringArray(props.complete_instances), skipped_instances: toStringArray(props.skipped_instances), + googleCalendarExceptionOriginalScheduled: toOptionalString( + props.googleCalendarExceptionOriginalScheduled + ), + googleCalendarMovedOriginalDates: toStringArray(props.googleCalendarMovedOriginalDates), blockedBy: toDependencies(props.blockedBy), blocking: blockingTasks.length > 0 ? blockingTasks : undefined, isBlocked: isBlocked, diff --git a/tests/unit/bases/calendarDataSignature.test.ts b/tests/unit/bases/calendarDataSignature.test.ts index 1b9298394..1c8cddf9f 100644 --- a/tests/unit/bases/calendarDataSignature.test.ts +++ b/tests/unit/bases/calendarDataSignature.test.ts @@ -7,6 +7,16 @@ import { } from "../../../src/bases/calendarDataSignature"; describe("calendarDataSignature", () => { + it("invalidates the calendar when mapped occurrence-move markers change", () => { + const properties = buildCalendarDataSignaturePropertyIds({ + mapField: (field) => field === "googleCalendarExceptionOriginalScheduled" ? "originalOccurrence" : undefined, + showPropertyBasedEvents: false, + }); + const before = [{path: "task.md", properties: {originalOccurrence: "2026-09-12"}}]; + const after = [{path: "task.md", properties: {originalOccurrence: "2026-09-19"}}]; + expect(buildCalendarDataSignature(before, properties)).not.toEqual(buildCalendarDataSignature(after, properties)); + expect(properties).toContain("googleCalendarMovedOriginalDates"); + }); it("selects mapped core fields, visible properties, and property-event fields", () => { const propertyIds = buildCalendarDataSignaturePropertyIds({ mapField: (field) => (field === "scheduled" ? "planned" : undefined), diff --git a/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts b/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts index 7a5b66c4e..2d47d971f 100644 --- a/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts +++ b/tests/unit/issues/issue-1603-recurring-calendar-instance-visibility.test.ts @@ -14,6 +14,7 @@ import { } from "../../../src/bases/calendar-core"; import type TaskNotesPlugin from "../../../src/main"; import { TaskFactory } from "../../helpers/mock-factories"; +import { createTaskInfoFromBasesData } from "../../../src/bases/helpers"; function createPlugin(): TaskNotesPlugin { return { @@ -52,6 +53,22 @@ describe("Issue #1603: recurring calendar instance visibility", () => { expect(JSON.stringify(task)).toBe(before); }); + it("preserves move markers through Bases conversion before the task cache is warm", () => { + const task = createTaskInfoFromBasesData({ + path: "tasks/moved.md", + properties: { + recurrence: "DTSTART:20260905;FREQ=WEEKLY;BYDAY=SA", + scheduled: "2026-09-11", + googleCalendarExceptionOriginalScheduled: "2026-09-12", + googleCalendarMovedOriginalDates: ["2026-09-05"], + }, + }); + expect(task).not.toBeNull(); + expect(getInstanceDates(generateRecurringTaskInstances( + task!, new Date("2026-09-01T00:00:00Z"), new Date("2026-09-21T00:00:00Z"), plugin + ))).toEqual(["2026-09-11", "2026-09-19"]); + }); + it("keeps recorded history for moved dates when history is requested", () => { const task = TaskFactory.createRecurringTask("DTSTART:20260905;FREQ=WEEKLY;BYDAY=SA", { scheduled: "2026-09-19", From f4e3f174997ae2b2685fa811152c0febfa071c04 Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:47:00 +0100 Subject: [PATCH 11/22] fix(google-calendar): resolve the primary calendar alias in every lookup A Google calendar enabled as `primary` is fetched under that alias, so its events carry `primary` as their calendar id while the calendar list reports the account's real id. Event cards already reconcile the two; three other lookups compared ids directly and missed. The calendar's own color was replaced by the default Google blue, its per-calendar visibility toggle matched nothing and so never hid anything, and mini calendar entries and event-linked notes fell back to the generic provider name. Resolve the alias through one shared helper, and register the account's own calendar under both keys when building visibility toggles. Event and subscription ids are unchanged, so notes already linked to alias-fetched events still match. --- docs/releases/unreleased.md | 5 + src/bases/CalendarView.ts | 8 +- src/bases/MiniCalendarView.ts | 15 ++- src/bases/calendarExternalEvents.ts | 17 +++ src/services/CalendarProvider.ts | 23 ++++ src/services/GoogleCalendarService.ts | 18 ++- src/services/ICSNoteService.ts | 6 +- src/ui/ICSCard.ts | 9 +- .../unit/bases/calendarExternalEvents.test.ts | 30 +++++ ...ssue-google-primary-calendar-alias.test.ts | 112 ++++++++++++++++++ 10 files changed, 221 insertions(+), 22 deletions(-) create mode 100644 tests/unit/issues/issue-google-primary-calendar-alias.test.ts diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index 21617927f..c84ab3e30 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -34,6 +34,11 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l ## Fixed +- (#2309) Fixed Google calendars enabled through the `primary` alias falling back to the + default blue instead of their own calendar color. The alias now also resolves for the + calendar's visibility toggle, which previously matched nothing, and for the calendar name + shown on mini calendar entries and event-linked notes. + - (#1849) Fixed context menus stacking on top of each other. Only one menu stays open at a time, and clicking the same indicator again closes its menu. This previously applied to date fields only, and now covers priority, status, recurrence, reminders, task, ICS event, and batch menus. - Thanks to @3zra47 for reporting and @YBKF for the contribution. diff --git a/src/bases/CalendarView.ts b/src/bases/CalendarView.ts index 9ca0bf0b6..fdc9d7ff4 100644 --- a/src/bases/CalendarView.ts +++ b/src/bases/CalendarView.ts @@ -79,7 +79,7 @@ import { getCalendarConfigValue as getCalendarConfigValueFromSnapshot, } from "./calendarConfigSnapshot"; import { buildCalendarPropertyEvent } from "./calendarPropertyEvents"; -import { buildExternalCalendarEvents } from "./calendarExternalEvents"; +import { buildExternalCalendarEvents, setProviderCalendarToggle } from "./calendarExternalEvents"; import { decorateCalendarIcsEventElement, getCalendarRelatedNoteTooltip, @@ -918,7 +918,11 @@ export class CalendarView extends BasesViewBase { const calendars = this.plugin.googleCalendarService.getAvailableCalendars(); for (const cal of calendars) { const key = `showGoogleCalendar_${cal.id}`; - this.googleCalendarToggles.set(cal.id, this.getConfigOption(key, true)); + setProviderCalendarToggle( + this.googleCalendarToggles, + cal, + this.getConfigOption(key, true) + ); } } diff --git a/src/bases/MiniCalendarView.ts b/src/bases/MiniCalendarView.ts index 18bb7e7e3..f27a348e4 100644 --- a/src/bases/MiniCalendarView.ts +++ b/src/bases/MiniCalendarView.ts @@ -36,6 +36,8 @@ import { import { ICSEventInfoModal } from "../modals/ICSEventInfoModal"; import { createTaskNotesLogger } from "../utils/tasknotesLogger"; import { createElementInDocument } from "../utils/documentDom"; +import { findProviderCalendar } from "../services/CalendarProvider"; +import { setProviderCalendarToggle } from "./calendarExternalEvents"; const tasknotesLogger = createTaskNotesLogger({ tag: "Bases/MiniCalendarView" }); @@ -209,8 +211,9 @@ export class MiniCalendarView extends BasesViewBase { if (this.plugin.googleCalendarService) { for (const calendar of this.plugin.googleCalendarService.getAvailableCalendars()) { - this.googleCalendarToggles.set( - calendar.id, + setProviderCalendarToggle( + this.googleCalendarToggles, + calendar, getToggleValue(`showGoogleCalendar_${calendar.id}`) ); } @@ -447,17 +450,13 @@ export class MiniCalendarView extends BasesViewBase { return; } - const calendars = new Map( - this.plugin.googleCalendarService - .getAvailableCalendars() - .map((calendar) => [calendar.id, calendar]) - ); + const calendars = this.plugin.googleCalendarService.getAvailableCalendars(); for (const icsEvent of this.plugin.googleCalendarService.getAllEvents()) { const calendarId = icsEvent.subscriptionId.replace("google-", ""); if (this.googleCalendarToggles.get(calendarId) === false) continue; - const calendar = calendars.get(calendarId); + const calendar = findProviderCalendar(calendars, calendarId); this.indexExternalEvent( icsEvent, calendar?.summary || "Google Calendar", diff --git a/src/bases/calendarExternalEvents.ts b/src/bases/calendarExternalEvents.ts index 45b56c9c0..091206c99 100644 --- a/src/bases/calendarExternalEvents.ts +++ b/src/bases/calendarExternalEvents.ts @@ -2,6 +2,7 @@ import type { EventInput } from "@fullcalendar/core"; import type TaskNotesPlugin from "../main"; import type { ICSEvent } from "../types"; import { createICSEvent } from "./calendar-core"; +import { PRIMARY_CALENDAR_ALIAS, ProviderCalendar } from "../services/CalendarProvider"; export type ExternalCalendarProvider = "ics" | "google" | "microsoft"; @@ -35,6 +36,22 @@ export function getExternalCalendarToggleId( return event.subscriptionId; } +/** + * Registers a provider calendar's visibility toggle. Calendars fetched under the + * primary alias carry that alias as their calendar id, so the account's own + * calendar has to answer to both keys for its toggle to take effect. + */ +export function setProviderCalendarToggle( + toggles: Map, + calendar: Pick, + visible: boolean +): void { + toggles.set(calendar.id, visible); + if (calendar.primary) { + toggles.set(PRIMARY_CALENDAR_ALIAS, visible); + } +} + export function shouldIncludeExternalCalendarEvent( event: Pick, provider: ExternalCalendarProvider, diff --git a/src/services/CalendarProvider.ts b/src/services/CalendarProvider.ts index 67847f2a1..b32922a7a 100644 --- a/src/services/CalendarProvider.ts +++ b/src/services/CalendarProvider.ts @@ -22,6 +22,29 @@ export interface ProviderCalendar { primary?: boolean; } +/** + * Google accepts `primary` as an alias for the signed-in account's own calendar. + * A calendar configured that way is fetched under the alias, so its events carry + * `primary` as their calendar id while the provider's calendar list reports the + * account's real calendar id. + */ +export const PRIMARY_CALENDAR_ALIAS = "primary"; + +/** + * Finds a provider calendar by id, resolving the primary alias to the account's + * own calendar so alias-configured events still match their calendar metadata. + */ +export function findProviderCalendar>( + calendars: readonly T[], + calendarId: string +): T | undefined { + return calendars.find( + (candidate) => + candidate.id === calendarId || + (calendarId === PRIMARY_CALENDAR_ALIAS && candidate.primary === true) + ); +} + /** * Event date/time configuration * Supports both all-day events (date) and timed events (dateTime + timeZone) diff --git a/src/services/GoogleCalendarService.ts b/src/services/GoogleCalendarService.ts index 8d85747d6..f8a34b2d7 100644 --- a/src/services/GoogleCalendarService.ts +++ b/src/services/GoogleCalendarService.ts @@ -13,7 +13,7 @@ import { TokenExpiredError, } from "./errors"; import { validateCalendarId, validateEventId, validateRequired } from "./validation"; -import { CalendarProvider, ProviderCalendar } from "./CalendarProvider"; +import { CalendarProvider, findProviderCalendar, ProviderCalendar } from "./CalendarProvider"; import { createTaskNotesLogger } from "../utils/tasknotesLogger"; import { publishUserNotice } from "../core/userNotices"; import { normalizeCalendarDescription } from "../utils/calendarDescription"; @@ -454,6 +454,20 @@ export class GoogleCalendarService extends CalendarProvider { } } + /** + * Resolves a calendar's color, including for calendars fetched under the + * primary alias, whose colors are cached under the account's real calendar id. + */ + private getCalendarColor(calendarId: string): string | undefined { + const directColor = this.calendarColors.get(calendarId); + if (directColor) { + return directColor; + } + + const calendar = findProviderCalendar(this.availableCalendars, calendarId); + return calendar ? this.calendarColors.get(calendar.id) : undefined; + } + /** * Converts a Google Calendar event to TaskNotes ICSEvent format */ @@ -492,7 +506,7 @@ export class GoogleCalendarService extends CalendarProvider { // Priority 2: Calendar-level color (from calendar metadata) if (!color) { - color = this.calendarColors.get(calendarId); + color = this.getCalendarColor(calendarId); } // Priority 3: Default Google Calendar blue diff --git a/src/services/ICSNoteService.ts b/src/services/ICSNoteService.ts index 8a6776d09..cd76f9330 100644 --- a/src/services/ICSNoteService.ts +++ b/src/services/ICSNoteService.ts @@ -15,6 +15,7 @@ import type { InterpolationValues, TranslationKey } from "../i18n"; import { createTaskNotesLogger } from "../utils/tasknotesLogger"; import { publishUserNotice } from "../core/userNotices"; import { processVaultFrontMatter } from "./VaultMutationService"; +import { findProviderCalendar } from "./CalendarProvider"; const tasknotesLogger = createTaskNotesLogger({ tag: "Services/ICSNoteService" }); @@ -60,10 +61,9 @@ export class ICSNoteService { .find((event) => event.id === trimmedEventId); if (googleEvent) { const calendarId = googleEvent.subscriptionId.replace("google-", ""); + const calendars = this.plugin.googleCalendarService?.getAvailableCalendars() ?? []; const subscriptionName = - this.plugin.googleCalendarService - ?.getAvailableCalendars() - .find((calendar) => calendar.id === calendarId)?.summary || "Google Calendar"; + findProviderCalendar(calendars, calendarId)?.summary || "Google Calendar"; return { event: googleEvent, subscriptionName }; } diff --git a/src/ui/ICSCard.ts b/src/ui/ICSCard.ts index 16a5c63dd..daa63b3fb 100644 --- a/src/ui/ICSCard.ts +++ b/src/ui/ICSCard.ts @@ -4,6 +4,7 @@ import { ICSEvent, ICSSubscription } from "../types"; import { ICSEventContextMenu } from "../components/ICSEventContextMenu"; import { formatTime } from "../utils/dateUtils"; import { ICSEventInfoModal } from "../modals/ICSEventInfoModal"; +import { findProviderCalendar } from "../services/CalendarProvider"; export interface ICSCardOptions { showDate: boolean; @@ -58,13 +59,7 @@ function getEventSourceName( const provider = plugin.calendarProviderRegistry?.findProviderForEvent(icsEvent); if (provider) { const { calendarId } = provider.extractEventIds(icsEvent); - const calendar = provider - .getAvailableCalendars() - .find( - (candidate) => - candidate.id === calendarId || - (calendarId === "primary" && candidate.primary === true) - ); + const calendar = findProviderCalendar(provider.getAvailableCalendars(), calendarId); return calendar?.summary || provider.providerName; } diff --git a/tests/unit/bases/calendarExternalEvents.test.ts b/tests/unit/bases/calendarExternalEvents.test.ts index 187204a63..52d0fb15f 100644 --- a/tests/unit/bases/calendarExternalEvents.test.ts +++ b/tests/unit/bases/calendarExternalEvents.test.ts @@ -1,6 +1,7 @@ import { buildExternalCalendarEvents, getExternalCalendarToggleId, + setProviderCalendarToggle, shouldIncludeExternalCalendarEvent, } from "../../../src/bases/calendarExternalEvents"; import type TaskNotesPlugin from "../../../src/main"; @@ -56,6 +57,35 @@ describe("calendar external event assembly", () => { ).toBe(true); }); + it("applies a primary calendar's toggle to events fetched under the alias", () => { + const toggles = new Map(); + + setProviderCalendarToggle(toggles, { id: "person@example.com", primary: true }, false); + setProviderCalendarToggle(toggles, { id: "team@example.com" }, true); + + expect( + shouldIncludeExternalCalendarEvent( + createEvent({ subscriptionId: "google-primary" }), + "google", + toggles + ) + ).toBe(false); + expect( + shouldIncludeExternalCalendarEvent( + createEvent({ subscriptionId: "google-person@example.com" }), + "google", + toggles + ) + ).toBe(false); + expect( + shouldIncludeExternalCalendarEvent( + createEvent({ subscriptionId: "google-team@example.com" }), + "google", + toggles + ) + ).toBe(true); + }); + it("builds provider events with related note counts", () => { const sourceEvents = [ createEvent({ id: "included", subscriptionId: "microsoft-work" }), diff --git a/tests/unit/issues/issue-google-primary-calendar-alias.test.ts b/tests/unit/issues/issue-google-primary-calendar-alias.test.ts new file mode 100644 index 000000000..d54990364 --- /dev/null +++ b/tests/unit/issues/issue-google-primary-calendar-alias.test.ts @@ -0,0 +1,112 @@ +import { requestUrl } from "obsidian"; +import type TaskNotesPlugin from "../../../src/main"; +import { GoogleCalendarService } from "../../../src/services/GoogleCalendarService"; +import { findProviderCalendar } from "../../../src/services/CalendarProvider"; +import type { OAuthService } from "../../../src/services/OAuthService"; + +jest.mock("obsidian", () => ({ + Platform: { isDesktopApp: true }, + requestUrl: jest.fn(), +})); + +const PRIMARY_CALENDAR = { + id: "person@example.com", + summary: "Personal", + primary: true, + backgroundColor: "#16a765", +}; + +function createPlugin(enabledGoogleCalendars: string[]): TaskNotesPlugin { + return { + settings: { + enabledGoogleCalendars, + googleCalendarSyncTokens: {}, + }, + saveSettingsDataOnly: jest.fn().mockResolvedValue(undefined), + } as unknown as TaskNotesPlugin; +} + +function createOAuthService(): OAuthService { + return { + isConnected: jest.fn().mockResolvedValue(true), + getValidToken: jest.fn().mockResolvedValue("access-token"), + } as unknown as OAuthService; +} + +function mockCalendarResponses(): void { + const requestMock = requestUrl as jest.MockedFunction; + requestMock.mockReset(); + requestMock + .mockResolvedValueOnce({ + status: 200, + json: { items: [PRIMARY_CALENDAR] }, + text: "", + arrayBuffer: new ArrayBuffer(0), + headers: {}, + }) + .mockResolvedValueOnce({ + status: 200, + json: { + items: [ + { + id: "event-1", + summary: "Standup", + start: { date: "2026-09-12" }, + end: { date: "2026-09-13" }, + }, + ], + nextSyncToken: "next-sync-token", + }, + text: "", + arrayBuffer: new ArrayBuffer(0), + headers: {}, + }); +} + +describe("Google primary calendar alias", () => { + it("colors events from a calendar enabled through the primary alias", async () => { + mockCalendarResponses(); + const service = new GoogleCalendarService(createPlugin(["primary"]), createOAuthService()); + + await service.refreshAllCalendars(); + + const [event] = service.getAllEvents(); + expect(event.color).toBe("#16a765"); + }); + + it("keeps alias-configured event ids stable", async () => { + mockCalendarResponses(); + const service = new GoogleCalendarService(createPlugin(["primary"]), createOAuthService()); + + await service.refreshAllCalendars(); + + const [event] = service.getAllEvents(); + expect(event.subscriptionId).toBe("google-primary"); + expect(event.id).toBe("google-primary-event-1"); + }); + + it("still colors events fetched under a calendar's own id", async () => { + mockCalendarResponses(); + const service = new GoogleCalendarService( + createPlugin(["person@example.com"]), + createOAuthService() + ); + + await service.refreshAllCalendars(); + + const [event] = service.getAllEvents(); + expect(event.color).toBe("#16a765"); + }); + + it("resolves the primary alias to the account's own calendar", () => { + const calendars = [{ id: "team@example.com", summary: "Work" }, PRIMARY_CALENDAR]; + + expect(findProviderCalendar(calendars, "primary")?.summary).toBe("Personal"); + expect(findProviderCalendar(calendars, "team@example.com")?.summary).toBe("Work"); + expect(findProviderCalendar(calendars, "missing@example.com")).toBeUndefined(); + }); + + it("has no primary calendar to resolve when none is marked", () => { + expect(findProviderCalendar([{ id: "team@example.com" }], "primary")).toBeUndefined(); + }); +}); From 4ae127f231752ebc5259ffe1d7fe3910c9352ade Mon Sep 17 00:00:00 2001 From: martin-forge <228563004+martin-forge@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:52:47 +0100 Subject: [PATCH 12/22] security: harden loopback OAuth callback and require token for API listeners --- docs/releases/unreleased.md | 4 + src/api/httpTypes.ts | 1 + src/services/HTTPAPIService.ts | 15 +- src/services/OAuthService.ts | 168 +++++++----------- tests/services/OAuthService.callback.test.ts | 35 ++++ .../issue-1923-http-api-loopback-cors.test.ts | 15 +- 6 files changed, 129 insertions(+), 109 deletions(-) create mode 100644 tests/services/OAuthService.callback.test.ts diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index 21617927f..199df8bff 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -47,3 +47,7 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l - (#2303) Removed the extra space after inline task links on desktop while retaining the task menu on touch devices. - Thanks to @nelsonlove for the fix. + +## Security + +- Require an API token before starting local API/MCP listeners, and validate one-use OAuth callbacks on an OS-assigned loopback port without reflecting callback text into HTML. diff --git a/src/api/httpTypes.ts b/src/api/httpTypes.ts index 9774ab23b..b631ac8d6 100644 --- a/src/api/httpTypes.ts +++ b/src/api/httpTypes.ts @@ -16,6 +16,7 @@ export interface HTTPResponseLike { } export interface HTTPServerLike { + address?(): { port: number } | string | null; listening?: boolean; listen(port: number, callback?: () => void): void; listen(port: number, hostname: string, callback?: () => void): void; diff --git a/src/services/HTTPAPIService.ts b/src/services/HTTPAPIService.ts index 7d8252072..d0f565638 100644 --- a/src/services/HTTPAPIService.ts +++ b/src/services/HTTPAPIService.ts @@ -188,9 +188,9 @@ export class HTTPAPIService implements IWebhookNotifier { private authenticate(req: HTTPRequestLike): boolean { const authToken = this.plugin.settings.apiAuthToken; - // Skip auth if no token is configured + // A missing credential never opens an unauthenticated listener. if (!authToken) { - return true; + return false; } const authHeader = req.headers.authorization; @@ -292,6 +292,17 @@ export class HTTPAPIService implements IWebhookNotifier { } async start(): Promise { + if (!Platform.isDesktop || !Platform.isDesktopApp) + throw new Error("The HTTP API is only available in the desktop app."); + if (!this.plugin.settings.apiAuthToken) { + this.plugin.settings.apiAuthToken = btoa( + String.fromCharCode(...crypto.getRandomValues(new Uint8Array(32))) + ) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/g, ""); + await this.plugin.saveSettings(); + } return new Promise((resolve, reject) => { if (!Platform.isDesktop || !Platform.isDesktopApp) { reject(new Error("The HTTP API is only available in the desktop app.")); diff --git a/src/services/OAuthService.ts b/src/services/OAuthService.ts index 9fed0b255..478aace5c 100644 --- a/src/services/OAuthService.ts +++ b/src/services/OAuthService.ts @@ -157,12 +157,7 @@ export class OAuthService { const codeChallenge = await this.generateCodeChallenge(codeVerifier); const state = this.generateState(); - // Find available port - const port = await this.findAvailablePort( - OAUTH_CONSTANTS.CALLBACK_PORT_START, - OAUTH_CONSTANTS.CALLBACK_PORT_END - ); - await this.startCallbackServer(port); + const port = await this.startCallbackServer(0); // Update redirect URI for this session const originalRedirectUri = config.redirectUri; @@ -185,11 +180,11 @@ export class OAuthService { `Opening browser for ${provider} authorization...` ); - // Open browser to authorization URL + // Register the resolver before the browser can return a fast callback. + const callback = this.waitForCallback(state, 300000); + void callback.catch(() => undefined); await this.openAuthorizationUrl(authUrl); - - // Wait for callback with timeout - const code = await this.waitForCallback(state, 300000); // 5 minute timeout + const code = await callback; // Exchange code for tokens const tokens = await this.exchangeCodeForTokens(config, code, codeVerifier); @@ -202,6 +197,10 @@ export class OAuthService { `Successfully connected to ${provider} Calendar!` ); } finally { + // Clear a pending resolver even if opening the browser failed. + const pending = this.pendingOAuthState.get(state); + this.pendingOAuthState.delete(state); + pending?.reject(new Error("OAuth flow ended before authorization")); // Restore original redirect URI config.redirectUri = originalRedirectUri; } @@ -231,43 +230,19 @@ export class OAuthService { return; } } catch (error) { - tasknotesLogger.warn("Failed to open OAuth URL in system browser; falling back to window.open.", { - category: "provider", - operation: "oauth-open-external", - error, - }); + tasknotesLogger.warn( + "Failed to open OAuth URL in system browser; falling back to window.open.", + { + category: "provider", + operation: "oauth-open-external", + error, + } + ); } window.open(authUrl, "_blank"); } - /** - * Finds an available port in the given range - */ - private async findAvailablePort(startPort: number, endPort: number): Promise { - const http = ensureHttpModule(); - - for (let port = startPort; port <= endPort; port++) { - try { - await new Promise((resolve, reject) => { - const server = http.createServer(); - server.once("error", reject); - server.once("listening", () => { - server.close(); - resolve(); - }); - server.listen(port, "127.0.0.1"); - }); - return port; - } catch { - // Port in use, try next one - continue; - } - } - - throw new Error(`No available ports found between ${startPort} and ${endPort}`); - } - /** * Generates a random code verifier for PKCE */ @@ -330,10 +305,10 @@ export class OAuthService { /** * Starts a temporary HTTP server to receive the OAuth callback */ - private async startCallbackServer(port: number): Promise { + private async startCallbackServer(port: number): Promise { return new Promise((resolve, reject) => { if (this.callbackServer) { - resolve(); // Already running + reject(new Error("An OAuth callback is already pending")); return; } @@ -363,7 +338,12 @@ export class OAuthService { }); this.callbackServer.listen(port, "127.0.0.1", () => { - resolve(); + const address = this.callbackServer?.address?.(); + if (!address || typeof address === "string") { + reject(new Error("OAuth listener did not expose its bound port")); + return; + } + resolve(address.port); }); }); } @@ -389,70 +369,48 @@ export class OAuthService { * Handles incoming HTTP requests to the callback server */ private handleCallback(req: HTTPRequestLike, res: HTTPResponseLike): void { - const hostHeader = req.headers.host; - const host = Array.isArray(hostHeader) ? hostHeader[0] : (hostHeader ?? "localhost"); - const url = new URL(req.url || "", `http://${host}`); - const code = url.searchParams.get("code"); - const state = url.searchParams.get("state"); - const error = url.searchParams.get("error"); - - // Send response to browser - res.writeHead(200, { "Content-Type": "text/html" }); - - if (error) { - res.end(` - - - OAuth Error - -

Authorization Failed

-

Error: ${error}

-

You can close this window.

- - - `); - - const pending = state ? this.pendingOAuthState.get(state) : null; - if (pending && state) { - pending.reject(new Error(`OAuth error: ${error}`)); - this.pendingOAuthState.delete(state); - } + const headers = { + "Content-Type": "text/html; charset=utf-8", + "Content-Security-Policy": + "default-src 'none'; frame-ancestors 'none'; base-uri 'none'", + "X-Content-Type-Options": "nosniff", + "Cache-Control": "no-store", + }; + let url: URL; + try { + url = new URL(req.url || "/", "http://127.0.0.1"); + } catch { + res.writeHead(400, headers); + res.end("Invalid callback"); return; } - - if (!code || !state) { - res.end(` - - - OAuth Error - -

Invalid Callback

-

Missing required parameters.

-

You can close this window.

- - - `); + const state = url.searchParams.get("state"); + const pending = state ? this.pendingOAuthState.get(state) : undefined; + const code = url.searchParams.get("code"); + const error = url.searchParams.get("error"); + if ( + req.method !== "GET" || + url.pathname !== "/" || + !state || + !pending || + (!code && !error) + ) { + res.writeHead(400, headers); + res.end( + "Invalid callback

No pending authorization matches this callback.

" + ); return; } - - res.end(` - - - OAuth Success - -

Authorization Successful!

-

You can close this window and return to Obsidian.

- - - - `); - - // Resolve the pending promise - const pending = this.pendingOAuthState.get(state); - if (pending) { - pending.resolve(code); - this.pendingOAuthState.delete(state); - } + // Consume before responding: a replay cannot resolve the pending flow. + this.pendingOAuthState.delete(state); + res.writeHead(error ? 400 : 200, headers); + res.end( + error + ? "Authorization failed

Authorization was not completed. Return to Obsidian.

" + : "Authorization complete

You can close this window and return to Obsidian.

" + ); + if (error) pending.reject(new Error("OAuth authorization failed")); + else pending.resolve(code as string); } /** diff --git a/tests/services/OAuthService.callback.test.ts b/tests/services/OAuthService.callback.test.ts new file mode 100644 index 000000000..e6825e735 --- /dev/null +++ b/tests/services/OAuthService.callback.test.ts @@ -0,0 +1,35 @@ +import { OAuthService } from "../../src/services/OAuthService"; +import { OAuthSecretStore } from "../../src/services/OAuthSecretStore"; + +describe("OAuth callback validation", () => { + it("validates and consumes OAuth state before writing a static response", () => { + const store = new OAuthSecretStore({ getSecret: () => null, setSecret: () => {} }); + const service: any = new OAuthService({} as any, store); + const response: any = { writeHead: jest.fn(), end: jest.fn() }; + service.handleCallback( + { method: "GET", url: "/?error=%3Cscript%3E&state=unknown", headers: {} }, + response + ); + expect(response.writeHead.mock.calls[0][0]).toBe(400); + expect(response.end.mock.calls[0][0]).not.toContain("