diff --git a/docs/docs/Advanced/CLI.md b/docs/docs/Advanced/CLI.md index b0442119..15bec83b 100644 --- a/docs/docs/Advanced/CLI.md +++ b/docs/docs/Advanced/CLI.md @@ -39,6 +39,51 @@ Check which inputs are still missing before a non-interactive run. obsidian vault=dev quickadd:check choice="Daily log" ``` +The check command is flow-aware. For macros, it follows nested Template and Capture choices, reports missing inputs for the whole flow, and includes diagnostics in the JSON response: + +- `diagnostics`: runtime, nested-choice, and integration capability notes. +- `flow.choices`: choices included in the check, with path and depth. +- `missingFlags`: CLI flags you can pass to satisfy unresolved inputs. + +Example response shape: + +```json +{ + "ok": false, + "command": "quickadd:check", + "requiredInputCount": 1, + "missingInputCount": 1, + "missingFlags": ["value-project="], + "diagnostics": [ + { + "severity": "error", + "code": "missing-required-inputs", + "source": "runtime", + "message": "1 required input(s) are missing for this flow." + } + ], + "flow": { + "choiceCount": 2, + "choices": [ + { + "id": "macro-choice-id", + "name": "Daily log", + "type": "Macro", + "path": "Daily log", + "depth": 0 + }, + { + "id": "capture-choice-id", + "name": "Capture project", + "type": "Capture", + "path": "Daily log / Capture project", + "depth": 1 + } + ] + } +} +``` + ## Passing variables QuickAdd CLI supports three variable patterns: diff --git a/docs/docs/Advanced/onePageInputs.md b/docs/docs/Advanced/onePageInputs.md index 27bfb76c..bd173bdb 100644 --- a/docs/docs/Advanced/onePageInputs.md +++ b/docs/docs/Advanced/onePageInputs.md @@ -21,6 +21,7 @@ This feature is currently in Beta. - `{{VALUE|type:multiline}}` and `{{VALUE:name|type:multiline}}` render as textareas in the one-page modal. - Capture target file when capturing to a folder or tag. - Script-declared inputs (from user scripts inside macros), if provided. +- Nested Template/Capture choices inside macros are included in flow-aware CLI checks, so missing inputs can be reported before a non-interactive macro run. ## Date UX - Date fields support natural language (e.g., “today”, “next friday”). @@ -178,6 +179,16 @@ Behavior: --- +## CLI diagnostics + +`quickadd:check` uses the same requirement parsing as one-page inputs, then adds flow diagnostics for macros and nested choices. The JSON response includes: + +- `missing` and `missingFlags` for unresolved inputs. +- `diagnostics` for flow/runtime information, missing nested choices, and integration capability notes. +- `flow.choices`, showing which choices were included in the flow check. + +This is read-only: it does not execute choices or run user scripts beyond the existing static metadata inspection used for declared `quickadd.inputs`. + ## Notes - Macro support is best-effort: user scripts can declare inputs via `quickadd.inputs`. - Preflight may import user script modules to statically read `quickadd.inputs`. This can execute module top-level code. diff --git a/docs/docs/Choices/CaptureChoice.md b/docs/docs/Choices/CaptureChoice.md index cde8d89d..bea89d6d 100644 --- a/docs/docs/Choices/CaptureChoice.md +++ b/docs/docs/Choices/CaptureChoice.md @@ -45,6 +45,17 @@ If you have a tag called `#people`, and you type `#people` in the _Capture To_ f ## Capture Options +Capture runs participate in the shared QuickAdd flow runtime. When a Capture choice is launched from a macro or nested choice, it reuses the same variable map and origin tab as the parent flow. + +QuickAdd preserves capture write ordering to avoid data loss: + +- Existing-file captures still use the merge guard before modifying the file. +- Missing insert-after targets abort before the file is modified, unless you enabled creating the missing line. +- Structured YAML/template-property post-processing runs only after a successful write. +- Creating a file from a template suppresses Templater's on-create trigger and renders the whole file once. +- Creating a file without a template waits for Templater's on-create trigger only when that trigger is enabled. +- Whole-file Templater rendering after capture remains opt-in via the Templater after-capture setting. + - _Create file if it doesn't exist_ will do as the name implies - you can also create the file from a template, if you specify the template (the input box will appear below the setting). - _Task_ will format your captured text as a task. - _Use editor selection as default value_ controls whether the current editor selection is used as `{{VALUE}}`. Choose **Follow global setting**, **Use selection**, or **Ignore selection** (global default lives in Settings > Input). This does not affect `{{SELECTED}}`. diff --git a/docs/docs/Choices/MacroChoice.md b/docs/docs/Choices/MacroChoice.md index a6918c8e..a42aa323 100644 --- a/docs/docs/Choices/MacroChoice.md +++ b/docs/docs/Choices/MacroChoice.md @@ -52,6 +52,7 @@ In the Macro Builder, you can add different types of commands: 4. **Nested Choice** - Execute another QuickAdd choice - Reuse existing templates, captures, or other macros - Create modular workflows + - Nested choices share the parent macro's runtime context, variables, and origin tab 5. **Wait** - Add delays between commands - Useful when commands need time to complete - Specified in milliseconds @@ -249,7 +250,13 @@ module.exports = async (params) => { ## Variables and Data Flow -Variables allow you to pass data between commands in a macro: +Variables allow you to pass data between commands in a macro. A macro run now uses one shared flow context from the first command through all nested choices. That means: + +- Variables set by one command are visible to later commands. +- Nested Template, Capture, and Macro choices reuse the same variable map. +- Nested choices reuse the macro's original leaf/tab for file-opening behavior, so later file opens stay anchored to the run that started the macro. +- If a nested choice aborts, the macro stops and later commands are skipped. + ### Setting Variables in Scripts @@ -508,6 +515,7 @@ Macros automatically stop execution in the following situations: - A message is logged explaining why the macro stopped - For user cancellations and explicit aborts, no error dialog is shown - For script errors, the full error with stack trace is preserved for debugging +- Nested choice aborts propagate back to the parent macro and stop the remaining macro commands ## Best Practices diff --git a/docs/docs/QuickAddAPI.md b/docs/docs/QuickAddAPI.md index d84d85d8..ca7a1849 100644 --- a/docs/docs/QuickAddAPI.md +++ b/docs/docs/QuickAddAPI.md @@ -381,6 +381,8 @@ console.log("Enabled features:", features); ### `executeChoice(choiceName: string, variables?: {[key: string]: any}): Promise` Executes another QuickAdd choice programmatically. +When called from a macro or script that already has variables, QuickAdd snapshots the current variable map, applies the variables you pass for the child choice, runs that choice, and then restores the snapshot. This prevents `executeChoice()` from clearing unrelated variables owned by the surrounding flow. + **Parameters:** - `choiceName`: Name of the choice to execute - `variables`: (Optional) Variables to pass to the choice diff --git a/src/IChoiceExecutor.ts b/src/IChoiceExecutor.ts index 483a1078..ddf47c84 100644 --- a/src/IChoiceExecutor.ts +++ b/src/IChoiceExecutor.ts @@ -1,9 +1,14 @@ import type IChoice from "./types/choices/IChoice"; import type { MacroAbortError } from "./errors/MacroAbortError"; +import type { + ChoiceExecutionContext, + ChoiceExecutionResult, +} from "./engine/runtime"; export interface IChoiceExecutor { - execute(choice: IChoice): Promise; + execute(choice: IChoice): Promise; variables: Map; + getExecutionContext?(): ChoiceExecutionContext | null; /** * Records that the most recent choice execution aborted so orchestrators can react. * Engines that handle cancellations without throwing should call this immediately after diff --git a/src/choiceExecutor.runtime.test.ts b/src/choiceExecutor.runtime.test.ts new file mode 100644 index 00000000..93aa7c0d --- /dev/null +++ b/src/choiceExecutor.runtime.test.ts @@ -0,0 +1,307 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type IChoice from "./types/choices/IChoice"; +import type IMacroChoice from "./types/choices/IMacroChoice"; +import type { IMacro } from "./types/macros/IMacro"; +import { CommandType } from "./types/macros/CommandType"; +import { MacroAbortError } from "./errors/MacroAbortError"; +import { createChoiceExecutionResult } from "./engine/runtime"; + +const { + mockGetOpenFileOriginLeaf, + mockTemplateConstructor, + mockMacroConstructor, + mockMacroRunResult, + mockChoiceSuggesterOpen, +} = vi.hoisted(() => ({ + mockGetOpenFileOriginLeaf: vi.fn(), + mockTemplateConstructor: vi.fn(), + mockMacroConstructor: vi.fn(), + mockMacroRunResult: vi.fn(), + mockChoiceSuggesterOpen: vi.fn(), +})); + +vi.mock("./utilityObsidian", () => ({ + getOpenFileOriginLeaf: mockGetOpenFileOriginLeaf, +})); + +vi.mock("./preflight/runOnePagePreflight", () => ({ + runOnePagePreflight: vi.fn(), +})); + +vi.mock("./settingsStore", () => ({ + settingsStore: { + getState: () => ({ onePageInputEnabled: false }), + }, +})); + +vi.mock("./gui/suggesters/choiceSuggester", () => ({ + __esModule: true, + default: { + Open: mockChoiceSuggesterOpen, + }, +})); + +vi.mock("./engine/TemplateChoiceEngine", () => ({ + TemplateChoiceEngine: class TemplateChoiceEngineMock { + constructor(...args: any[]) { + mockTemplateConstructor({ + args, + context: args[3]?.getExecutionContext?.(), + originLeaf: args[4], + }); + } + + async run() { + return undefined; + } + }, +})); + +vi.mock("./engine/CaptureChoiceEngine", () => ({ + CaptureChoiceEngine: class CaptureChoiceEngineMock { + async run() { + return undefined; + } + }, +})); + +vi.mock("./engine/MacroChoiceEngine", () => ({ + MacroChoiceEngine: class MacroChoiceEngineMock { + params = { variables: {} }; + + constructor( + _app: unknown, + _plugin: unknown, + private readonly choice: IMacroChoice, + private readonly choiceExecutor: { + execute: (choice: IChoice) => Promise; + getExecutionContext?: () => unknown; + }, + _variables: Map, + _preloaded?: unknown, + _promptLabel?: unknown, + private readonly originLeaf?: unknown, + ) { + mockMacroConstructor({ + choice, + context: this.choiceExecutor.getExecutionContext?.(), + originLeaf, + }); + } + + async run() { + const overrideResult = mockMacroRunResult(); + if (overrideResult) return overrideResult; + + const firstCommand = this.choice.macro?.commands?.[0] as + | { type: string; choice?: IChoice } + | undefined; + const nestedChoice = firstCommand?.type === "NestedChoice" + ? firstCommand.choice + : undefined; + if (nestedChoice) { + await this.choiceExecutor.execute(nestedChoice); + } + return createChoiceExecutionResult({ + status: "success", + choiceId: this.choice.id, + }); + } + }, +})); + +import { ChoiceExecutor } from "./choiceExecutor"; + +describe("ChoiceExecutor runtime context", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockMacroRunResult.mockReturnValue(undefined); + }); + + it("creates one root context and reuses its origin leaf for nested choices", async () => { + const originLeaf = { id: "origin-leaf" }; + mockGetOpenFileOriginLeaf.mockReturnValue(originLeaf); + const app = { plugins: { plugins: {} } } as any; + const plugin = {} as any; + const nestedChoice: IChoice = { + id: "nested-template", + name: "Nested Template", + type: "Template", + command: false, + }; + const macroChoice: IMacroChoice = { + id: "macro-choice", + name: "Macro", + type: "Macro", + command: false, + runOnStartup: false, + macro: { + id: "macro", + name: "Macro", + commands: [ + { + id: "nested-command", + name: "Nested", + type: CommandType.NestedChoice, + choice: nestedChoice, + } as any, + ], + } as IMacro, + }; + + const executor = new ChoiceExecutor(app, plugin); + const result = await executor.execute(macroChoice); + + expect(result.status).toBe("success"); + expect(mockGetOpenFileOriginLeaf).toHaveBeenCalledTimes(1); + const macroCall = mockMacroConstructor.mock.calls[0]?.[0]; + expect(macroCall.originLeaf).toBe(originLeaf); + expect(macroCall.context?.originLeaf).toBe(originLeaf); + const templateCall = mockTemplateConstructor.mock.calls[0]?.[0]; + expect(templateCall.originLeaf).toBe(originLeaf); + expect(templateCall.context).toBe(macroCall.context); + }); + + it("preserves an already-aborted macro result when a pending abort signal exists", async () => { + const app = { plugins: { plugins: {} } } as any; + const plugin = {} as any; + const executor = new ChoiceExecutor(app, plugin); + const pendingAbort = new MacroAbortError("Pending child abort"); + const engineError = new Error("Macro child aborted with context"); + const macroChoice: IMacroChoice = { + id: "macro-choice", + name: "Macro", + type: "Macro", + command: false, + runOnStartup: false, + macro: { + id: "macro", + name: "Macro", + commands: [], + } as IMacro, + }; + mockMacroRunResult.mockImplementationOnce(() => { + executor.signalAbort(pendingAbort); + return createChoiceExecutionResult({ + status: "aborted", + choiceId: macroChoice.id, + stepId: "engine-step", + error: engineError, + diagnostics: [ + { + severity: "error", + code: "engine-aborted", + message: "Engine aborted with step context", + source: "runtime", + }, + ], + }); + }); + + const result = await executor.execute(macroChoice); + + expect(result.status).toBe("aborted"); + expect(result.stepId).toBe("engine-step"); + expect(result.error).toBe(engineError); + expect(result.diagnostics).toEqual([ + expect.objectContaining({ code: "engine-aborted" }), + ]); + expect(executor.consumeAbortSignal()).toBeNull(); + }); + + it("clears a synthesized root pending abort after returning an aborted result", async () => { + const app = { plugins: { plugins: {} } } as any; + const plugin = {} as any; + const executor = new ChoiceExecutor(app, plugin); + const pendingAbort = new MacroAbortError("Pending child abort"); + const macroChoice: IMacroChoice = { + id: "macro-choice", + name: "Macro", + type: "Macro", + command: false, + runOnStartup: false, + macro: { + id: "macro", + name: "Macro", + commands: [], + } as IMacro, + }; + mockMacroRunResult.mockImplementationOnce(() => { + executor.signalAbort(pendingAbort); + return createChoiceExecutionResult({ + status: "success", + choiceId: macroChoice.id, + }); + }); + + const result = await executor.execute(macroChoice); + + expect(result.status).toBe("aborted"); + expect(result.error).toBe(pendingAbort); + expect(executor.consumeAbortSignal()).toBeNull(); + }); + + it("snapshots runtime context arrays when creating choice results", async () => { + const app = { plugins: { plugins: {} } } as any; + const plugin = {} as any; + const templateChoice: IChoice = { + id: "template-choice", + name: "Template", + type: "Template", + command: false, + }; + + const executor = new ChoiceExecutor(app, plugin); + const result = await executor.execute(templateChoice); + const templateCall = mockTemplateConstructor.mock.calls[0]?.[0]; + + templateCall.context.addDiagnostic({ + severity: "info", + code: "late-context-mutation", + message: "Added after result creation", + source: "runtime", + }); + templateCall.context.addArtifact({ + id: "late-artifact", + kind: "custom", + label: "Late artifact", + createdAt: 1, + }); + + expect(result.status).toBe("success"); + expect(result.diagnostics).toHaveLength(0); + expect(result.artifacts).toHaveLength(0); + }); + + it("returns macro engine aborted results without a pending abort signal", async () => { + const app = { plugins: { plugins: {} } } as any; + const plugin = {} as any; + const abortError = new Error("Macro child aborted"); + const macroChoice: IMacroChoice = { + id: "macro-choice", + name: "Macro", + type: "Macro", + command: false, + runOnStartup: false, + macro: { + id: "macro", + name: "Macro", + commands: [], + } as IMacro, + }; + mockMacroRunResult.mockReturnValueOnce( + createChoiceExecutionResult({ + status: "aborted", + choiceId: macroChoice.id, + error: abortError, + }), + ); + + const executor = new ChoiceExecutor(app, plugin); + const result = await executor.execute(macroChoice); + + expect(result.status).toBe("aborted"); + expect(result.error).toBe(abortError); + expect(executor.consumeAbortSignal()).toBeNull(); + }); +}); diff --git a/src/choiceExecutor.ts b/src/choiceExecutor.ts index 456abb56..1fac5941 100644 --- a/src/choiceExecutor.ts +++ b/src/choiceExecutor.ts @@ -15,10 +15,19 @@ import { runOnePagePreflight } from "./preflight/runOnePagePreflight"; import { MacroAbortError } from "./errors/MacroAbortError"; import { isCancellationError } from "./utils/errorUtils"; import { getOpenFileOriginLeaf } from "./utilityObsidian"; +import { + createChoiceExecutionContext, + createChoiceExecutionResult, + type ChoiceExecutionContext, + type ChoiceExecutionResult, +} from "./engine/runtime"; +import { getIntegrationRegistry } from "./integrations/IntegrationRegistry"; export class ChoiceExecutor implements IChoiceExecutor { public variables: Map = new Map(); private pendingAbort: MacroAbortError | null = null; + private executionContext: ChoiceExecutionContext | null = null; + private executionDepth = 0; constructor(private app: App, private plugin: QuickAdd) {} @@ -32,92 +41,145 @@ export class ChoiceExecutor implements IChoiceExecutor { return abort ?? null; } - async execute(choice: IChoice): Promise { - this.pendingAbort = null; - const originLeaf = getOpenFileOriginLeaf(this.app); - // One-page preflight honoring per-choice override. + getExecutionContext(): ChoiceExecutionContext | null { + return this.executionContext; + } + + async execute(choice: IChoice): Promise { + const isRootExecution = this.executionDepth === 0 || !this.executionContext; + if (isRootExecution) { + this.pendingAbort = null; + this.executionContext = this.createRootContext(choice); + this.variables = this.executionContext.variables; + } else { + this.pendingAbort = null; + } + + this.executionDepth++; + + try { + await this.runOnePagePreflightIfEnabled(choice); + + const result = await this.executeChoiceByType(choice); + if (result.status === "aborted") { + if (isRootExecution) { + this.pendingAbort = null; + } + return result; + } + + const abort = this.pendingAbort; + if (abort) { + if (isRootExecution) { + this.pendingAbort = null; + } + return this.createResult(choice, "aborted", abort); + } + + return result; + } catch (error) { + if (error instanceof MacroAbortError) { + this.signalAbort(error); + return this.createResult(choice, "aborted", error); + } + + throw error; + } finally { + this.executionDepth--; + if (isRootExecution) { + this.executionContext = null; + this.executionDepth = 0; + } + } + } + + private createRootContext(choice: IChoice): ChoiceExecutionContext { + return createChoiceExecutionContext({ + rootChoiceId: choice.id, + originLeaf: getOpenFileOriginLeaf(this.app), + variables: this.variables, + integrations: getIntegrationRegistry(this.app), + }); + } + + private async runOnePagePreflightIfEnabled(choice: IChoice): Promise { const globalEnabled = settingsStore.getState().onePageInputEnabled; const override = choice.onePageInput; const shouldUseOnePager = override === "always" || (override !== "never" && globalEnabled); if ( - shouldUseOnePager && - (choice.type === "Template" || - choice.type === "Capture" || - choice.type === "Macro") + !shouldUseOnePager || + (choice.type !== "Template" && + choice.type !== "Capture" && + choice.type !== "Macro") ) { - try { - await runOnePagePreflight( - this.app, - this.plugin as unknown as QuickAdd, - this, - choice, - ); - } catch (error) { - if (isCancellationError(error)) { - throw new MacroAbortError("One-page input cancelled by user"); - } - throw error; + return; + } + + try { + await runOnePagePreflight( + this.app, + this.plugin as unknown as QuickAdd, + this, + choice, + ); + } catch (error) { + if (isCancellationError(error)) { + throw new MacroAbortError("One-page input cancelled by user"); } + throw error; } + } + private async executeChoiceByType( + choice: IChoice, + ): Promise { switch (choice.type) { - case "Template": { - const templateChoice: ITemplateChoice = - choice as ITemplateChoice; - await this.onChooseTemplateType(templateChoice, originLeaf); - break; - } - case "Capture": { - const captureChoice: ICaptureChoice = choice as ICaptureChoice; - await this.onChooseCaptureType(captureChoice, originLeaf); - break; - } - case "Macro": { - const macroChoice: IMacroChoice = choice as IMacroChoice; - await this.onChooseMacroType(macroChoice, originLeaf); - break; - } - case "Multi": { - const multiChoice: IMultiChoice = choice as IMultiChoice; - this.onChooseMultiType(multiChoice); - break; - } + case "Template": + return await this.onChooseTemplateType(choice as ITemplateChoice); + case "Capture": + return await this.onChooseCaptureType(choice as ICaptureChoice); + case "Macro": + return await this.onChooseMacroType(choice as IMacroChoice); + case "Multi": + this.onChooseMultiType(choice as IMultiChoice); + return this.createResult(choice, "success"); default: - break; + return this.createResult(choice, "skipped"); } } private async onChooseTemplateType( templateChoice: ITemplateChoice, - originLeaf: WorkspaceLeaf | null, - ): Promise { + ): Promise { await new TemplateChoiceEngine( this.app, this.plugin, templateChoice, this, - originLeaf, + this.getOriginLeaf(), + this.executionContext ?? undefined, ).run(); + return this.createResult(templateChoice, "success"); } private async onChooseCaptureType( captureChoice: ICaptureChoice, - originLeaf: WorkspaceLeaf | null, - ) { + ): Promise { await new CaptureChoiceEngine( this.app, this.plugin, captureChoice, this, - originLeaf, + this.getOriginLeaf(), + this.executionContext ?? undefined, ).run(); + return this.createResult(captureChoice, "success"); } private async onChooseMacroType( macroChoice: IMacroChoice, - originLeaf: WorkspaceLeaf | null, - ) { + ): Promise { const macroEngine = new MacroChoiceEngine( this.app, this.plugin, @@ -126,13 +188,15 @@ export class ChoiceExecutor implements IChoiceExecutor { this.variables, undefined, undefined, - originLeaf, + this.getOriginLeaf(), ); - await macroEngine.run(); + const result = await macroEngine.run(); Object.entries(macroEngine.params.variables).forEach(([key, value]) => { this.variables.set(key, value as string); }); + + return result; } private onChooseMultiType(multiChoice: IMultiChoice) { @@ -141,4 +205,23 @@ export class ChoiceExecutor implements IChoiceExecutor { placeholder: multiChoice.placeholder, }); } + + private getOriginLeaf(): WorkspaceLeaf | null { + return this.executionContext?.originLeaf ?? null; + } + + private createResult( + choice: IChoice, + status: ChoiceExecutionResult["status"], + error?: unknown, + ): ChoiceExecutionResult { + return createChoiceExecutionResult({ + status, + choiceId: choice.id, + stepId: this.executionContext?.createStepId(choice.type), + artifacts: [...(this.executionContext?.artifacts ?? [])], + diagnostics: [...(this.executionContext?.diagnostics ?? [])], + error, + }); + } } diff --git a/src/cli/registerQuickAddCliHandlers.test.ts b/src/cli/registerQuickAddCliHandlers.test.ts index a72f8c53..75cb98f8 100644 --- a/src/cli/registerQuickAddCliHandlers.test.ts +++ b/src/cli/registerQuickAddCliHandlers.test.ts @@ -8,21 +8,18 @@ import type IMultiChoice from "../types/choices/IMultiChoice"; const { ChoiceExecutorMock, - collectChoiceRequirementsMock, - getUnresolvedRequirementsMock, + collectChoiceFlowPreflightMock, } = vi.hoisted(() => ({ ChoiceExecutorMock: vi.fn(), - collectChoiceRequirementsMock: vi.fn(), - getUnresolvedRequirementsMock: vi.fn(), + collectChoiceFlowPreflightMock: vi.fn(), })); vi.mock("../choiceExecutor", () => ({ ChoiceExecutor: ChoiceExecutorMock, })); -vi.mock("../preflight/collectChoiceRequirements", () => ({ - collectChoiceRequirements: collectChoiceRequirementsMock, - getUnresolvedRequirements: getUnresolvedRequirementsMock, +vi.mock("../preflight/collectChoiceFlowPreflight", () => ({ + collectChoiceFlowPreflight: collectChoiceFlowPreflightMock, })); interface RegisteredCliHandler { @@ -130,12 +127,15 @@ describe("registerQuickAddCliHandlers", () => { beforeEach(() => { executors = []; ChoiceExecutorMock.mockReset(); - collectChoiceRequirementsMock.mockReset(); - getUnresolvedRequirementsMock.mockReset(); + collectChoiceFlowPreflightMock.mockReset(); ChoiceExecutorMock.mockImplementation(() => { const executor: IChoiceExecutor = { - execute: vi.fn().mockResolvedValue(undefined), + execute: vi.fn().mockResolvedValue({ + status: "success", + artifacts: [], + diagnostics: [], + }), variables: new Map(), consumeAbortSignal: vi.fn().mockReturnValue(null), }; @@ -143,8 +143,12 @@ describe("registerQuickAddCliHandlers", () => { return executor; }); - collectChoiceRequirementsMock.mockResolvedValue([]); - getUnresolvedRequirementsMock.mockReturnValue([]); + collectChoiceFlowPreflightMock.mockResolvedValue({ + requirements: [], + unresolvedRequirements: [], + diagnostics: [], + choices: [], + }); }); it("registers run/list/check handlers when CLI API is available", () => { @@ -215,22 +219,115 @@ describe("registerQuickAddCliHandlers", () => { label: "Title", type: "text", }; - collectChoiceRequirementsMock.mockResolvedValue([missingRequirement]); - getUnresolvedRequirementsMock.mockReturnValue([missingRequirement]); + collectChoiceFlowPreflightMock.mockResolvedValue({ + requirements: [missingRequirement], + unresolvedRequirements: [missingRequirement], + diagnostics: [], + choices: [ + { + id: macroChoice.id, + name: macroChoice.name, + type: macroChoice.type, + path: macroChoice.name, + depth: 0, + }, + { + id: nestedCaptureChoice.id, + name: nestedCaptureChoice.name, + type: nestedCaptureChoice.type, + path: `${macroChoice.name} / ${nestedCaptureChoice.name}`, + depth: 1, + }, + ], + }); const output = await Promise.resolve( run!.handler({ - choice: templateChoice.name, + choice: macroChoice.name, }), ); const payload = JSON.parse(String(output)); expect(payload.ok).toBe(false); - expect(payload.missingInputCount).toBeUndefined(); + expect(payload.requiredInputCount).toBe(1); + expect(payload.missingInputCount).toBe(1); expect(payload.missingFlags).toContain("value-title="); + expect(payload.flow.choiceCount).toBe(2); + expect(payload.flow.choices[1]).toMatchObject({ + id: nestedCaptureChoice.id, + depth: 1, + }); + expect(collectChoiceFlowPreflightMock).toHaveBeenCalledWith( + plugin.app, + plugin, + executors[0], + macroChoice, + ); expect(executors[0].execute).not.toHaveBeenCalled(); }); + it("returns aborted when executor result status is aborted", async () => { + const executor: IChoiceExecutor = { + execute: vi.fn().mockResolvedValue({ + status: "aborted", + error: new Error("Stopped by child"), + artifacts: [], + diagnostics: [], + }), + variables: new Map(), + consumeAbortSignal: vi.fn().mockReturnValue(null), + }; + ChoiceExecutorMock.mockReturnValueOnce(executor); + const { plugin, handlers } = createPlugin([ + templateChoice, + macroChoice, + multiChoice, + ]); + registerQuickAddCliHandlers(plugin); + const run = handlers.find((handler) => handler.command === "quickadd:run"); + expect(run).toBeDefined(); + + const output = await Promise.resolve( + run!.handler({ choice: templateChoice.name }), + ); + const payload = JSON.parse(String(output)); + + expect(payload.ok).toBe(false); + expect(payload.aborted).toBe(true); + expect(payload.error).toBe("Stopped by child"); + }); + + it("returns failed when executor result status is failed", async () => { + const executor: IChoiceExecutor = { + execute: vi.fn().mockResolvedValue({ + status: "failed", + error: new Error("Boom"), + artifacts: [], + diagnostics: [], + }), + variables: new Map(), + consumeAbortSignal: vi.fn().mockReturnValue(null), + }; + ChoiceExecutorMock.mockReturnValueOnce(executor); + const { plugin, handlers } = createPlugin([ + templateChoice, + macroChoice, + multiChoice, + ]); + registerQuickAddCliHandlers(plugin); + const run = handlers.find((handler) => handler.command === "quickadd:run"); + expect(run).toBeDefined(); + + const output = await Promise.resolve( + run!.handler({ choice: templateChoice.name }), + ); + const payload = JSON.parse(String(output)); + + expect(payload.ok).toBe(false); + expect(payload.aborted).toBeUndefined(); + expect(payload.error).toBe("Boom"); + }); + it("lists flattened choices and supports command filter", async () => { const { plugin, handlers } = createPlugin([ templateChoice, @@ -271,8 +368,28 @@ describe("registerQuickAddCliHandlers", () => { label: "Project", type: "text", }; - collectChoiceRequirementsMock.mockResolvedValue([requirement]); - getUnresolvedRequirementsMock.mockReturnValue([requirement]); + collectChoiceFlowPreflightMock.mockResolvedValue({ + requirements: [requirement], + unresolvedRequirements: [requirement], + diagnostics: [ + { + severity: "error", + code: "missing-required-inputs", + message: "1 required input(s) are missing for this flow.", + source: "runtime", + details: { missingIds: ["project"] }, + }, + ], + choices: [ + { + id: templateChoice.id, + name: templateChoice.name, + type: templateChoice.type, + path: templateChoice.name, + depth: 0, + }, + ], + }); const output = await Promise.resolve( check!.handler({ @@ -284,6 +401,12 @@ describe("registerQuickAddCliHandlers", () => { expect(payload.ok).toBe(false); expect(payload.requiredInputCount).toBe(1); expect(payload.missingInputCount).toBe(1); + expect(payload.diagnosticCount).toBe(1); + expect(payload.diagnostics[0]).toMatchObject({ + severity: "error", + code: "missing-required-inputs", + }); + expect(payload.flow.choiceCount).toBe(1); expect(executors[0].execute).not.toHaveBeenCalled(); }); }); diff --git a/src/cli/registerQuickAddCliHandlers.ts b/src/cli/registerQuickAddCliHandlers.ts index afd0d3ff..85e99bbd 100644 --- a/src/cli/registerQuickAddCliHandlers.ts +++ b/src/cli/registerQuickAddCliHandlers.ts @@ -3,10 +3,7 @@ import { ChoiceExecutor } from "../choiceExecutor"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import { log } from "../logger/logManager"; import type QuickAdd from "../main"; -import { - collectChoiceRequirements, - getUnresolvedRequirements, -} from "../preflight/collectChoiceRequirements"; +import { collectChoiceFlowPreflight } from "../preflight/collectChoiceFlowPreflight"; import type IChoice from "../types/choices/IChoice"; import type IMultiChoice from "../types/choices/IMultiChoice"; @@ -211,6 +208,28 @@ function toMissingFieldSummary(requirement: { }; } +function toDiagnosticSummary(diagnostic: { + severity: string; + code: string; + message: string; + source: string; + stepId?: string; + choiceId?: string; + integrationId?: string; + details?: Record; +}) { + return { + severity: diagnostic.severity, + code: diagnostic.code, + message: diagnostic.message, + source: diagnostic.source, + stepId: diagnostic.stepId, + choiceId: diagnostic.choiceId, + integrationId: diagnostic.integrationId, + details: diagnostic.details, + }; +} + function setExecutorVariables( choiceExecutor: IChoiceExecutor, variables: Record, @@ -228,6 +247,12 @@ function describeChoice(choice: IChoice) { }; } +function errorMessageFromUnknown(error: unknown, fallback: string): string { + if (error instanceof Error && error.message) return error.message; + if (typeof error === "string" && error.length > 0) return error; + return fallback; +} + async function runChoiceHandler( plugin: QuickAdd, params: CliData, @@ -255,31 +280,36 @@ async function runChoiceHandler( const interactiveMode = isTruthy(params.ui); if (!interactiveMode) { - const requirements = await collectChoiceRequirements( + const preflight = await collectChoiceFlowPreflight( plugin.app, plugin, choiceExecutor, choice, ); - const unresolved = getUnresolvedRequirements( - requirements, - choiceExecutor.variables, - ); + const unresolved = preflight.unresolvedRequirements; if (unresolved.length > 0) { return serialize({ ok: false, command, error: "Missing required inputs for non-interactive CLI run.", choice: describeChoice(choice), + requiredInputCount: preflight.requirements.length, + missingInputCount: unresolved.length, missing: unresolved.map(toMissingFieldSummary), missingFlags: unresolved.map( (requirement) => `value-${requirement.id}=`, ), + diagnosticCount: preflight.diagnostics.length, + diagnostics: preflight.diagnostics.map(toDiagnosticSummary), + flow: { + choiceCount: preflight.choices.length, + choices: preflight.choices, + }, }); } } - await choiceExecutor.execute(choice); + const result = await choiceExecutor.execute(choice); const aborted = choiceExecutor.consumeAbortSignal?.(); const durationMs = Date.now() - startedAt; @@ -294,6 +324,33 @@ async function runChoiceHandler( }); } + if (result.status === "aborted") { + return serialize({ + ok: false, + command, + error: errorMessageFromUnknown( + result.error, + "Choice execution aborted", + ), + aborted: true, + choice: describeChoice(choice), + durationMs, + }); + } + + if (result.status === "failed") { + return serialize({ + ok: false, + command, + error: errorMessageFromUnknown( + result.error, + "Choice execution failed", + ), + choice: describeChoice(choice), + durationMs, + }); + } + return serialize({ ok: true, command, @@ -372,27 +429,30 @@ async function checkChoiceHandler( ) as IChoiceExecutor; setExecutorVariables(choiceExecutor, variables); - const requirements = await collectChoiceRequirements( + const preflight = await collectChoiceFlowPreflight( plugin.app, plugin, choiceExecutor, choice, ); - const unresolved = getUnresolvedRequirements( - requirements, - choiceExecutor.variables, - ); + const unresolved = preflight.unresolvedRequirements; return serialize({ ok: unresolved.length === 0, command: CLI_COMMANDS.check, choice: describeChoice(choice), - requiredInputCount: requirements.length, + requiredInputCount: preflight.requirements.length, missingInputCount: unresolved.length, missing: unresolved.map(toMissingFieldSummary), missingFlags: unresolved.map( (requirement) => `value-${requirement.id}=`, ), + diagnosticCount: preflight.diagnostics.length, + diagnostics: preflight.diagnostics.map(toDiagnosticSummary), + flow: { + choiceCount: preflight.choices.length, + choices: preflight.choices, + }, }); } catch (error) { return serialize({ diff --git a/src/engine/CaptureChoiceEngine.ts b/src/engine/CaptureChoiceEngine.ts index 55e81634..d3bc336d 100644 --- a/src/engine/CaptureChoiceEngine.ts +++ b/src/engine/CaptureChoiceEngine.ts @@ -28,14 +28,9 @@ import { insertFileLinkToActiveView, insertOnNewLineAbove, insertOnNewLineBelow, - isTemplaterTriggerOnCreateEnabled, - jumpToNextTemplaterCursorIfPossible, isFolder, openExistingFileTab, openFile, - overwriteTemplaterOnce, - templaterParseTemplate, - waitForTemplaterTriggerOnCreateToComplete, } from "../utilityObsidian"; import { isCancellationError, reportError } from "../utils/errorUtils"; import { normalizeFileOpening } from "../utils/fileOpeningDefaults"; @@ -54,6 +49,11 @@ import { type ConfiguredCanvasCaptureTarget, } from "./canvasCapture"; import { handleMacroAbort } from "../utils/macroAbortHandler"; +import { + FormatOrchestrator, + type CaptureRunPlan, + type ChoiceExecutionContext, +} from "./runtime"; const DEFAULT_NOTICE_DURATION = 4000; @@ -63,6 +63,7 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { private readonly plugin: QuickAdd; private templatePropertyVars?: Map; private capturePropertyVars: Map = new Map(); + private readonly formatOrchestrator: FormatOrchestrator; constructor( app: App, @@ -70,11 +71,19 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { choice: ICaptureChoice, private choiceExecutor: IChoiceExecutor, private readonly originLeaf: WorkspaceLeaf | null = null, + executionContext?: ChoiceExecutionContext, ) { - super(app); + const context = executionContext ?? choiceExecutor.getExecutionContext?.() ?? undefined; + super(app, context); this.choice = choice; this.plugin = plugin; - this.formatter = new CaptureChoiceFormatter(app, plugin, choiceExecutor); + this.formatOrchestrator = new FormatOrchestrator(app, context); + this.formatter = new CaptureChoiceFormatter( + app, + plugin, + choiceExecutor, + context, + ); } private showSuccessNotice( @@ -256,6 +265,13 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { ); } + const capturePlan = this.createCaptureRunPlan( + filePath, + action, + fileAlreadyExists, + ); + this.formatOrchestrator.recordCapturePlan(capturePlan); + const { file, newFileContent, captureContent } = await getFileAndAddContentFn(filePath, content); @@ -268,11 +284,11 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { if (isEditorInsertionAction) { // Parse Templater syntax in the capture content. // If Templater isn't installed, it just returns the capture content. - const content = await templaterParseTemplate( - this.app, - captureContent, - file, - ); + const content = + await this.formatOrchestrator.parseTemplaterTemplate( + captureContent, + file, + ); switch (action) { case "currentLine": @@ -288,11 +304,20 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { } else { await this.app.vault.modify(file, newFileContent); if (this.choice.templater?.afterCapture === "wholeFile") { - await overwriteTemplaterOnce(this.app, file); + await this.formatOrchestrator.overwriteTemplaterOnce(file, { + diagnoseMissingCapability: true, + }); } await this.applyCapturePropertyVars(file); } + this.formatOrchestrator.recordCaptureResult({ + choiceId: this.choice.id, + filePath: file.path, + action, + wasNewFile: !fileAlreadyExists, + }); + // Show success notification if (this.plugin.settings.showCaptureNotification) { this.showSuccessNotice(file, { @@ -313,11 +338,13 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { if (!openExistingTab) { await openFile(this.app, file, { ...fileOpening, - originLeaf: this.originLeaf, + originLeaf: this.getOriginLeaf(), }); } - await jumpToNextTemplaterCursorIfPossible(this.app, file); + await this.formatOrchestrator.jumpToNextTemplaterCursorIfPossible( + file, + ); } } catch (err) { if ( @@ -334,6 +361,56 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { } } + private getOriginLeaf(): WorkspaceLeaf | null { + return this.getExecutionContext()?.originLeaf ?? this.originLeaf; + } + + private createCaptureRunPlan( + filePath: string, + action: CaptureAction, + fileAlreadyExists: boolean, + ): CaptureRunPlan { + const createWithTemplate = + !fileAlreadyExists && + !!this.choice.createFileIfItDoesntExist?.createWithTemplate; + return { + choiceId: this.choice.id, + targetPath: filePath, + action, + fileAlreadyExists, + createWithTemplate, + templaterPolicy: this.getCaptureTemplaterPolicy( + action, + createWithTemplate, + fileAlreadyExists, + ), + }; + } + + private getCaptureTemplaterPolicy( + action: CaptureAction, + createWithTemplate: boolean, + fileAlreadyExists: boolean, + ): CaptureRunPlan["templaterPolicy"] { + if ( + action === "currentLine" || + action === "newLineAbove" || + action === "newLineBelow" + ) { + return "parse-capture"; + } + if (this.choice.templater?.afterCapture === "wholeFile") { + return "render-whole-file"; + } + if (createWithTemplate) { + return "render-whole-file"; + } + if (!fileAlreadyExists && this.formatOrchestrator.isTemplaterTriggerOnCreateEnabled()) { + return "wait-for-on-create"; + } + return "none"; + } + private async handleCanvasTextCapture( target: CanvasTextCaptureTarget, action: CaptureAction, @@ -393,11 +470,13 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { if (!openExistingTab) { await openFile(this.app, file, { ...fileOpening, - originLeaf: this.originLeaf, + originLeaf: this.getOriginLeaf(), }); } - await jumpToNextTemplaterCursorIfPossible(this.app, file); + await this.formatOrchestrator.jumpToNextTemplaterCursorIfPossible( + file, + ); } } @@ -715,6 +794,7 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { this.plugin, this.choice.createFileIfItDoesntExist.template, this.choiceExecutor, + this.getExecutionContext(), ); if (linkOptions?.enabled && !linkOptions.requireActiveFile) { @@ -753,9 +833,13 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine { this.choice.createFileIfItDoesntExist.createWithTemplate && fileContent ) { - await overwriteTemplaterOnce(this.app, file); - } else if (isTemplaterTriggerOnCreateEnabled(this.app)) { - await waitForTemplaterTriggerOnCreateToComplete(this.app, file); + await this.formatOrchestrator.overwriteTemplaterOnce(file, { + diagnoseMissingCapability: fileContent.includes("<%"), + }); + } else if (this.formatOrchestrator.isTemplaterTriggerOnCreateEnabled()) { + await this.formatOrchestrator.waitForTemplaterTriggerOnCreateToComplete( + file, + ); } // Read the file fresh from disk to avoid any potential cached content diff --git a/src/engine/MacroChoiceEngine.entry.test.ts b/src/engine/MacroChoiceEngine.entry.test.ts index dc63f85d..3ab1737e 100644 --- a/src/engine/MacroChoiceEngine.entry.test.ts +++ b/src/engine/MacroChoiceEngine.entry.test.ts @@ -12,6 +12,7 @@ import type { INestedChoiceCommand } from "../types/macros/QuickCommands/INested import type IChoice from "../types/choices/IChoice"; import { MacroAbortError } from "../errors/MacroAbortError"; import { QuickAddApi } from "../quickAddApi"; +import { createChoiceExecutionResult } from "./runtime"; const { mockGetUserScript, mockInitializeUserScriptSettings, mockSuggest, mockGetApi, mockInputPrompt } = vi.hoisted(() => ({ @@ -574,7 +575,13 @@ describe("MacroChoiceEngine choice command cancellation", () => { choiceId: "target-choice", }; const abortError = new MacroAbortError("Input cancelled by user"); - choiceExecutor.execute = vi.fn().mockResolvedValue(undefined); + choiceExecutor.execute = vi.fn(async (choiceToRun: IChoice) => + createChoiceExecutionResult({ + status: "aborted", + choiceId: choiceToRun.id, + error: abortError, + }), + ); (choiceExecutor.consumeAbortSignal as ReturnType).mockReturnValueOnce(abortError); await expect( @@ -607,7 +614,13 @@ describe("MacroChoiceEngine choice command cancellation", () => { choice, }; const abortError = new MacroAbortError("Input cancelled by user"); - choiceExecutor.execute = vi.fn().mockResolvedValue(undefined); + choiceExecutor.execute = vi.fn(async (choiceToRun: IChoice) => + createChoiceExecutionResult({ + status: "aborted", + choiceId: choiceToRun.id, + error: abortError, + }), + ); (choiceExecutor.consumeAbortSignal as ReturnType).mockReturnValueOnce(abortError); await expect( diff --git a/src/engine/MacroChoiceEngine.notice.test.ts b/src/engine/MacroChoiceEngine.notice.test.ts index d9608499..a721e225 100644 --- a/src/engine/MacroChoiceEngine.notice.test.ts +++ b/src/engine/MacroChoiceEngine.notice.test.ts @@ -65,6 +65,11 @@ import { MacroChoiceEngine } from "./MacroChoiceEngine"; import { MacroAbortError } from "../errors/MacroAbortError"; import { settingsStore } from "../settingsStore"; import type IChoice from "../types/choices/IChoice"; +import { + createChoiceExecutionContext, + createChoiceExecutionResult, +} from "./runtime"; +import { IntegrationRegistry } from "../integrations/IntegrationRegistry"; const defaultSettingsState = structuredClone(settingsStore.getState()); @@ -174,6 +179,154 @@ describe("MacroChoiceEngine cancellation notices", () => { expect(noticeClass.instances[0]?.message).toContain("Invalid project name"); }); +describe("MacroChoiceEngine command results", () => { + it("returns typed command results for executed macro commands", async () => { + const executeCommandById = vi.fn(); + const app = { + commands: { executeCommandById }, + } as unknown as App; + const plugin = { settings: settingsStore.getState() } as any; + const macro: IMacro = { + id: "macro-id", + name: "Macro with commands", + commands: [ + { + id: "first-command", + name: "First", + type: CommandType.Obsidian, + commandId: "first", + } as any, + { + id: "second-command", + name: "Second", + type: CommandType.Obsidian, + commandId: "second", + } as any, + ], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + const choiceExecutor: IChoiceExecutor = { + variables: new Map(), + execute: vi.fn(async (choiceToRun) => + createChoiceExecutionResult({ + status: "success", + choiceId: choiceToRun.id, + }), + ), + }; + + const engine = new MacroChoiceEngine( + app, + plugin, + choice, + choiceExecutor, + new Map(), + ); + + const result = await engine.run(); + const commandResults = engine.getCommandResults(); + + expect(result.status).toBe("success"); + expect(commandResults).toHaveLength(2); + expect(commandResults.map((command) => command.commandId)).toEqual([ + "first-command", + "second-command", + ]); + expect(commandResults.every((command) => command.status === "success")).toBe( + true, + ); + expect(executeCommandById).toHaveBeenCalledWith("first"); + expect(executeCommandById).toHaveBeenCalledWith("second"); + }); + + it("snapshots runtime context arrays when creating macro results", async () => { + const executeCommandById = vi.fn(); + const app = { + commands: { executeCommandById }, + plugins: { plugins: {} }, + } as unknown as App; + const plugin = { settings: settingsStore.getState() } as any; + const macro: IMacro = { + id: "macro-id", + name: "Macro with commands", + commands: [], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + const context = createChoiceExecutionContext({ + integrations: new IntegrationRegistry(), + variables: new Map(), + }); + context.addDiagnostic({ + severity: "info", + code: "macro-before", + message: "Before result creation", + source: "runtime", + }); + context.addArtifact({ + id: "macro-before-artifact", + kind: "custom", + label: "Before artifact", + createdAt: 1, + }); + const choiceExecutor: IChoiceExecutor = { + variables: context.variables, + execute: vi.fn(async (choiceToRun) => + createChoiceExecutionResult({ + status: "success", + choiceId: choiceToRun.id, + }), + ), + getExecutionContext: () => context, + }; + + const engine = new MacroChoiceEngine( + app, + plugin, + choice, + choiceExecutor, + context.variables, + undefined, + undefined, + null, + ); + + const result = await engine.run(); + context.addDiagnostic({ + severity: "info", + code: "macro-after", + message: "After result creation", + source: "runtime", + }); + context.addArtifact({ + id: "macro-after-artifact", + kind: "custom", + label: "After artifact", + createdAt: 2, + }); + + expect(result.diagnostics).toEqual([ + expect.objectContaining({ code: "macro-before" }), + ]); + expect(result.artifacts).toEqual([ + expect.objectContaining({ id: "macro-before-artifact" }), + ]); + }); +}); + describe("MacroChoiceEngine nested choice propagation", () => { it("halts subsequent commands when a nested choice cancels", async () => { const app = {} as App; @@ -223,8 +376,19 @@ describe("MacroChoiceEngine nested choice propagation", () => { variables: new Map(), execute: vi.fn(async (choiceToRun) => { if (choiceToRun.id === nestedChoice.id) { - signalAbort(new MacroAbortError("Input cancelled by user")); + const abort = new MacroAbortError("Input cancelled by user"); + signalAbort(abort); + return createChoiceExecutionResult({ + status: "aborted", + choiceId: choiceToRun.id, + error: abort, + }); } + + return createChoiceExecutionResult({ + status: "success", + choiceId: choiceToRun.id, + }); }), signalAbort, consumeAbortSignal, @@ -246,8 +410,11 @@ describe("MacroChoiceEngine nested choice propagation", () => { new Map(), ); - await engine.run(); + const result = await engine.run(); + expect(result.status).toBe("aborted"); + expect(engine.getCommandResults()).toHaveLength(1); + expect(engine.getCommandResults()[0].status).toBe("aborted"); expect(choiceExecutor.execute).toHaveBeenCalledTimes(1); expect(signalAbort).toHaveBeenCalled(); expect(signalAbort.mock.calls.at(-1)?.[0]).toBeInstanceOf(MacroAbortError); diff --git a/src/engine/MacroChoiceEngine.review-feedback.test.ts b/src/engine/MacroChoiceEngine.review-feedback.test.ts new file mode 100644 index 00000000..32e3fd9f --- /dev/null +++ b/src/engine/MacroChoiceEngine.review-feedback.test.ts @@ -0,0 +1,518 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { mockGetUserScript, mockOpenFile } = vi.hoisted(() => ({ + mockGetUserScript: vi.fn(), + mockOpenFile: vi.fn(), +})); + +vi.mock("../quickAddSettingsTab", () => { + const defaultSettings = { + choices: [], + inputPrompt: "single-line", + devMode: false, + templateFolderPath: "", + useSelectionAsCaptureValue: true, + announceUpdates: "major", + version: "0.0.0", + globalVariables: {}, + onePageInputEnabled: false, + disableOnlineFeatures: true, + enableRibbonIcon: false, + showCaptureNotification: true, + showInputCancellationNotification: true, + enableTemplatePropertyTypes: false, + ai: { + defaultModel: "Ask me", + defaultSystemPrompt: "", + promptTemplatesFolderPath: "", + showAssistant: true, + providers: [], + }, + migrations: { + migrateToMacroIDFromEmbeddedMacro: true, + useQuickAddTemplateFolder: false, + incrementFileNameSettingMoveToDefaultBehavior: false, + consolidateFileExistsBehavior: false, + mutualExclusionInsertAfterAndWriteToBottomOfFile: false, + setVersionAfterUpdateModalRelease: false, + addDefaultAIProviders: false, + removeMacroIndirection: false, + migrateFileOpeningSettings: false, + backfillFileOpeningDefaults: false, + }, + }; + + return { + DEFAULT_SETTINGS: defaultSettings, + QuickAddSettingsTab: class {}, + }; +}); + +vi.mock("../formatters/completeFormatter", () => ({ + CompleteFormatter: class CompleteFormatterMock { + async formatFileName(input: string) { + return input; + } + }, +})); + +vi.mock("obsidian-dataview", () => ({ + getAPI: vi.fn(), +})); + +vi.mock("../main", () => ({ + default: class QuickAddMock {}, +})); + +vi.mock("../utilityObsidian", () => ({ + getUserScript: mockGetUserScript, + openFile: mockOpenFile, +})); + +import type { App } from "obsidian"; +import { TFile } from "obsidian"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; +import { MacroAbortError } from "../errors/MacroAbortError"; +import type IChoice from "../types/choices/IChoice"; +import type IMacroChoice from "../types/choices/IMacroChoice"; +import { CommandType } from "../types/macros/CommandType"; +import type { IMacro } from "../types/macros/IMacro"; +import type { IUserScript } from "../types/macros/IUserScript"; +import { IntegrationRegistry } from "../integrations/IntegrationRegistry"; +import { MacroChoiceEngine } from "./MacroChoiceEngine"; +import { + createChoiceExecutionContext, + createChoiceExecutionResult, +} from "./runtime"; + +function createChoiceExecutor(): IChoiceExecutor { + return { + variables: new Map(), + execute: vi.fn(async (choice) => + createChoiceExecutionResult({ + status: "success", + choiceId: choice.id, + }), + ), + }; +} + +describe("MacroChoiceEngine review feedback regressions", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("does not keep partial branch results when a conditional branch aborts", async () => { + const app = {} as App; + const plugin = {} as any; + const nestedChoice: IChoice = { + id: "nested-choice", + name: "Nested choice", + type: "Template", + command: false, + }; + const macro: IMacro = { + id: "macro-id", + name: "Conditional macro", + commands: [ + { + id: "conditional-command", + name: "Conditional", + type: CommandType.Conditional, + condition: { + mode: "variable", + variableName: "runThen", + operator: "equals", + valueType: "boolean", + expectedValue: "true", + }, + thenCommands: [ + { + id: "branch-obsidian", + name: "Branch Obsidian", + type: CommandType.Obsidian, + commandId: "branch-obsidian", + } as any, + { + id: "branch-nested", + name: "Branch nested choice", + type: CommandType.NestedChoice, + choice: nestedChoice, + } as any, + ], + elseCommands: [], + } as any, + ], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Conditional Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + + let pendingAbort: MacroAbortError | null = null; + const choiceExecutor: IChoiceExecutor = { + variables: new Map([["runThen", true]]), + execute: vi.fn(async (choiceToRun) => { + if (choiceToRun.id === nestedChoice.id) { + const abort = new MacroAbortError("Input cancelled by user"); + pendingAbort = abort; + return createChoiceExecutionResult({ + status: "aborted", + choiceId: choiceToRun.id, + error: abort, + }); + } + + return createChoiceExecutionResult({ + status: "success", + choiceId: choiceToRun.id, + }); + }), + signalAbort: vi.fn((error: MacroAbortError) => { + pendingAbort = error; + }), + consumeAbortSignal: vi.fn(() => { + const abort = pendingAbort; + pendingAbort = null; + return abort; + }), + }; + + class ObservationMacroChoiceEngine extends MacroChoiceEngine { + public obsidianExecutions = 0; + + protected override executeObsidianCommand(): void { + this.obsidianExecutions += 1; + } + } + + const engine = new ObservationMacroChoiceEngine( + app, + plugin, + choice, + choiceExecutor, + choiceExecutor.variables, + ); + + const result = await engine.run(); + + expect(result.status).toBe("aborted"); + expect(engine.obsidianExecutions).toBe(1); + expect(engine.getCommandResults()).toHaveLength(1); + expect(engine.getCommandResults()[0]).toMatchObject({ + commandId: "conditional-command", + status: "aborted", + }); + }); + + it("preserves the abort error on the macro-level result", async () => { + const app = {} as App; + const plugin = {} as any; + const macro: IMacro = { + id: "macro-id", + name: "Abort macro", + commands: [ + { + id: "obsidian-command", + name: "Obsidian", + type: CommandType.Obsidian, + commandId: "do-something", + } as any, + ], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Abort Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + + class AbortMacroChoiceEngine extends MacroChoiceEngine { + protected override executeObsidianCommand(): void { + throw new MacroAbortError("Input cancelled by user"); + } + } + + const engine = new AbortMacroChoiceEngine( + app, + plugin, + choice, + createChoiceExecutor(), + new Map(), + ); + + const result = await engine.run(); + + expect(result.status).toBe("aborted"); + expect(result.error).toBeInstanceOf(MacroAbortError); + expect((result.error as MacroAbortError).message).toBe( + "Input cancelled by user", + ); + }); + + it("uses the execution context origin leaf for open-file commands", async () => { + const originLeaf = { id: "origin-leaf" } as any; + const file = Object.assign(Object.create(TFile.prototype), { + path: "note.md", + }) as TFile; + const app = { + vault: { + getAbstractFileByPath: vi.fn().mockReturnValue(file), + }, + } as unknown as App; + const plugin = {} as any; + const macro: IMacro = { + id: "macro-id", + name: "Open file macro", + commands: [ + { + id: "open-file-command", + name: "Open file", + type: CommandType.OpenFile, + filePath: "note.md", + } as any, + ], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Open File Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + const context = createChoiceExecutionContext({ + originLeaf, + variables: new Map(), + integrations: new IntegrationRegistry(), + }); + const choiceExecutor: IChoiceExecutor = { + variables: context.variables, + execute: vi.fn(), + getExecutionContext: () => context, + }; + + const engine = new MacroChoiceEngine( + app, + plugin, + choice, + choiceExecutor, + context.variables, + undefined, + undefined, + null, + ); + + await engine.run(); + + expect(mockOpenFile).toHaveBeenCalledWith( + app, + file, + expect.objectContaining({ originLeaf }), + ); + }); + + it("keeps conditional branch command results in execution order", async () => { + const executeCommandById = vi.fn(); + const app = { + commands: { executeCommandById }, + } as unknown as App; + const plugin = {} as any; + const macro: IMacro = { + id: "macro-id", + name: "Ordered conditional macro", + commands: [ + { + id: "first-command", + name: "First", + type: CommandType.Obsidian, + commandId: "first", + } as any, + { + id: "conditional-command", + name: "Conditional", + type: CommandType.Conditional, + condition: { + mode: "variable", + variableName: "runThen", + operator: "equals", + valueType: "boolean", + expectedValue: "true", + }, + thenCommands: [ + { + id: "branch-command", + name: "Branch", + type: CommandType.Obsidian, + commandId: "branch", + } as any, + ], + elseCommands: [], + } as any, + { + id: "third-command", + name: "Third", + type: CommandType.Obsidian, + commandId: "third", + } as any, + ], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Ordered Conditional Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + const choiceExecutor: IChoiceExecutor = { + variables: new Map([["runThen", true]]), + execute: vi.fn(), + }; + + const engine = new MacroChoiceEngine( + app, + plugin, + choice, + choiceExecutor, + choiceExecutor.variables, + ); + + await engine.run(); + + expect(executeCommandById).toHaveBeenNthCalledWith(1, "first"); + expect(executeCommandById).toHaveBeenNthCalledWith(2, "branch"); + expect(executeCommandById).toHaveBeenNthCalledWith(3, "third"); + expect(engine.getCommandResults().map((result) => result.commandId)).toEqual([ + "first-command", + "branch-command", + "conditional-command", + "third-command", + ]); + }); + + it("preserves successful command results when a later command fails", async () => { + const app = {} as App; + const plugin = {} as any; + const macro: IMacro = { + id: "macro-id", + name: "Failure macro", + commands: [ + { + id: "first-command", + name: "First", + type: CommandType.Obsidian, + commandId: "first", + } as any, + { + id: "second-command", + name: "Second", + type: CommandType.Obsidian, + commandId: "second", + } as any, + ], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Failure Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + + class FailureMacroChoiceEngine extends MacroChoiceEngine { + private executionCount = 0; + + protected override executeObsidianCommand(): void { + this.executionCount += 1; + if (this.executionCount === 2) { + throw new Error("Boom"); + } + } + } + + const engine = new FailureMacroChoiceEngine( + app, + plugin, + choice, + createChoiceExecutor(), + new Map(), + ); + + await expect(engine.run()).rejects.toThrow("Boom"); + expect(engine.getCommandResults()).toHaveLength(2); + expect(engine.getCommandResults()[0]).toMatchObject({ + commandId: "first-command", + status: "success", + }); + expect(engine.getCommandResults()[1]).toMatchObject({ + commandId: "second-command", + status: "failed", + }); + }); + + it("only records command values for commands that produce output", async () => { + const app = {} as App; + const plugin = {} as any; + const macro: IMacro = { + id: "macro-id", + name: "Output macro", + commands: [ + { + id: "script-command", + name: "Script", + type: CommandType.UserScript, + path: "script.js", + settings: {}, + } as IUserScript, + { + id: "obsidian-command", + name: "Obsidian", + type: CommandType.Obsidian, + commandId: "do-something", + } as any, + ], + }; + const choice: IMacroChoice = { + id: "choice-id", + name: "Output Macro", + type: "Macro", + command: false, + macro, + runOnStartup: false, + }; + + class OutputMacroChoiceEngine extends MacroChoiceEngine { + protected override async executeUserScript(): Promise { + this.setOutput("script output"); + } + + protected override executeObsidianCommand(): void {} + } + + const engine = new OutputMacroChoiceEngine( + app, + plugin, + choice, + createChoiceExecutor(), + new Map(), + ); + + const result = await engine.run(); + const [scriptResult, obsidianResult] = engine.getCommandResults(); + + expect(result.status).toBe("success"); + expect(scriptResult?.value).toBe("script output"); + expect(obsidianResult?.value).toBeUndefined(); + expect(result.value).toMatchObject({ + output: "script output", + }); + }); +}); diff --git a/src/engine/MacroChoiceEngine.ts b/src/engine/MacroChoiceEngine.ts index 4a83d226..889ec2d2 100644 --- a/src/engine/MacroChoiceEngine.ts +++ b/src/engine/MacroChoiceEngine.ts @@ -53,6 +53,12 @@ import { evaluateCondition } from "./helpers/conditionalEvaluator"; import { handleMacroAbort } from "../utils/macroAbortHandler"; import { buildOpenFileOptions } from "./helpers/openFileOptions"; import { createVariablesProxy } from "../utils/variablesProxy"; +import { + createChoiceExecutionResult, + createCommandExecutionResult, + type ChoiceExecutionResult, + type CommandExecutionResult, +} from "./runtime"; type ConditionalScriptRunner = () => Promise; @@ -85,6 +91,7 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { private conditionalScriptCache = new Map(); private readonly preloadedUserScripts: Map; private readonly promptLabel?: string; + private readonly commandResults: CommandExecutionResult[] = []; private buildParams( app: App, plugin: QuickAdd, @@ -161,7 +168,7 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { promptLabel?: string, private readonly originLeaf: WorkspaceLeaf | null = null, ) { - super(app); + super(app, choiceExecutor.getExecutionContext?.() ?? undefined); this.choice = choice; this.plugin = plugin; this.macro = choice?.macro; @@ -176,62 +183,152 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { this.params = this.buildParams(app, plugin, choiceExecutor, sharedVariables); } - async run(): Promise { + async run(): Promise { if (!this.macro || !this.macro.commands) { log.logError( `No commands in the macro for choice '${this.choice.name}'` ); - return; + return this.createMacroResult("skipped"); } - await this.executeCommands(this.macro.commands); + const commandResults = await this.executeCommands(this.macro.commands); + const aborted = commandResults.some((result) => result.status === "aborted"); + return this.createMacroResult(aborted ? "aborted" : "success", commandResults); } public getOutput(): unknown { return this.output; } - protected async executeCommands(commands: ICommand[]) { - try { - for (const command of commands) { - if (command?.type === CommandType.Obsidian) - this.executeObsidianCommand(command as IObsidianCommand); - if (command?.type === CommandType.UserScript) - await this.executeUserScript(command as IUserScript); - if (command?.type === CommandType.Choice) - await this.executeChoice(command as IChoiceCommand); - if (command?.type === CommandType.Wait) { - const waitCommand: IWaitCommand = command as IWaitCommand; - await waitFor(waitCommand.time); - } - if (command?.type === CommandType.NestedChoice) { - await this.executeNestedChoice(command as INestedChoiceCommand); - } - if (command?.type === CommandType.EditorCommand) { - await this.executeEditorCommand(command as IEditorCommand); - } - if (command?.type === CommandType.AIAssistant) { - await this.executeAIAssistant(command as IAIAssistantCommand); - } - if (command?.type === CommandType.OpenFile) { - await this.executeOpenFile(command as IOpenFileCommand); - } - if (command?.type === CommandType.Conditional) { - await this.executeConditional(command as IConditionalCommand); + public getCommandResults(): CommandExecutionResult[] { + return this.commandResults; + } + + protected async executeCommands( + commands: ICommand[], + handleAbort = true, + ): Promise { + const results: CommandExecutionResult[] = []; + + for (const command of commands) { + const stepId = this.createCommandStepId(command); + + try { + const nestedResults = await this.executeCommand(command); + results.push(...nestedResults); + results.push( + createCommandExecutionResult({ + status: "success", + commandId: command.id, + stepId, + value: this.getCommandResultValue(command), + }), + ); + } catch (error) { + if (!handleAbort) throw error; + + if ( + handleMacroAbort(error, { + logPrefix: "Macro execution aborted", + noticePrefix: "Macro execution aborted", + defaultReason: "Macro execution aborted", + }) + ) { + this.choiceExecutor.signalAbort?.(error as MacroAbortError); + results.push( + createCommandExecutionResult({ + status: "aborted", + commandId: command.id, + stepId, + error, + }), + ); + this.commandResults.push(...results); + return results; } + + results.push( + createCommandExecutionResult({ + status: "failed", + commandId: command.id, + stepId, + error, + }), + ); + this.commandResults.push(...results); + throw error; } - } catch (error) { - if ( - handleMacroAbort(error, { - logPrefix: "Macro execution aborted", - noticePrefix: "Macro execution aborted", - defaultReason: "Macro execution aborted", - }) - ) { - this.choiceExecutor.signalAbort?.(error as MacroAbortError); - return; - } - throw error; + } + + if (handleAbort) { + this.commandResults.push(...results); + } + return results; + } + + private createMacroResult( + status: ChoiceExecutionResult["status"], + commandResults: CommandExecutionResult[] = [], + ): ChoiceExecutionResult { + const abortError = commandResults.find( + (result) => result.status === "aborted", + )?.error; + + return createChoiceExecutionResult({ + status, + choiceId: this.choice.id, + stepId: this.getExecutionContext()?.createStepId("macro"), + value: { + output: this.output, + commands: commandResults, + }, + error: status === "aborted" ? abortError : undefined, + artifacts: [...(this.getExecutionContext()?.artifacts ?? [])], + diagnostics: [...(this.getExecutionContext()?.diagnostics ?? [])], + }); + } + + private getCommandResultValue(command: ICommand): unknown { + return command?.type === CommandType.UserScript ? this.output : undefined; + } + + private createCommandStepId(command: ICommand): string { + return this.getExecutionContext()?.createStepId(command?.type ?? "command") ?? + `${this.choice.id}:${command?.id ?? command?.name ?? command?.type ?? "command"}`; + } + + private async executeCommand( + command: ICommand, + ): Promise { + switch (command?.type) { + case CommandType.Obsidian: + this.executeObsidianCommand(command as IObsidianCommand); + return []; + case CommandType.UserScript: + await this.executeUserScript(command as IUserScript); + return []; + case CommandType.Choice: + await this.executeChoice(command as IChoiceCommand); + return []; + case CommandType.Wait: + await waitFor((command as IWaitCommand).time); + return []; + case CommandType.NestedChoice: + await this.executeNestedChoice(command as INestedChoiceCommand); + return []; + case CommandType.EditorCommand: + await this.executeEditorCommand(command as IEditorCommand); + return []; + case CommandType.AIAssistant: + await this.executeAIAssistant(command as IAIAssistantCommand); + return []; + case CommandType.OpenFile: + await this.executeOpenFile(command as IOpenFileCommand); + return []; + case CommandType.Conditional: + return await this.executeConditional(command as IConditionalCommand); + default: + return []; } } @@ -412,11 +509,8 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { return; } - await this.choiceExecutor.execute(targetChoice); - const abort = this.choiceExecutor.consumeAbortSignal?.(); - if (abort) { - throw abort; - } + const result = await this.choiceExecutor.execute(targetChoice); + this.throwIfChildChoiceAborted(result); } private async executeNestedChoice(command: INestedChoiceCommand) { @@ -426,11 +520,21 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { return; } - await this.choiceExecutor.execute(choice); + const result = await this.choiceExecutor.execute(choice); + this.throwIfChildChoiceAborted(result); + } + + private throwIfChildChoiceAborted(result: ChoiceExecutionResult): void { const abort = this.choiceExecutor.consumeAbortSignal?.(); if (abort) { throw abort; } + + if (result.status === "aborted") { + throw result.error instanceof MacroAbortError + ? result.error + : new MacroAbortError("Nested choice aborted"); + } } private async executeEditorCommand(command: IEditorCommand) { @@ -505,7 +609,9 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { const formatter = new CompleteFormatter( this.app, QuickAdd.instance, - this.choiceExecutor + this.choiceExecutor, + undefined, + this.getExecutionContext(), ); const modelProvider = getModelProvider(model.name); @@ -540,7 +646,9 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { } } - private async executeConditional(command: IConditionalCommand) { + private async executeConditional( + command: IConditionalCommand, + ): Promise { const shouldRunThenBranch = await evaluateCondition(command.condition, { variables: this.params.variables, evaluateScriptCondition: async (condition: ScriptCondition) => @@ -552,15 +660,15 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { : command.elseCommands; if (!Array.isArray(branch) || branch.length === 0) { - return; + return []; } - await this.executeCommands(branch); + return await this.executeCommands(branch, false); } - public async runSubset(commands: ICommand[]): Promise { - if (!commands?.length) return; - await this.executeCommands(commands); + public async runSubset(commands: ICommand[]): Promise { + if (!commands?.length) return []; + return await this.executeCommands(commands); } public setOutput(value: unknown): void { @@ -650,7 +758,9 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { const formatter = new CompleteFormatter( this.app, QuickAdd.instance, - this.choiceExecutor + this.choiceExecutor, + undefined, + this.getExecutionContext(), ); const resolvedPath = await formatter.formatFileName(command.filePath, ""); @@ -674,7 +784,7 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { await openFile(this.app, file, { ...openOptions, - originLeaf: this.originLeaf, + originLeaf: this.getExecutionContext()?.originLeaf ?? this.originLeaf, }); } catch (error) { log.logError(`OpenFile: Failed to open file '${command.filePath}': ${error.message}`); diff --git a/src/engine/QuickAddEngine.ts b/src/engine/QuickAddEngine.ts index e5a6ea8b..65df1925 100644 --- a/src/engine/QuickAddEngine.ts +++ b/src/engine/QuickAddEngine.ts @@ -5,6 +5,7 @@ import { log } from "../logger/logManager"; import { withTemplaterFileCreationSuppressed } from "../utilityObsidian"; import { coerceYamlValue } from "../utils/yamlValues"; import { TemplatePropertyCollector } from "../utils/TemplatePropertyCollector"; +import type { ChoiceExecutionContext } from "./runtime"; /** * Configuration for structured variable validation @@ -32,11 +33,18 @@ export abstract class QuickAddEngine { */ private static readonly YAML_FRONTMATTER_EXTENSIONS = ['md']; - protected constructor(app: App) { + protected constructor( + app: App, + protected executionContext?: ChoiceExecutionContext, + ) { this.app = app; } - public abstract run(): void; + protected getExecutionContext(): ChoiceExecutionContext | undefined { + return this.executionContext; + } + + public abstract run(): unknown; /** * Validates structured variables to ensure they can be safely processed. @@ -230,10 +238,22 @@ export abstract class QuickAddEngine { filePath.toLowerCase().endsWith(".md"); return shouldSuppress - ? await withTemplaterFileCreationSuppressed(this.app, filePath, createFile) + ? await this.withTemplaterFileCreationSuppressed(filePath, createFile) : await createFile(); } + private async withTemplaterFileCreationSuppressed( + filePath: string, + fn: () => Promise, + ): Promise { + const templater = this.executionContext?.integrations.templater; + if (templater) { + return await templater.withFileCreationSuppressed(filePath, fn); + } + + return await withTemplaterFileCreationSuppressed(this.app, filePath, fn); + } + /** * Determines if a file's front matter should be post-processed for template property types. * Only processes files with supported extensions (Markdown) when template variables are present. diff --git a/src/engine/SingleTemplateEngine.ts b/src/engine/SingleTemplateEngine.ts index 0f7b35b7..6f92dfff 100644 --- a/src/engine/SingleTemplateEngine.ts +++ b/src/engine/SingleTemplateEngine.ts @@ -3,15 +3,17 @@ import type { App } from "obsidian"; import type QuickAdd from "../main"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import { log } from "../logger/logManager"; +import type { ChoiceExecutionContext } from "./runtime"; export class SingleTemplateEngine extends TemplateEngine { constructor( app: App, plugin: QuickAdd, private templatePath: string, - choiceExecutor?: IChoiceExecutor + choiceExecutor?: IChoiceExecutor, + executionContext?: ChoiceExecutionContext, ) { - super(app, plugin, choiceExecutor); + super(app, plugin, choiceExecutor, executionContext); } public async run(): Promise { let templateContent: string = await this.getTemplateContent( diff --git a/src/engine/StartupMacroEngine.test.ts b/src/engine/StartupMacroEngine.test.ts new file mode 100644 index 00000000..01473228 --- /dev/null +++ b/src/engine/StartupMacroEngine.test.ts @@ -0,0 +1,101 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { App } from "obsidian"; +import { MacroAbortError } from "../errors/MacroAbortError"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; +import type IChoice from "../types/choices/IChoice"; +import type IMacroChoice from "../types/choices/IMacroChoice"; +import { StartupMacroEngine } from "./StartupMacroEngine"; +import { createChoiceExecutionResult } from "./runtime"; + +const { + mockMacroRun, + mockMacroConstructor, + mockLogWarning, +} = vi.hoisted(() => ({ + mockMacroRun: vi.fn(), + mockMacroConstructor: vi.fn(), + mockLogWarning: vi.fn(), +})); + +vi.mock("./MacroChoiceEngine", () => ({ + MacroChoiceEngine: class MacroChoiceEngineMock { + constructor(...args: unknown[]) { + mockMacroConstructor(args); + } + + async run() { + return await mockMacroRun(); + } + }, +})); + +vi.mock("../logger/logManager", () => ({ + log: { + logWarning: mockLogWarning, + }, +})); + +function createStartupMacroChoice(id: string, name: string): IMacroChoice { + return { + id, + name, + type: "Macro", + command: false, + runOnStartup: true, + macro: { + id: `${id}-macro`, + name: `${name} macro`, + commands: [], + }, + }; +} + +describe("StartupMacroEngine", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("stops running remaining startup macros after an aborted macro", async () => { + const firstChoice = createStartupMacroChoice("first", "First startup macro"); + const secondChoice = createStartupMacroChoice("second", "Second startup macro"); + const abort = new MacroAbortError("Input cancelled by user"); + const choiceExecutor: IChoiceExecutor = { + variables: new Map(), + execute: vi.fn(), + consumeAbortSignal: vi + .fn<() => MacroAbortError | null>() + .mockReturnValueOnce(abort) + .mockReturnValue(null), + }; + + mockMacroRun + .mockResolvedValueOnce( + createChoiceExecutionResult({ + status: "aborted", + choiceId: firstChoice.id, + error: abort, + }), + ) + .mockResolvedValueOnce( + createChoiceExecutionResult({ + status: "success", + choiceId: secondChoice.id, + }), + ); + + const engine = new StartupMacroEngine( + {} as App, + {} as any, + [firstChoice as IChoice, secondChoice as IChoice], + choiceExecutor, + ); + + await engine.run(); + + expect(mockMacroRun).toHaveBeenCalledTimes(1); + expect(choiceExecutor.consumeAbortSignal).toHaveBeenCalledTimes(1); + expect(mockLogWarning).toHaveBeenCalledWith( + expect.stringContaining(firstChoice.name), + ); + }); +}); diff --git a/src/engine/StartupMacroEngine.ts b/src/engine/StartupMacroEngine.ts index 936d2161..96703d4b 100644 --- a/src/engine/StartupMacroEngine.ts +++ b/src/engine/StartupMacroEngine.ts @@ -5,6 +5,7 @@ import type { IChoiceExecutor } from "../IChoiceExecutor"; import type IChoice from "../types/choices/IChoice"; import type IMacroChoice from "../types/choices/IMacroChoice"; import { flattenChoices } from "../utils/choiceUtils"; +import { log } from "../logger/logManager"; export class StartupMacroEngine { constructor( @@ -19,13 +20,21 @@ export class StartupMacroEngine { .filter((c): c is IMacroChoice => c.type === "Macro" && (c as IMacroChoice).runOnStartup); for (const choice of macroChoices) { - await new MacroChoiceEngine( + const result = await new MacroChoiceEngine( this.app, this.plugin, choice, this.choiceExecutor, new Map() ).run(); + + const abort = this.choiceExecutor.consumeAbortSignal?.(); + if (result.status === "aborted" || abort) { + log.logWarning( + `Startup macro '${choice.name}' aborted; skipping remaining startup macros.`, + ); + return; + } } } } diff --git a/src/engine/TemplateChoiceEngine.ts b/src/engine/TemplateChoiceEngine.ts index 797c558a..9824c38d 100644 --- a/src/engine/TemplateChoiceEngine.ts +++ b/src/engine/TemplateChoiceEngine.ts @@ -18,7 +18,6 @@ import { normalizeAppendLinkOptions } from "../types/linkPlacement"; import { getAllFolderPathsInVault, insertFileLinkToActiveView, - jumpToNextTemplaterCursorIfPossible, openExistingFileTab, openFile, } from "../utilityObsidian"; @@ -27,6 +26,7 @@ import { normalizeFileOpening } from "../utils/fileOpeningDefaults"; import { TemplateEngine } from "./TemplateEngine"; import { MacroAbortError } from "../errors/MacroAbortError"; import { handleMacroAbort } from "../utils/macroAbortHandler"; +import type { ChoiceExecutionContext } from "./runtime"; export class TemplateChoiceEngine extends TemplateEngine { public choice: ITemplateChoice; @@ -38,8 +38,9 @@ export class TemplateChoiceEngine extends TemplateEngine { choice: ITemplateChoice, choiceExecutor: IChoiceExecutor, private readonly originLeaf: WorkspaceLeaf | null = null, + executionContext?: ChoiceExecutionContext, ) { - super(app, plugin, choiceExecutor); + super(app, plugin, choiceExecutor, executionContext); this.choiceExecutor = choiceExecutor; this.choice = choice; } @@ -149,11 +150,13 @@ export class TemplateChoiceEngine extends TemplateEngine { if (!openExistingTab) { await openFile(this.app, createdFile, { ...fileOpening, - originLeaf: this.originLeaf, + originLeaf: this.getOriginLeaf(), }); } - await jumpToNextTemplaterCursorIfPossible(this.app, createdFile); + await this.formatOrchestrator.jumpToNextTemplaterCursorIfPossible( + createdFile, + ); } } catch (err) { if ( @@ -170,6 +173,10 @@ export class TemplateChoiceEngine extends TemplateEngine { } } + private getOriginLeaf(): WorkspaceLeaf | null { + return this.getExecutionContext()?.originLeaf ?? this.originLeaf; + } + private async getSelectedFileExistsMode(): Promise { this.choice.fileExistsBehavior ??= { kind: "prompt" }; diff --git a/src/engine/TemplateEngine.ts b/src/engine/TemplateEngine.ts index 7453487a..d7f20859 100644 --- a/src/engine/TemplateEngine.ts +++ b/src/engine/TemplateEngine.ts @@ -4,11 +4,6 @@ import type { LinkToCurrentFileBehavior } from "../formatters/formatter"; import type { App } from "obsidian"; import { Notice, TFile } from "obsidian"; import type QuickAdd from "../main"; -import { - getTemplater, - overwriteTemplaterOnce, - templaterParseTemplate, -} from "../utilityObsidian"; import GenericSuggester from "../gui/GenericSuggester/genericSuggester"; import InputSuggester from "../gui/InputSuggester/inputSuggester"; import { @@ -28,6 +23,8 @@ import { MacroAbortError } from "../errors/MacroAbortError"; import { isCancellationError } from "../utils/errorUtils"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import { log } from "../logger/logManager"; +import type { ChoiceExecutionContext } from "./runtime"; +import { FormatOrchestrator } from "./runtime"; type FolderChoiceOptions = { allowCreate?: boolean; @@ -76,16 +73,24 @@ function isMacroAbortError(error: unknown): error is MacroAbortError { export abstract class TemplateEngine extends QuickAddEngine { protected formatter: CompleteFormatter; - protected readonly templater; + protected formatOrchestrator: FormatOrchestrator; protected constructor( app: App, protected plugin: QuickAdd, - choiceFormatter?: IChoiceExecutor + choiceFormatter?: IChoiceExecutor, + executionContext?: ChoiceExecutionContext, ) { - super(app); - this.templater = getTemplater(app); - this.formatter = new CompleteFormatter(app, plugin, choiceFormatter); + const context = executionContext ?? choiceFormatter?.getExecutionContext?.() ?? undefined; + super(app, context); + this.formatOrchestrator = new FormatOrchestrator(app, context); + this.formatter = new CompleteFormatter( + app, + plugin, + choiceFormatter, + undefined, + context, + ); } public abstract run(): @@ -507,7 +512,9 @@ export abstract class TemplateEngine extends QuickAddEngine { } // Process Templater commands for template choices - await overwriteTemplaterOnce(this.app, createdFile); + await this.formatOrchestrator.overwriteTemplaterOnce(createdFile, { + diagnoseMissingCapability: formattedTemplateContent.includes("<%"), + }); return createdFile; } catch (err) { @@ -559,7 +566,9 @@ export abstract class TemplateEngine extends QuickAddEngine { } // Process Templater commands - await overwriteTemplaterOnce(this.app, file); + await this.formatOrchestrator.overwriteTemplaterOnce(file, { + diagnoseMissingCapability: formattedTemplateContent.includes("<%"), + }); return file; } catch (err) { @@ -588,11 +597,11 @@ export abstract class TemplateEngine extends QuickAddEngine { let formattedTemplateContent: string = await this.formatter.formatFileContent(templateContent); if (file.extension === "md") { - formattedTemplateContent = await templaterParseTemplate( - this.app, - formattedTemplateContent, - file, - ); + formattedTemplateContent = + await this.formatOrchestrator.parseTemplaterTemplate( + formattedTemplateContent, + file, + ); } const fileContent: string = await this.app.vault.cachedRead(file); const newFileContent: string = diff --git a/src/engine/runtime/FormatOrchestrator.test.ts b/src/engine/runtime/FormatOrchestrator.test.ts new file mode 100644 index 00000000..84fdc8b6 --- /dev/null +++ b/src/engine/runtime/FormatOrchestrator.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it, vi } from "vitest"; +import type { App, TFile } from "obsidian"; +import { FormatOrchestrator } from "./FormatOrchestrator"; +import { createChoiceExecutionContext } from "./context"; +import { IntegrationRegistry } from "../../integrations/IntegrationRegistry"; +import type { TemplaterIntegration } from "../../integrations/TemplaterIntegration"; + +function createFile(path = "Notes/Test.md", extension = "md"): TFile { + return { + path, + extension, + } as TFile; +} + +function createTemplater( + capabilities: Partial> = {}, +): TemplaterIntegration { + return { + id: "templater-obsidian", + getRawPlugin: vi.fn(() => null), + getPlugin: vi.fn(() => null), + getCapabilityReport: vi.fn(() => ({ + pluginId: "templater-obsidian", + installed: true, + capabilities: {} as any, + missingCapabilities: [], + })), + hasCapability: vi.fn( + (capability: string) => capabilities[capability] === true, + ), + isTriggerOnCreateEnabled: vi.fn(() => false), + waitForTriggerOnCreateToComplete: vi.fn(), + withFileCreationSuppressed: vi.fn(async (_filePath, fn) => await fn()), + overwriteFileOnce: vi.fn(), + parseTemplate: vi.fn(async (content: string) => content), + jumpToNextCursorIfPossible: vi.fn(), + } as unknown as TemplaterIntegration; +} + +function createOrchestrator(templater: TemplaterIntegration) { + const context = createChoiceExecutionContext({ + integrations: new IntegrationRegistry({ templater }), + }); + return { + context, + orchestrator: new FormatOrchestrator({} as App, context), + }; +} + +describe("FormatOrchestrator Templater diagnostics", () => { + it("does not diagnose missing parseTemplate for plain content", async () => { + const templater = createTemplater({ parseTemplate: false }); + const { context, orchestrator } = createOrchestrator(templater); + + await orchestrator.parseTemplaterTemplate("Plain content", createFile()); + + expect(context.diagnostics).toHaveLength(0); + expect(templater.parseTemplate).toHaveBeenCalledWith( + "Plain content", + expect.objectContaining({ path: "Notes/Test.md" }), + ); + }); + + it("diagnoses missing parseTemplate when markdown content has Templater tags", async () => { + const templater = createTemplater({ parseTemplate: false }); + const { context, orchestrator } = createOrchestrator(templater); + + await orchestrator.parseTemplaterTemplate("<% tp.date.now() %>", createFile()); + + expect(context.diagnostics).toEqual([ + expect.objectContaining({ + code: "templater-capability-missing", + details: expect.objectContaining({ capability: "parseTemplate" }), + }), + ]); + }); + + it("does not diagnose missing overwriteFileCommands when caller disables diagnostics", async () => { + const templater = createTemplater({ overwriteFileCommands: false }); + const { context, orchestrator } = createOrchestrator(templater); + + await orchestrator.overwriteTemplaterOnce(createFile(), { + diagnoseMissingCapability: false, + }); + + expect(context.diagnostics).toHaveLength(0); + expect(templater.overwriteFileOnce).toHaveBeenCalledWith( + expect.objectContaining({ path: "Notes/Test.md" }), + {}, + ); + }); + + it("does not diagnose missing triggerOnFileCreation for best-effort waits", async () => { + const templater = createTemplater({ triggerOnFileCreation: false }); + const { context, orchestrator } = createOrchestrator(templater); + + await orchestrator.waitForTemplaterTriggerOnCreateToComplete(createFile()); + + expect(context.diagnostics).toHaveLength(0); + expect(templater.waitForTriggerOnCreateToComplete).toHaveBeenCalledWith( + expect.objectContaining({ path: "Notes/Test.md" }), + ); + }); + + it("does not diagnose missing cursorJump for best-effort cursor jumps", async () => { + const templater = createTemplater({ cursorJump: false }); + const { context, orchestrator } = createOrchestrator(templater); + + await orchestrator.jumpToNextTemplaterCursorIfPossible(createFile()); + + expect(context.diagnostics).toHaveLength(0); + expect(templater.jumpToNextCursorIfPossible).toHaveBeenCalledWith( + expect.objectContaining({ path: "Notes/Test.md" }), + ); + }); + + it("diagnoses missing overwriteFileCommands by default for markdown files", async () => { + const templater = createTemplater({ overwriteFileCommands: false }); + const { context, orchestrator } = createOrchestrator(templater); + + await orchestrator.overwriteTemplaterOnce(createFile()); + + expect(context.diagnostics).toEqual([ + expect.objectContaining({ + code: "templater-capability-missing", + details: expect.objectContaining({ + capability: "overwriteFileCommands", + }), + }), + ]); + }); +}); diff --git a/src/engine/runtime/FormatOrchestrator.ts b/src/engine/runtime/FormatOrchestrator.ts new file mode 100644 index 00000000..71ecc58b --- /dev/null +++ b/src/engine/runtime/FormatOrchestrator.ts @@ -0,0 +1,140 @@ +import type { App, TFile } from "obsidian"; +import { getIntegrationRegistry } from "../../integrations/IntegrationRegistry"; +import type { IntegrationRegistry } from "../../integrations/IntegrationRegistry"; +import type { ChoiceExecutionContext } from "./context"; +import { createDiagnostic } from "./diagnostic"; + +export interface CaptureRunPlan { + choiceId?: string; + targetPath: string; + action: string; + fileAlreadyExists: boolean; + createWithTemplate: boolean; + templaterPolicy: + | "parse-capture" + | "render-whole-file" + | "wait-for-on-create" + | "none"; +} + +export interface CaptureRunResult { + choiceId?: string; + filePath: string; + action: string; + wasNewFile: boolean; +} + +export interface TemplaterOverwriteOptions { + skipIfNoTags?: boolean; + postWait?: boolean; + diagnoseMissingCapability?: boolean; +} + +export class FormatOrchestrator { + constructor( + private readonly app: App, + private readonly context?: ChoiceExecutionContext, + ) {} + + private get integrations(): IntegrationRegistry { + return this.context?.integrations ?? getIntegrationRegistry(this.app); + } + + async parseTemplaterTemplate( + content: string, + file: TFile, + ): Promise { + const templater = this.integrations.templater; + + const shouldDiagnoseMissingCapability = + file.extension === "md" && content.includes("<%"); + if ( + shouldDiagnoseMissingCapability && + !templater.hasCapability("parseTemplate") + ) { + this.addMissingTemplaterCapabilityDiagnostic("parseTemplate", file); + } + + return await templater.parseTemplate(content, file); + } + + async overwriteTemplaterOnce( + file: TFile, + options: TemplaterOverwriteOptions = {}, + ): Promise { + const templater = this.integrations.templater; + const { diagnoseMissingCapability = true, ...templaterOptions } = options; + + if ( + file.extension === "md" && + diagnoseMissingCapability && + !templater.hasCapability("overwriteFileCommands") + ) { + this.addMissingTemplaterCapabilityDiagnostic( + "overwriteFileCommands", + file, + ); + } + + await templater.overwriteFileOnce(file, templaterOptions); + } + + isTemplaterTriggerOnCreateEnabled(): boolean { + return this.integrations.templater.isTriggerOnCreateEnabled(); + } + + async waitForTemplaterTriggerOnCreateToComplete( + file: TFile, + ): Promise { + await this.integrations.templater.waitForTriggerOnCreateToComplete(file); + } + + async jumpToNextTemplaterCursorIfPossible(file: TFile): Promise { + await this.integrations.templater.jumpToNextCursorIfPossible(file); + } + + recordCapturePlan(plan: CaptureRunPlan): void { + this.context?.addArtifact({ + id: this.context.createStepId("capture-plan"), + kind: "custom", + label: "Capture run plan", + path: plan.targetPath, + value: plan, + createdAt: Date.now(), + }); + } + + recordCaptureResult(result: CaptureRunResult): void { + this.context?.addArtifact({ + id: this.context.createStepId("capture-result"), + kind: "file", + label: "Capture target", + path: result.filePath, + value: result, + metadata: { + action: result.action, + wasNewFile: result.wasNewFile, + }, + createdAt: Date.now(), + }); + } + + private addMissingTemplaterCapabilityDiagnostic( + capability: string, + file: TFile, + ): void { + this.context?.addDiagnostic( + createDiagnostic({ + severity: "info", + code: "templater-capability-missing", + message: `Templater capability '${capability}' is unavailable for ${file.path}.`, + source: "integration", + integrationId: "templater-obsidian", + details: { + capability, + filePath: file.path, + }, + }), + ); + } +} diff --git a/src/engine/runtime/artifact.ts b/src/engine/runtime/artifact.ts new file mode 100644 index 00000000..55830b34 --- /dev/null +++ b/src/engine/runtime/artifact.ts @@ -0,0 +1,27 @@ +export type ChoiceExecutionArtifactKind = + | "file" + | "content" + | "variable" + | "integration" + | "custom"; + +export interface ChoiceExecutionArtifact { + id: string; + kind: ChoiceExecutionArtifactKind; + label?: string; + path?: string; + value?: unknown; + metadata?: Record; + createdAt: number; +} + +export function createArtifact( + artifact: Omit & { + createdAt?: number; + }, +): ChoiceExecutionArtifact { + return { + ...artifact, + createdAt: artifact.createdAt ?? Date.now(), + }; +} diff --git a/src/engine/runtime/context.ts b/src/engine/runtime/context.ts new file mode 100644 index 00000000..380b7f3d --- /dev/null +++ b/src/engine/runtime/context.ts @@ -0,0 +1,65 @@ +import type { WorkspaceLeaf } from "obsidian"; +import type { IntegrationRegistry } from "../../integrations/IntegrationRegistry"; +import type { ChoiceExecutionArtifact } from "./artifact"; +import type { ChoiceExecutionDiagnostic } from "./diagnostic"; + +export type ExecutionStepId = string; + +export interface ChoiceExecutionContext { + id: string; + rootChoiceId?: string; + originLeaf?: WorkspaceLeaf | null; + variables: Map; + integrations: IntegrationRegistry; + diagnostics: ChoiceExecutionDiagnostic[]; + artifacts: ChoiceExecutionArtifact[]; + createStepId(label?: string): ExecutionStepId; + addDiagnostic(diagnostic: ChoiceExecutionDiagnostic): void; + addArtifact(artifact: ChoiceExecutionArtifact): void; +} + +export interface ChoiceExecutionContextOptions { + id?: string; + rootChoiceId?: string; + originLeaf?: WorkspaceLeaf | null; + variables?: Map; + integrations: IntegrationRegistry; + diagnostics?: ChoiceExecutionDiagnostic[]; + artifacts?: ChoiceExecutionArtifact[]; +} + +export function createChoiceExecutionContext( + options: ChoiceExecutionContextOptions, +): ChoiceExecutionContext { + let nextStep = 0; + const diagnostics = options.diagnostics ?? []; + const artifacts = options.artifacts ?? []; + const id = options.id ?? createRuntimeId("ctx"); + + return { + id, + rootChoiceId: options.rootChoiceId, + originLeaf: options.originLeaf, + variables: options.variables ?? new Map(), + integrations: options.integrations, + diagnostics, + artifacts, + createStepId(label = "step") { + nextStep += 1; + return `${id}:${label}:${nextStep}`; + }, + addDiagnostic(diagnostic) { + diagnostics.push(diagnostic); + }, + addArtifact(artifact) { + artifacts.push(artifact); + }, + }; +} + +function createRuntimeId(prefix: string): string { + const cryptoLike = globalThis.crypto as + | { randomUUID?: () => string } + | undefined; + return cryptoLike?.randomUUID?.() ?? `${prefix}-${Date.now()}`; +} diff --git a/src/engine/runtime/diagnostic.ts b/src/engine/runtime/diagnostic.ts new file mode 100644 index 00000000..d781a068 --- /dev/null +++ b/src/engine/runtime/diagnostic.ts @@ -0,0 +1,26 @@ +export type RuntimeDiagnosticSeverity = "info" | "warning" | "error"; + +export type RuntimeDiagnosticSource = + | "runtime" + | "integration" + | "choice" + | "command" + | "formatter"; + +export interface ChoiceExecutionDiagnostic { + severity: RuntimeDiagnosticSeverity; + code: string; + message: string; + source: RuntimeDiagnosticSource; + stepId?: string; + choiceId?: string; + integrationId?: string; + details?: Record; + cause?: unknown; +} + +export function createDiagnostic( + diagnostic: ChoiceExecutionDiagnostic, +): ChoiceExecutionDiagnostic { + return diagnostic; +} diff --git a/src/engine/runtime/index.ts b/src/engine/runtime/index.ts new file mode 100644 index 00000000..2a3d8d52 --- /dev/null +++ b/src/engine/runtime/index.ts @@ -0,0 +1,5 @@ +export * from "./artifact"; +export * from "./context"; +export * from "./diagnostic"; +export * from "./FormatOrchestrator"; +export * from "./result"; diff --git a/src/engine/runtime/result.ts b/src/engine/runtime/result.ts new file mode 100644 index 00000000..1507aaa8 --- /dev/null +++ b/src/engine/runtime/result.ts @@ -0,0 +1,51 @@ +import type { ChoiceExecutionArtifact } from "./artifact"; +import type { ChoiceExecutionDiagnostic } from "./diagnostic"; + +export type ChoiceExecutionStatus = "success" | "skipped" | "aborted" | "failed"; +export type CommandExecutionStatus = ChoiceExecutionStatus; + +export interface ChoiceExecutionResult { + status: ChoiceExecutionStatus; + choiceId?: string; + stepId?: string; + value?: unknown; + artifacts: ChoiceExecutionArtifact[]; + diagnostics: ChoiceExecutionDiagnostic[]; + error?: unknown; +} + +export interface CommandExecutionResult { + status: CommandExecutionStatus; + commandId?: string; + stepId: string; + value?: unknown; + artifacts: ChoiceExecutionArtifact[]; + diagnostics: ChoiceExecutionDiagnostic[]; + error?: unknown; +} + +export function createChoiceExecutionResult( + result: Omit & { + artifacts?: ChoiceExecutionArtifact[]; + diagnostics?: ChoiceExecutionDiagnostic[]; + }, +): ChoiceExecutionResult { + return { + ...result, + artifacts: result.artifacts ?? [], + diagnostics: result.diagnostics ?? [], + }; +} + +export function createCommandExecutionResult( + result: Omit & { + artifacts?: ChoiceExecutionArtifact[]; + diagnostics?: ChoiceExecutionDiagnostic[]; + }, +): CommandExecutionResult { + return { + ...result, + artifacts: result.artifacts ?? [], + diagnostics: result.diagnostics ?? [], + }; +} diff --git a/src/formatters/captureChoiceFormatter.ts b/src/formatters/captureChoiceFormatter.ts index 404a93b7..eb8ab488 100644 --- a/src/formatters/captureChoiceFormatter.ts +++ b/src/formatters/captureChoiceFormatter.ts @@ -1,4 +1,4 @@ -import { MarkdownView, type TFile } from "obsidian"; +import { MarkdownView, type App, type TFile } from "obsidian"; import { getLinesInString } from "src/utility"; import { CREATE_IF_NOT_FOUND_BOTTOM, @@ -8,15 +8,18 @@ import { import { log } from "../logger/logManager"; import type ICaptureChoice from "../types/choices/ICaptureChoice"; import type { BlankLineAfterMatchMode } from "../types/choices/ICaptureChoice"; -import { templaterParseTemplate } from "../utilityObsidian"; import { reportError } from "../utils/errorUtils"; import { ChoiceAbortError } from "../errors/ChoiceAbortError"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; +import type QuickAdd from "../main"; +import { FormatOrchestrator, type ChoiceExecutionContext } from "../engine/runtime"; import { CompleteFormatter } from "./completeFormatter"; import getEndOfSection from "./helpers/getEndOfSection"; import { findYamlFrontMatterRange } from "../utils/yamlContext"; export class CaptureChoiceFormatter extends CompleteFormatter { private choice: ICaptureChoice; + private readonly formatOrchestrator: FormatOrchestrator; private file: TFile | null = null; private fileContent = ""; private sourcePath: string | null = null; @@ -29,6 +32,19 @@ export class CaptureChoiceFormatter extends CompleteFormatter { */ private templaterProcessed = false; + constructor( + app: App, + plugin: QuickAdd, + choiceExecutor?: IChoiceExecutor, + executionContext?: ChoiceExecutionContext, + ) { + super(app, plugin, choiceExecutor, undefined, executionContext); + this.formatOrchestrator = new FormatOrchestrator( + app, + this.executionContext, + ); + } + public setDestinationFile(file: TFile): void { this.file = file; this.sourcePath = file.path; @@ -107,11 +123,11 @@ export class CaptureChoiceFormatter extends CompleteFormatter { // Run templater only once per capture payload to prevent #533 double execution if (runTemplater && this.file && !this.templaterProcessed) { - const templaterFormatted = await templaterParseTemplate( - this.app, - formatted, - this.file, - ); + const templaterFormatted = + await this.formatOrchestrator.parseTemplaterTemplate( + formatted, + this.file, + ); if (templaterFormatted) { formatted = templaterFormatted; } diff --git a/src/formatters/completeFormatter.ts b/src/formatters/completeFormatter.ts index 90171cbf..61fefec7 100644 --- a/src/formatters/completeFormatter.ts +++ b/src/formatters/completeFormatter.ts @@ -27,6 +27,7 @@ import { FieldValueProcessor } from "../utils/FieldValueProcessor"; import { Formatter, type PromptContext } from "./formatter"; import { MacroAbortError } from "../errors/MacroAbortError"; import { isCancellationError } from "../utils/errorUtils"; +import type { ChoiceExecutionContext } from "../engine/runtime"; export class CompleteFormatter extends Formatter { private valueHeader: string; @@ -36,9 +37,12 @@ export class CompleteFormatter extends Formatter { private plugin: QuickAdd, protected choiceExecutor?: IChoiceExecutor, dateParser?: IDateParser, + protected executionContext?: ChoiceExecutionContext, ) { super(app); this.dateParser = dateParser || NLDParser; + this.executionContext ??= + choiceExecutor?.getExecutionContext?.() ?? undefined; if (choiceExecutor) { this.variables = choiceExecutor?.variables; } @@ -372,6 +376,7 @@ export class CompleteFormatter extends Formatter { this.plugin, templatePath, this.choiceExecutor, + this.executionContext, ).run(); } diff --git a/src/integrations/IntegrationRegistry.ts b/src/integrations/IntegrationRegistry.ts new file mode 100644 index 00000000..a23a7d4c --- /dev/null +++ b/src/integrations/IntegrationRegistry.ts @@ -0,0 +1,47 @@ +import type { App } from "obsidian"; +import { + createTemplaterIntegration, + NoopTemplaterIntegration, + type TemplaterIntegration, +} from "./TemplaterIntegration"; + +export interface IntegrationRegistryOptions { + templater?: TemplaterIntegration; +} + +export class IntegrationRegistry { + templater: TemplaterIntegration; + + constructor(options: IntegrationRegistryOptions = {}) { + this.templater = options.templater ?? new NoopTemplaterIntegration(); + } + + registerTemplater(templater: TemplaterIntegration): void { + this.templater = templater; + } +} + +const registriesByApp = new WeakMap(); + +export function createIntegrationRegistry(app: App): IntegrationRegistry { + return new IntegrationRegistry({ + templater: createTemplaterIntegration(app), + }); +} + +export function registerIntegrationRegistry( + app: App, + registry: IntegrationRegistry, +): IntegrationRegistry { + registriesByApp.set(app, registry); + return registry; +} + +export function getIntegrationRegistry(app: App): IntegrationRegistry { + let registry = registriesByApp.get(app); + if (!registry) { + registry = createIntegrationRegistry(app); + registerIntegrationRegistry(app, registry); + } + return registry; +} diff --git a/src/integrations/TemplaterIntegration.test.ts b/src/integrations/TemplaterIntegration.test.ts new file mode 100644 index 00000000..59a65587 --- /dev/null +++ b/src/integrations/TemplaterIntegration.test.ts @@ -0,0 +1,76 @@ +import { App, TFile } from "obsidian"; +import { describe, expect, it, vi } from "vitest"; +import { + createTemplaterIntegration, + TEMPLATER_PLUGIN_ID, +} from "./TemplaterIntegration"; + +function createMarkdownFile(path = "target.md"): TFile { + const file = new TFile(); + file.path = path; + file.extension = "md"; + return file; +} + +describe("TemplaterIntegration", () => { + it("reports missing plugin and no-ops optional operations", async () => { + const app = new App(); + const file = createMarkdownFile(); + const integration = createTemplaterIntegration(app as any); + + const report = integration.getCapabilityReport(); + expect(report.installed).toBe(false); + expect(report.missingCapabilities).toContain("parseTemplate"); + expect(integration.hasCapability("parseTemplate")).toBe(false); + expect(integration.isTriggerOnCreateEnabled()).toBe(false); + expect(await integration.parseTemplate("hello", file)).toBe("hello"); + expect( + await integration.withFileCreationSuppressed("target.md", async () => 42), + ).toBe(42); + + await expect(integration.overwriteFileOnce(file)).resolves.toBeUndefined(); + await expect( + integration.waitForTriggerOnCreateToComplete(file), + ).resolves.toBeUndefined(); + await expect( + integration.jumpToNextCursorIfPossible(file), + ).resolves.toBeUndefined(); + }); + + it("reports missing capabilities without crashing legacy-safe operations", async () => { + const app = new App(); + const file = createMarkdownFile(); + const read = vi.fn(async () => "<% tp.file.title %>"); + (app as any).vault.read = read; + (app as any).plugins.plugins[TEMPLATER_PLUGIN_ID] = { + settings: { + trigger_on_file_creation: true, + auto_jump_to_cursor: true, + }, + templater: {}, + editor_handler: {}, + }; + + const integration = createTemplaterIntegration(app as any); + const report = integration.getCapabilityReport(); + + expect(report.installed).toBe(true); + expect(report.capabilities.triggerOnFileCreation).toBe(true); + expect(report.capabilities.parseTemplate).toBe(false); + expect(report.capabilities.overwriteFileCommands).toBe(false); + expect(report.capabilities.pendingTemplates).toBe(false); + expect(report.capabilities.cursorJump).toBe(false); + expect(report.missingCapabilities).toEqual( + expect.arrayContaining([ + "parseTemplate", + "overwriteFileCommands", + "pendingTemplates", + "cursorJump", + ]), + ); + expect(await integration.parseTemplate("hello", file)).toBe("hello"); + + await integration.overwriteFileOnce(file); + expect(read).not.toHaveBeenCalled(); + }); +}); diff --git a/src/integrations/TemplaterIntegration.ts b/src/integrations/TemplaterIntegration.ts new file mode 100644 index 00000000..3cf78d6c --- /dev/null +++ b/src/integrations/TemplaterIntegration.ts @@ -0,0 +1,592 @@ +import type { App, TFile } from "obsidian"; +import { log } from "../logger/logManager"; +import { reportError } from "../utils/errorUtils"; + +export const TEMPLATER_PLUGIN_ID = "templater-obsidian"; + +export type TemplaterPluginLike = { + settings?: { + trigger_on_file_creation?: boolean; + auto_jump_to_cursor?: boolean; + }; + templater?: { + overwrite_file_commands?: (f: TFile) => Promise; + parse_template?: ( + opt: { + target_file: TFile; + run_mode: number; + frontmatter?: Record; + }, + content: string, + ) => Promise; + create_running_config?: ( + template_file: TFile | undefined, + target_file: TFile, + run_mode: number, + ) => { + target_file: TFile; + run_mode: number; + frontmatter: Record; + }; + files_with_pending_templates?: Set; + functions_generator?: { teardown?: () => Promise }; + }; + editor_handler?: { + plugin?: unknown; + jump_to_next_cursor_location?: ( + file?: TFile | null, + auto_jump?: boolean, + ) => Promise; + }; +}; + +export type TemplaterCapability = + | "triggerOnFileCreation" + | "pendingTemplates" + | "overwriteFileCommands" + | "parseTemplate" + | "createRunningConfig" + | "cursorAutoJump" + | "cursorJump" + | "teardown"; + +export type TemplaterCapabilityMap = Record; + +export interface TemplaterCapabilityReport { + pluginId: typeof TEMPLATER_PLUGIN_ID; + installed: boolean; + capabilities: TemplaterCapabilityMap; + missingCapabilities: TemplaterCapability[]; +} + +export interface TemplaterIntegration { + readonly id: typeof TEMPLATER_PLUGIN_ID; + getRawPlugin(): unknown | null; + getPlugin(): TemplaterPluginLike | null; + getCapabilityReport(): TemplaterCapabilityReport; + hasCapability(capability: TemplaterCapability): boolean; + isTriggerOnCreateEnabled(): boolean; + waitForTriggerOnCreateToComplete( + file: TFile, + opts?: { timeoutMs?: number; appearTimeoutMs?: number }, + ): Promise; + withFileCreationSuppressed( + filePath: string, + fn: () => Promise, + ): Promise; + overwriteFileOnce( + file: TFile, + opts?: { skipIfNoTags?: boolean; postWait?: boolean }, + ): Promise; + parseTemplate( + templateContent: string, + targetFile: TFile, + ): Promise; + jumpToNextCursorIfPossible(file: TFile): Promise; +} + +const ALL_CAPABILITIES: TemplaterCapability[] = [ + "triggerOnFileCreation", + "pendingTemplates", + "overwriteFileCommands", + "parseTemplate", + "createRunningConfig", + "cursorAutoJump", + "cursorJump", + "teardown", +]; + +const NO_CAPABILITIES = ALL_CAPABILITIES.reduce( + (capabilities, capability) => { + capabilities[capability] = false; + return capabilities; + }, + {} as TemplaterCapabilityMap, +); + +function createReport( + installed: boolean, + capabilities: TemplaterCapabilityMap, +): TemplaterCapabilityReport { + return { + pluginId: TEMPLATER_PLUGIN_ID, + installed, + capabilities, + missingCapabilities: ALL_CAPABILITIES.filter( + (capability) => !capabilities[capability], + ), + }; +} + +export class NoopTemplaterIntegration implements TemplaterIntegration { + readonly id = TEMPLATER_PLUGIN_ID; + + getRawPlugin(): null { + return null; + } + + getPlugin(): null { + return null; + } + + getCapabilityReport(): TemplaterCapabilityReport { + return createReport(false, { ...NO_CAPABILITIES }); + } + + hasCapability(_capability: TemplaterCapability): boolean { + return false; + } + + isTriggerOnCreateEnabled(): boolean { + return false; + } + + async waitForTriggerOnCreateToComplete(): Promise { + return; + } + + async withFileCreationSuppressed( + _filePath: string, + fn: () => Promise, + ): Promise { + return await fn(); + } + + async overwriteFileOnce(): Promise { + return; + } + + async parseTemplate( + templateContent: string, + _targetFile: TFile, + ): Promise { + return templateContent; + } + + async jumpToNextCursorIfPossible(): Promise { + return; + } +} + +export class ObsidianTemplaterIntegration implements TemplaterIntegration { + readonly id = TEMPLATER_PLUGIN_ID; + private readonly fileCreationSuppressions = new Map< + string, + TemplaterFileCreationSuppressionState + >(); + private activeFileCreationSuppressions = 0; + private suppressionTeardownLock: Promise | null = null; + private readonly renderLocks = new Map>(); + + constructor(private readonly app: App) {} + + getRawPlugin(): unknown | null { + return getRawTemplaterPlugin(this.app); + } + + getPlugin(): TemplaterPluginLike | null { + const plugin = this.getRawPlugin(); + if (!plugin) return null; + return plugin as TemplaterPluginLike; + } + + getCapabilityReport(): TemplaterCapabilityReport { + const plugin = this.getPlugin(); + if (!plugin) return createReport(false, { ...NO_CAPABILITIES }); + + const templater = plugin.templater; + const editorHandler = plugin.editor_handler; + const capabilities: TemplaterCapabilityMap = { + triggerOnFileCreation: + typeof plugin.settings?.trigger_on_file_creation === "boolean", + pendingTemplates: + templater?.files_with_pending_templates instanceof Set, + overwriteFileCommands: + typeof templater?.overwrite_file_commands === "function", + parseTemplate: typeof templater?.parse_template === "function", + createRunningConfig: + typeof templater?.create_running_config === "function", + cursorAutoJump: + typeof plugin.settings?.auto_jump_to_cursor === "boolean", + cursorJump: + typeof editorHandler?.jump_to_next_cursor_location === "function", + teardown: + typeof templater?.functions_generator?.teardown === "function", + }; + + return createReport(true, capabilities); + } + + hasCapability(capability: TemplaterCapability): boolean { + return this.getCapabilityReport().capabilities[capability]; + } + + isTriggerOnCreateEnabled(): boolean { + return !!this.getPlugin()?.settings?.trigger_on_file_creation; + } + + async waitForTriggerOnCreateToComplete( + file: TFile, + opts: { timeoutMs?: number; appearTimeoutMs?: number } = {}, + ): Promise { + if (file.extension !== "md") return; + if (!this.isTriggerOnCreateEnabled()) return; + + const pendingFiles = this.getPlugin()?.templater?.files_with_pending_templates; + if (!(pendingFiles instanceof Set)) { + await waitForFileToStopChanging(this.app, file, { + timeoutMs: opts.timeoutMs ?? 5000, + gracePeriodMs: opts.appearTimeoutMs ?? 2500, + quietPeriodMs: 200, + }); + return; + } + + const { timeoutMs = 5000, appearTimeoutMs = 2500 } = opts; + const start = Date.now(); + + while (Date.now() - start < appearTimeoutMs) { + if (pendingFiles.has(file.path)) break; + await sleep(50); + } + + while (Date.now() - start < timeoutMs) { + if (!pendingFiles.has(file.path)) break; + await sleep(50); + } + + await waitForFileSettle(this.app, file, 800); + } + + async withFileCreationSuppressed( + filePath: string, + fn: () => Promise, + ): Promise { + const plugin = this.getPlugin(); + const pendingFiles = plugin?.templater?.files_with_pending_templates; + if ( + !plugin || + !this.isTriggerOnCreateEnabled() || + !(pendingFiles instanceof Set) + ) { + return await fn(); + } + + this.activeFileCreationSuppressions++; + + let state = this.fileCreationSuppressions.get(filePath); + if (!state) { + state = { + count: 0, + hadPathInitially: pendingFiles.has(filePath), + }; + this.fileCreationSuppressions.set(filePath, state); + + if (!state.hadPathInitially) { + pendingFiles.add(filePath); + } + } + + state.count++; + + let fnSucceeded = false; + try { + const result = await fn(); + fnSucceeded = true; + return result; + } finally { + state.count--; + this.activeFileCreationSuppressions--; + + if (state.count <= 0) { + this.fileCreationSuppressions.delete(filePath); + + if (!state.hadPathInitially) { + if (fnSucceeded) { + await sleep(TEMPLATER_PENDING_CHECK_BUFFER_MS); + } + + pendingFiles.delete(filePath); + await this.maybeTeardownAfterSuppression(plugin, pendingFiles); + } + } + } + } + + async overwriteFileOnce( + file: TFile, + opts: { skipIfNoTags?: boolean; postWait?: boolean } = {}, + ): Promise { + if (file.extension !== "md") return; + + const plugin = this.getPlugin(); + const templater = plugin?.templater; + const overwrite = templater?.overwrite_file_commands; + if (!plugin || !templater || typeof overwrite !== "function") return; + + const { skipIfNoTags = true, postWait = true } = opts; + + await this.withFileLock(file.path, async () => { + await waitForFileSettle(this.app, file); + + let original: string; + try { + original = await this.app.vault.read(file); + } catch (err) { + reportError( + err as Error, + `overwriteTemplaterOnce: failed to read ${file.path} before render`, + ); + return; + } + + if (skipIfNoTags && !original.includes("<%")) { + return; + } + + try { + await overwrite.call(templater, file); + if (postWait) { + await waitForFileSettle(this.app, file, 800); + } + } catch (err) { + try { + await this.app.vault.modify(file, original); + } catch (rollbackErr) { + log.logWarning( + `Failed to rollback ${file.path} after Templater error: ${(rollbackErr as Error).message}`, + ); + } + reportError( + err as Error, + `Templater failed on ${file.path}. Rolled back to pre-render state.`, + ); + } + }); + } + + async parseTemplate( + templateContent: string, + targetFile: TFile, + ): Promise { + if (targetFile.extension !== "md") return templateContent; + + const templater = this.getPlugin()?.templater; + const parseTemplate = templater?.parse_template; + if (!templater || typeof parseTemplate !== "function") { + return templateContent; + } + + const createConfig = templater.create_running_config; + const config = + typeof createConfig === "function" + ? createConfig.call(templater, undefined, targetFile, 4) + : { target_file: targetFile, run_mode: 4, frontmatter: {} }; + + return await parseTemplate.call(templater, config, templateContent); + } + + async jumpToNextCursorIfPossible(file: TFile): Promise { + if (file.extension !== "md") return; + + const plugin = this.getPlugin(); + const autoJumpEnabled = !!plugin?.settings?.auto_jump_to_cursor; + if (!autoJumpEnabled) return; + if (this.app.workspace.getActiveFile?.()?.path !== file.path) return; + + const editorHandler = plugin?.editor_handler; + const jump = editorHandler?.jump_to_next_cursor_location; + + if (typeof jump === "function") { + try { + await jump.call(editorHandler, file, true); + return; + } catch (err) { + log.logWarning( + `jumpToNextTemplaterCursorIfPossible: API failed – ${(err as Error).message}`, + ); + } + } + + try { + ( + this.app.commands as unknown as { + executeCommandById?: (commandId: string) => boolean; + } + ).executeCommandById?.( + "templater-obsidian:jump-to-next-cursor-location", + ); + } catch { + // no-op + } + } + + private async maybeTeardownAfterSuppression( + plugin: TemplaterPluginLike, + pendingFiles: Set, + ): Promise { + if (this.activeFileCreationSuppressions > 0) return; + if (pendingFiles.size !== 0) return; + + if (this.suppressionTeardownLock) { + await this.suppressionTeardownLock; + return; + } + + this.suppressionTeardownLock = (async () => { + try { + this.app.workspace.trigger("templater:all-templates-executed"); + await plugin.templater?.functions_generator?.teardown?.(); + } catch (err) { + log.logWarning( + `withTemplaterFileCreationSuppressed: teardown failed – ${(err as Error).message}`, + ); + } + })(); + + try { + await this.suppressionTeardownLock; + } finally { + this.suppressionTeardownLock = null; + } + } + + private async withFileLock( + filePath: string, + fn: () => Promise, + ): Promise { + const previous = this.renderLocks.get(filePath) ?? Promise.resolve(); + let release!: () => void; + const current = new Promise((resolve) => { + release = () => resolve(); + }); + + const chain = previous.catch(() => undefined).then(() => current); + this.renderLocks.set(filePath, chain); + + chain + .finally(() => { + if (this.renderLocks.get(filePath) === chain) { + this.renderLocks.delete(filePath); + } + }) + .catch(() => undefined); + + await previous.catch(() => undefined); + try { + return await fn(); + } finally { + release(); + } + } +} + +export function createTemplaterIntegration(app: App): TemplaterIntegration { + return new ObsidianTemplaterIntegration(app); +} + +type AppWithPlugins = App & { + plugins?: { plugins?: Record }; +}; + +type TemplaterFileCreationSuppressionState = { + count: number; + hadPathInitially: boolean; +}; + +const TEMPLATER_PENDING_CHECK_BUFFER_MS = 350; + +function getRawTemplaterPlugin(app: App): unknown | null { + return (app as AppWithPlugins).plugins?.plugins?.[TEMPLATER_PLUGIN_ID] ?? null; +} + +async function waitForFileSettle( + app: App, + file: TFile, + timeoutMs = 500, +): Promise { + try { + const adapter = app.vault.adapter; + if (!("stat" in adapter) || typeof adapter.stat !== "function") return; + + const firstStat = await adapter.stat(file.path); + if (!firstStat) return; + let previousMtime = firstStat.mtime; + const start = Date.now(); + let pollIntervalMs = 30; + + while (Date.now() - start < timeoutMs) { + await sleep(pollIntervalMs); + const current = await adapter.stat(file.path); + if (!current) return; + if (current.mtime === previousMtime) return; + previousMtime = current.mtime; + pollIntervalMs = Math.min(pollIntervalMs * 2, 200); + } + } catch (err) { + log.logWarning( + `waitForFileSettle: fallback due to adapter/stat failure – ${(err as Error).message}`, + ); + } +} + +async function waitForFileToStopChanging( + app: App, + file: TFile, + opts: { + timeoutMs?: number; + quietPeriodMs?: number; + gracePeriodMs?: number; + } = {}, +): Promise { + const { + timeoutMs = 2000, + quietPeriodMs = 150, + gracePeriodMs = 800, + } = opts; + + try { + const adapter = app.vault.adapter; + if (!("stat" in adapter) || typeof adapter.stat !== "function") return; + + const firstStat = await adapter.stat(file.path); + if (!firstStat) return; + + let lastMtime = firstStat.mtime; + let lastChangeAt = Date.now(); + let sawExternalChange = false; + const start = lastChangeAt; + let pollIntervalMs = 50; + + while (Date.now() - start < timeoutMs) { + await sleep(pollIntervalMs); + const current = await adapter.stat(file.path); + if (!current) return; + + const now = Date.now(); + if (current.mtime !== lastMtime) { + sawExternalChange = true; + lastMtime = current.mtime; + lastChangeAt = now; + pollIntervalMs = 50; + continue; + } + + if (sawExternalChange) { + if (now - lastChangeAt >= quietPeriodMs) return; + } else if (now - start >= gracePeriodMs) { + return; + } + + pollIntervalMs = Math.min(Math.floor(pollIntervalMs * 1.5), 200); + } + } catch (err) { + log.logWarning( + `waitForFileToStopChanging: fallback due to adapter/stat failure – ${(err as Error).message}`, + ); + } +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/src/integrations/index.ts b/src/integrations/index.ts new file mode 100644 index 00000000..87635acd --- /dev/null +++ b/src/integrations/index.ts @@ -0,0 +1,2 @@ +export * from "./IntegrationRegistry"; +export * from "./TemplaterIntegration"; diff --git a/src/main.ts b/src/main.ts index 0c7d9c43..58b9d724 100644 --- a/src/main.ts +++ b/src/main.ts @@ -24,6 +24,11 @@ import { InfiniteAIAssistantCommandSettingsModal } from "./gui/MacroGUIs/AIAssis import { FieldSuggestionCache } from "./utils/FieldSuggestionCache"; import { isMajorUpdate } from "./utils/semver"; import { registerQuickAddCliHandlers } from "./cli/registerQuickAddCliHandlers"; +import { + createIntegrationRegistry, + registerIntegrationRegistry, + type IntegrationRegistry, +} from "./integrations/IntegrationRegistry"; // Parameters prefixed with `value-` get used as named values for the executed choice type CaptureValueParameters = { [key in `value-${string}`]?: string }; @@ -37,6 +42,7 @@ type UriParameters = DefinedUriParameters & CaptureValueParameters; export default class QuickAdd extends Plugin { static instance: QuickAdd; settings: QuickAddSettings; + integrations: IntegrationRegistry; private unsubscribeSettingsStore: () => void; get api(): ReturnType { @@ -50,6 +56,10 @@ export default class QuickAdd extends Plugin { async onload() { log.logMessage("Loading QuickAdd"); QuickAdd.instance = this; + this.integrations = registerIntegrationRegistry( + this.app, + createIntegrationRegistry(this.app), + ); await this.loadSettings(); settingsStore.setState(this.settings); diff --git a/src/preflight/collectChoiceFlowPreflight.test.ts b/src/preflight/collectChoiceFlowPreflight.test.ts new file mode 100644 index 00000000..d79f95ad --- /dev/null +++ b/src/preflight/collectChoiceFlowPreflight.test.ts @@ -0,0 +1,463 @@ +import { describe, expect, it, vi } from "vitest"; +import type { App } from "obsidian"; +import { TFile } from "obsidian"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; +import type ICaptureChoice from "../types/choices/ICaptureChoice"; +import type IMacroChoice from "../types/choices/IMacroChoice"; +import type ITemplateChoice from "../types/choices/ITemplateChoice"; +import { CommandType } from "../types/macros/CommandType"; +import { collectChoiceFlowPreflight } from "./collectChoiceFlowPreflight"; + +vi.mock("../utilityObsidian", () => ({ + getMarkdownFilesInFolder: vi.fn(() => []), + getMarkdownFilesWithTag: vi.fn(() => []), + getUserScript: vi.fn(), + isFolder: vi.fn(() => false), +})); + +function createExecutor(): IChoiceExecutor { + return { + execute: vi.fn(), + variables: new Map(), + }; +} + +function createApp(templateContent = ""): App { + const templateFile = new TFile(); + templateFile.path = "Templates/Note.md"; + templateFile.name = "Note.md"; + templateFile.basename = "Note"; + templateFile.extension = "md"; + + return { + plugins: { plugins: {} }, + vault: { + getAbstractFileByPath: vi.fn((path: string) => + path === templateFile.path ? templateFile : null, + ), + cachedRead: vi.fn().mockResolvedValue(templateContent), + }, + workspace: { + getActiveViewOfType: vi.fn(() => null), + }, + } as unknown as App; +} + +function createPlugin(choices: Array) { + const byId = new Map(choices.map((choice) => [choice.id, choice])); + return { + settings: { + inputPrompt: "single-line", + globalVariables: {}, + useSelectionAsCaptureValue: true, + choices, + }, + getChoiceById: vi.fn((id: string) => { + const choice = byId.get(id); + if (!choice) throw new Error(`Choice ${id} not found`); + return choice; + }), + } as any; +} + +function createCaptureChoice(): ICaptureChoice { + return { + id: "capture-choice", + name: "Capture Project", + type: "Capture", + command: false, + captureTo: "Inbox.md", + captureToActiveFile: false, + createFileIfItDoesntExist: { + enabled: false, + createWithTemplate: false, + template: "", + }, + format: { enabled: true, format: "Project: {{VALUE:project}}" }, + prepend: false, + appendLink: false, + task: false, + insertAfter: { + enabled: false, + after: "", + insertAtEnd: false, + considerSubsections: false, + createIfNotFound: false, + createIfNotFoundLocation: "", + }, + newLineCapture: { + enabled: false, + direction: "below", + }, + openFile: false, + fileOpening: { + location: "tab", + direction: "vertical", + mode: "default", + focus: true, + }, + }; +} + +function createTemplateChoice(): ITemplateChoice { + return { + id: "template-choice", + name: "Template Note", + type: "Template", + command: false, + templatePath: "Templates/Note.md", + folder: { + enabled: false, + folders: [], + chooseWhenCreatingNote: false, + createInSameFolderAsActiveFile: false, + chooseFromSubfolders: false, + }, + fileNameFormat: { enabled: false, format: "" }, + appendLink: false, + openFile: false, + fileOpening: { + location: "tab", + direction: "vertical", + mode: "default", + focus: true, + }, + fileExistsBehavior: { kind: "prompt" }, + }; +} + +describe("collectChoiceFlowPreflight", () => { + it("reuses requirement parsing for nested macro choices and emits flow diagnostics", async () => { + const captureChoice = createCaptureChoice(); + const macroChoice: IMacroChoice = { + id: "macro-choice", + name: "Macro Flow", + type: "Macro", + command: false, + runOnStartup: false, + macro: { + id: "macro", + name: "Macro Flow", + commands: [ + { + id: "nested-command", + name: "Run capture", + type: CommandType.NestedChoice, + choice: captureChoice, + } as any, + ], + }, + }; + const app = createApp(); + const plugin = createPlugin([macroChoice, captureChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + macroChoice, + ); + + expect(result.requirements).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: "project" }), + ]), + ); + expect(result.unresolvedRequirements.map((req) => req.id)).toContain( + "project", + ); + expect(result.choices.map((choice) => choice.id)).toEqual([ + "macro-choice", + "capture-choice", + ]); + expect(result.diagnostics.map((diagnostic) => diagnostic.code)).toEqual( + expect.arrayContaining([ + "flow-shared-context", + "nested-choice-shares-context", + "missing-required-inputs", + ]), + ); + }); + + it("only follows the taken variable conditional branch during preflight", async () => { + const thenChoice = createCaptureChoice(); + thenChoice.id = "then-capture"; + thenChoice.name = "Then Capture"; + thenChoice.format.format = "Then: {{VALUE:thenValue}}"; + + const elseChoice = createCaptureChoice(); + elseChoice.id = "else-capture"; + elseChoice.name = "Else Capture"; + elseChoice.format.format = "Else: {{VALUE:elseValue}}"; + + const macroChoice: IMacroChoice = { + id: "macro-choice", + name: "Macro Flow", + type: "Macro", + command: false, + runOnStartup: false, + macro: { + id: "macro", + name: "Macro Flow", + commands: [ + { + id: "conditional-command", + name: "Choose branch", + type: CommandType.Conditional, + condition: { + mode: "variable", + variableName: "route", + operator: "equals", + valueType: "string", + expectedValue: "then", + }, + thenCommands: [ + { + id: "then-command", + name: "Run then", + type: CommandType.NestedChoice, + choice: thenChoice, + } as any, + ], + elseCommands: [ + { + id: "else-command", + name: "Run else", + type: CommandType.NestedChoice, + choice: elseChoice, + } as any, + ], + } as any, + ], + }, + }; + const app = createApp(); + const plugin = createPlugin([macroChoice, thenChoice, elseChoice]); + const executor = createExecutor(); + executor.variables.set("route", "then"); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + executor, + macroChoice, + ); + + expect(result.choices.map((choice) => choice.id)).toEqual([ + "macro-choice", + "then-capture", + ]); + expect(result.unresolvedRequirements.map((req) => req.id)).toEqual([ + "thenValue", + ]); + expect(result.unresolvedRequirements.map((req) => req.id)) + .not.toContain("elseValue"); + }); + + it("conservatively follows both script conditional branches during preflight", async () => { + const thenChoice = createCaptureChoice(); + thenChoice.id = "then-capture"; + thenChoice.name = "Then Capture"; + const elseChoice = createCaptureChoice(); + elseChoice.id = "else-capture"; + elseChoice.name = "Else Capture"; + + const macroChoice: IMacroChoice = { + id: "macro-choice", + name: "Macro Flow", + type: "Macro", + command: false, + runOnStartup: false, + macro: { + id: "macro", + name: "Macro Flow", + commands: [ + { + id: "conditional-command", + name: "Choose branch", + type: CommandType.Conditional, + condition: { + mode: "script", + scriptPath: "Scripts/branch.js", + }, + thenCommands: [ + { + id: "then-command", + name: "Run then", + type: CommandType.NestedChoice, + choice: thenChoice, + } as any, + ], + elseCommands: [ + { + id: "else-command", + name: "Run else", + type: CommandType.NestedChoice, + choice: elseChoice, + } as any, + ], + } as any, + ], + }, + }; + const app = createApp(); + const plugin = createPlugin([macroChoice, thenChoice, elseChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + macroChoice, + ); + + expect(result.choices.map((choice) => choice.id)).toEqual([ + "macro-choice", + "then-capture", + "else-capture", + ]); + }); + + it("emits integration diagnostics when Templater syntax is detected without Templater", async () => { + const templateChoice = createTemplateChoice(); + const app = createApp("Created at <% tp.date.now() %>"); + const plugin = createPlugin([templateChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + templateChoice, + ); + + expect(result.diagnostics).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "templater-not-installed", + severity: "info", + details: expect.objectContaining({ + requiredCapabilities: ["overwriteFileCommands"], + }), + }), + ]), + ); + }); + + it("does not report Templater diagnostics for plain template content", async () => { + const templateChoice = createTemplateChoice(); + const app = createApp("Plain note content"); + const plugin = createPlugin([templateChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + templateChoice, + ); + + expect(result.diagnostics.map((diagnostic) => diagnostic.code)) + .not.toContain("templater-not-installed"); + }); + + it("does not report parseTemplate for append captures with Templater syntax", async () => { + const captureChoice = createCaptureChoice(); + captureChoice.format.format = "Append <% tp.date.now() %>"; + captureChoice.createFileIfItDoesntExist = { + enabled: true, + createWithTemplate: false, + template: "", + }; + const app = createApp(); + const plugin = createPlugin([captureChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + captureChoice, + ); + + expect(result.diagnostics.map((diagnostic) => diagnostic.code)) + .not.toContain("templater-not-installed"); + }); + + it("reports parseTemplate for editor insertion captures with Templater syntax", async () => { + const captureChoice = createCaptureChoice(); + captureChoice.captureToActiveFile = true; + captureChoice.format.format = "Inline <% tp.date.now() %>"; + const app = createApp(); + const plugin = createPlugin([captureChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + captureChoice, + ); + + expect(result.diagnostics).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "templater-not-installed", + details: expect.objectContaining({ + requiredCapabilities: ["parseTemplate"], + }), + }), + ]), + ); + }); + + it("reports overwriteFileCommands for capture template rendering", async () => { + const captureChoice = createCaptureChoice(); + captureChoice.createFileIfItDoesntExist = { + enabled: true, + createWithTemplate: true, + template: "Templates/Note.md", + }; + const app = createApp("Template <% tp.date.now() %>"); + const plugin = createPlugin([captureChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + captureChoice, + ); + + expect(result.diagnostics).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "templater-not-installed", + details: expect.objectContaining({ + requiredCapabilities: ["overwriteFileCommands"], + }), + }), + ]), + ); + }); + + it("reports overwriteFileCommands for capture whole-file Templater policy", async () => { + const captureChoice = createCaptureChoice(); + captureChoice.templater = { afterCapture: "wholeFile" }; + const app = createApp(); + const plugin = createPlugin([captureChoice]); + + const result = await collectChoiceFlowPreflight( + app, + plugin, + createExecutor(), + captureChoice, + ); + + expect(result.diagnostics).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "templater-not-installed", + details: expect.objectContaining({ + requiredCapabilities: ["overwriteFileCommands"], + reason: "capture-after-whole-file", + }), + }), + ]), + ); + }); +}); diff --git a/src/preflight/collectChoiceFlowPreflight.ts b/src/preflight/collectChoiceFlowPreflight.ts new file mode 100644 index 00000000..477bdf45 --- /dev/null +++ b/src/preflight/collectChoiceFlowPreflight.ts @@ -0,0 +1,431 @@ +import type { App, TFile } from "obsidian"; +import { TFile as ObsidianTFile } from "obsidian"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; +import { + BASE_FILE_EXTENSION_REGEX, + CANVAS_FILE_EXTENSION_REGEX, + MARKDOWN_FILE_EXTENSION_REGEX, +} from "../constants"; +import { getCaptureAction } from "../engine/captureAction"; +import { + createDiagnostic, + type ChoiceExecutionDiagnostic, +} from "../engine/runtime"; +import { getIntegrationRegistry } from "../integrations/IntegrationRegistry"; +import type QuickAdd from "../main"; +import type ICaptureChoice from "../types/choices/ICaptureChoice"; +import type IChoice from "../types/choices/IChoice"; +import type IMacroChoice from "../types/choices/IMacroChoice"; +import type ITemplateChoice from "../types/choices/ITemplateChoice"; +import type { IConditionalCommand } from "../types/macros/Conditional/IConditionalCommand"; +import { evaluateCondition } from "../engine/helpers/conditionalEvaluator"; +import { CommandType } from "../types/macros/CommandType"; +import type { IChoiceCommand } from "../types/macros/IChoiceCommand"; +import type { ICommand } from "../types/macros/ICommand"; +import type { INestedChoiceCommand } from "../types/macros/QuickCommands/INestedChoiceCommand"; +import type { TemplaterCapability } from "../integrations/TemplaterIntegration"; +import { + collectChoiceRequirements, + getUnresolvedRequirements, +} from "./collectChoiceRequirements"; +import type { FieldRequirement } from "./RequirementCollector"; + +export interface ChoiceFlowPreflightChoiceSummary { + id: string; + name: string; + type: IChoice["type"]; + path: string; + depth: number; +} + +export interface ChoiceFlowPreflightResult { + requirements: FieldRequirement[]; + unresolvedRequirements: FieldRequirement[]; + diagnostics: ChoiceExecutionDiagnostic[]; + choices: ChoiceFlowPreflightChoiceSummary[]; +} + +export async function collectChoiceFlowPreflight( + app: App, + plugin: QuickAdd, + choiceExecutor: IChoiceExecutor, + choice: IChoice, +): Promise { + const requirementsById = new Map(); + const diagnostics: ChoiceExecutionDiagnostic[] = []; + const choices: ChoiceFlowPreflightChoiceSummary[] = []; + const visited = new Set(); + + const visitChoice = async ( + currentChoice: IChoice, + path: string[], + depth: number, + ) => { + const key = currentChoice.id || path.join("/"); + if (visited.has(key)) { + diagnostics.push( + createDiagnostic({ + severity: "warning", + code: "flow-cycle-skipped", + message: `Skipping already visited choice '${currentChoice.name}' during flow preflight.`, + source: "choice", + choiceId: currentChoice.id, + details: { path: path.join(" / ") }, + }), + ); + return; + } + + visited.add(key); + choices.push({ + id: currentChoice.id, + name: currentChoice.name, + type: currentChoice.type, + path: path.join(" / "), + depth, + }); + + const requirements = await collectChoiceRequirements( + app, + plugin, + choiceExecutor, + currentChoice, + ); + for (const requirement of requirements) { + if (!requirementsById.has(requirement.id)) { + requirementsById.set(requirement.id, requirement); + } + } + + await addIntegrationDiagnostics( + app, + currentChoice, + diagnostics, + ); + + if (currentChoice.type !== "Macro") return; + + diagnostics.push( + createDiagnostic({ + severity: "info", + code: "flow-shared-context", + message: + "Macro commands and nested choices share one execution context, variable map, and origin leaf.", + source: "runtime", + choiceId: currentChoice.id, + details: { path: path.join(" / ") }, + }), + ); + + await visitCommands( + (currentChoice as IMacroChoice).macro?.commands ?? [], + path, + depth, + ); + }; + + const visitCommands = async ( + commands: ICommand[], + path: string[], + depth: number, + ) => { + for (const command of commands) { + if (command?.type === CommandType.NestedChoice) { + const nested = (command as INestedChoiceCommand).choice; + if (!nested) { + addMissingNestedChoiceDiagnostic(diagnostics, command); + continue; + } + diagnostics.push(createNestedChoiceDiagnostic(command, nested)); + await visitChoice(nested, [...path, nested.name], depth + 1); + continue; + } + + if (command?.type === CommandType.Choice) { + const choiceCommand = command as IChoiceCommand; + const nested = resolveChoiceById( + plugin, + choiceCommand.choiceId, + diagnostics, + command, + ); + if (!nested) continue; + diagnostics.push(createNestedChoiceDiagnostic(command, nested)); + await visitChoice(nested, [...path, nested.name], depth + 1); + continue; + } + + if (command?.type === CommandType.Conditional) { + const conditional = command as IConditionalCommand; + const branch = await evaluatePreflightConditionalBranch( + conditional, + choiceExecutor.variables, + ); + + if (branch === "then" || branch === "unknown") { + await visitCommands( + conditional.thenCommands ?? [], + [...path, `${command.name} then`], + depth, + ); + } + + if (branch === "else" || branch === "unknown") { + await visitCommands( + conditional.elseCommands ?? [], + [...path, `${command.name} else`], + depth, + ); + } + } + } + }; + + await visitChoice(choice, [choice.name], 0); + + const requirements = Array.from(requirementsById.values()); + const unresolvedRequirements = getUnresolvedRequirements( + requirements, + choiceExecutor.variables, + ); + + if (unresolvedRequirements.length > 0) { + diagnostics.push( + createDiagnostic({ + severity: "error", + code: "missing-required-inputs", + message: `${unresolvedRequirements.length} required input(s) are missing for this flow.`, + source: "runtime", + choiceId: choice.id, + details: { + missingIds: unresolvedRequirements.map( + (requirement) => requirement.id, + ), + }, + }), + ); + } + + return { + requirements, + unresolvedRequirements, + diagnostics, + choices, + }; +} + +type PreflightConditionalBranch = "then" | "else" | "unknown"; + +async function evaluatePreflightConditionalBranch( + conditional: IConditionalCommand, + variables: Map, +): Promise { + if (!conditional.condition || conditional.condition.mode === "script") { + return "unknown"; + } + + const shouldRunThenBranch = await evaluateCondition(conditional.condition, { + variables: Object.fromEntries(variables), + evaluateScriptCondition: async () => false, + }); + + return shouldRunThenBranch ? "then" : "else"; +} + +function resolveChoiceById( + plugin: QuickAdd, + choiceId: string, + diagnostics: ChoiceExecutionDiagnostic[], + command: ICommand, +): IChoice | null { + try { + const choice = plugin.getChoiceById(choiceId); + if (choice) return choice; + } catch (error) { + addChoiceCommandNotFoundDiagnostic( + diagnostics, + choiceId, + command, + error, + ); + return null; + } + + addChoiceCommandNotFoundDiagnostic(diagnostics, choiceId, command); + return null; +} + +function addChoiceCommandNotFoundDiagnostic( + diagnostics: ChoiceExecutionDiagnostic[], + choiceId: string, + command: ICommand, + cause?: unknown, +): void { + diagnostics.push( + createDiagnostic({ + severity: "warning", + code: "nested-choice-not-found", + message: `Nested choice '${choiceId}' referenced by '${command.name}' could not be found.`, + source: "command", + stepId: command.id, + details: { choiceId, commandName: command.name }, + cause, + }), + ); +} + +function addMissingNestedChoiceDiagnostic( + diagnostics: ChoiceExecutionDiagnostic[], + command: ICommand, +): void { + diagnostics.push( + createDiagnostic({ + severity: "warning", + code: "nested-choice-not-found", + message: `Nested choice command '${command.name}' has no choice configured.`, + source: "command", + stepId: command.id, + details: { commandName: command.name }, + }), + ); +} + +function createNestedChoiceDiagnostic( + command: ICommand, + choice: IChoice, +): ChoiceExecutionDiagnostic { + return createDiagnostic({ + severity: "info", + code: "nested-choice-shares-context", + message: `Nested choice '${choice.name}' will reuse the parent flow context.`, + source: "command", + stepId: command.id, + choiceId: choice.id, + details: { + commandName: command.name, + choiceType: choice.type, + }, + }); +} + +async function addIntegrationDiagnostics( + app: App, + choice: IChoice, + diagnostics: ChoiceExecutionDiagnostic[], +): Promise { + const templaterUsage = await getTemplaterUsage(app, choice); + if (!templaterUsage.usesTemplater) return; + + const templater = getIntegrationRegistry(app).templater; + const report = templater.getCapabilityReport(); + if (!report.installed) { + diagnostics.push( + createDiagnostic({ + severity: "info", + code: "templater-not-installed", + message: + "Templater syntax or policy was detected, but Templater is not installed. QuickAdd will skip Templater operations.", + source: "integration", + choiceId: choice.id, + integrationId: report.pluginId, + details: templaterUsage, + }), + ); + return; + } + + const missing = templaterUsage.requiredCapabilities.filter( + (capability) => !templater.hasCapability(capability), + ); + if (missing.length === 0) return; + + diagnostics.push( + createDiagnostic({ + severity: "warning", + code: "templater-capabilities-missing", + message: `Templater is missing optional capability/capabilities: ${missing.join(", ")}.`, + source: "integration", + choiceId: choice.id, + integrationId: report.pluginId, + details: { ...templaterUsage, missingCapabilities: missing }, + }), + ); +} + +async function getTemplaterUsage( + app: App, + choice: IChoice, +): Promise<{ + usesTemplater: boolean; + requiredCapabilities: TemplaterCapability[]; + reason: string; +}> { + if (choice.type === "Template") { + const content = await readTemplate(app, (choice as ITemplateChoice).templatePath); + const requiredCapabilities: TemplaterCapability[] = content.includes("<%") + ? ["overwriteFileCommands"] + : []; + return { + usesTemplater: requiredCapabilities.length > 0, + requiredCapabilities, + reason: "template-content", + }; + } + + if (choice.type === "Capture") { + const capture = choice as ICaptureChoice; + const captureFormat = capture.format?.enabled ? capture.format.format : ""; + const templateContent = capture.createFileIfItDoesntExist?.createWithTemplate + ? await readTemplate(app, capture.createFileIfItDoesntExist.template) + : ""; + const action = getCaptureAction(capture); + const actionParsesCaptureContent = + action === "currentLine" || + action === "newLineAbove" || + action === "newLineBelow"; + const afterCaptureWholeFile = + capture.templater?.afterCapture === "wholeFile"; + const requiredCapabilities = uniqueCapabilities([ + ...(actionParsesCaptureContent && captureFormat.includes("<%") + ? (["parseTemplate"] as TemplaterCapability[]) + : []), + ...(templateContent.includes("<%") || afterCaptureWholeFile + ? (["overwriteFileCommands"] as TemplaterCapability[]) + : []), + ]); + + return { + usesTemplater: requiredCapabilities.length > 0, + requiredCapabilities, + reason: afterCaptureWholeFile + ? "capture-after-whole-file" + : "capture-content", + }; + } + + return { + usesTemplater: false, + requiredCapabilities: [], + reason: "not-applicable", + }; +} + +function uniqueCapabilities( + capabilities: TemplaterCapability[], +): TemplaterCapability[] { + return Array.from(new Set(capabilities)); +} + +async function readTemplate(app: App, path: string): Promise { + if (!path) return ""; + const addExt = + !MARKDOWN_FILE_EXTENSION_REGEX.test(path) && + !CANVAS_FILE_EXTENSION_REGEX.test(path) && + !BASE_FILE_EXTENSION_REGEX.test(path); + const normalized = addExt ? `${path}.md` : path; + const file = app.vault.getAbstractFileByPath(normalized); + if (file instanceof ObsidianTFile) { + return await app.vault.cachedRead(file as TFile); + } + return ""; +} diff --git a/src/quickAddApi.executeChoice.test.ts b/src/quickAddApi.executeChoice.test.ts index 73423191..8d06652b 100644 --- a/src/quickAddApi.executeChoice.test.ts +++ b/src/quickAddApi.executeChoice.test.ts @@ -34,7 +34,11 @@ describe("QuickAddApi.executeChoice", () => { beforeEach(() => { variables = new Map(); choiceExecutor = { - execute: vi.fn().mockResolvedValue(undefined), + execute: vi.fn().mockResolvedValue({ + status: "success", + artifacts: [], + diagnostics: [], + }), variables, consumeAbortSignal: vi.fn().mockReturnValue(null), }; @@ -54,15 +58,50 @@ describe("QuickAddApi.executeChoice", () => { await expect(api.executeChoice("My Template")) .rejects.toBe(abortError); expect(choiceExecutor.consumeAbortSignal).toHaveBeenCalledTimes(1); - expect(variables.size).toBe(0); + expect(variables.get("foo")).toBe("bar"); }); - it("clears variables and resolves when no abort is signalled", async () => { + it("restores variables when no abort is signalled", async () => { const api = QuickAddApi.GetApi(app, plugin, choiceExecutor); + variables.set("existing", "kept"); await expect( api.executeChoice("My Template", { project: "QA" }), ).resolves.toBeUndefined(); expect(choiceExecutor.consumeAbortSignal).toHaveBeenCalledTimes(1); - expect(variables.size).toBe(0); + expect(variables.get("existing")).toBe("kept"); + expect(variables.has("project")).toBe(false); + }); + + it("rejects when execution result status is aborted", async () => { + const abortError = new MacroAbortError("Nested choice aborted"); + (choiceExecutor.execute as ReturnType).mockResolvedValueOnce({ + status: "aborted", + error: abortError, + artifacts: [], + diagnostics: [], + }); + const api = QuickAddApi.GetApi(app, plugin, choiceExecutor); + + variables.set("existing", "kept"); + await expect(api.executeChoice("My Template", { project: "QA" })) + .rejects.toBe(abortError); + expect(choiceExecutor.consumeAbortSignal).toHaveBeenCalledTimes(1); + expect(variables.get("existing")).toBe("kept"); + expect(variables.has("project")).toBe(false); + }); + + it("rejects when execution result status is failed", async () => { + const failure = new Error("Choice failed"); + (choiceExecutor.execute as ReturnType).mockResolvedValueOnce({ + status: "failed", + error: failure, + artifacts: [], + diagnostics: [], + }); + const api = QuickAddApi.GetApi(app, plugin, choiceExecutor); + + await expect(api.executeChoice("My Template")) + .rejects.toBe(failure); + expect(choiceExecutor.consumeAbortSignal).toHaveBeenCalledTimes(1); }); }); diff --git a/src/quickAddApi.ts b/src/quickAddApi.ts index d64ea6ec..7bdfa8e1 100644 --- a/src/quickAddApi.ts +++ b/src/quickAddApi.ts @@ -17,6 +17,7 @@ import { import type { OpenAIModelParameters } from "./ai/OpenAIModelParameters"; import type { Model } from "./ai/Provider"; import { resolveProviderApiKey } from "./ai/providerSecrets"; +import type { ChoiceExecutionResult } from "./engine/runtime"; import { CompleteFormatter } from "./formatters/completeFormatter"; import GenericCheckboxPrompt from "./gui/GenericCheckboxPrompt/genericCheckboxPrompt"; import GenericInfoDialog from "./gui/GenericInfoDialog/GenericInfoDialog"; @@ -57,6 +58,24 @@ function restoreVariables( } } +function throwIfChoiceExecutionDidNotComplete( + result: ChoiceExecutionResult, +): void { + if (result.status === "aborted") { + if (result.error instanceof MacroAbortError) throw result.error; + throw new MacroAbortError( + result.error instanceof Error && result.error.message + ? result.error.message + : "Choice execution aborted", + ); + } + + if (result.status === "failed") { + if (result.error instanceof Error) throw result.error; + throw new Error("Choice execution failed"); + } +} + export class QuickAddApi { public static GetApi( app: App, @@ -219,17 +238,23 @@ export class QuickAddApi { "API executeChoice error", ); - if (variables) { - Object.keys(variables).forEach((key) => { - choiceExecutor.variables.set(key, variables[key]); - }); - } + const snapshot = snapshotVariables(choiceExecutor.variables); + + try { + if (variables) { + Object.keys(variables).forEach((key) => { + choiceExecutor.variables.set(key, variables[key]); + }); + } - await choiceExecutor.execute(choice); - const abort = choiceExecutor.consumeAbortSignal?.(); - choiceExecutor.variables.clear(); - if (abort) { - throw abort; + const result = await choiceExecutor.execute(choice); + const abort = choiceExecutor.consumeAbortSignal?.(); + if (abort) { + throw abort; + } + throwIfChoiceExecutionDidNotComplete(result); + } finally { + restoreVariables(choiceExecutor.variables, snapshot); } }, format: async ( diff --git a/src/utilityObsidian.templater-binding.test.ts b/src/utilityObsidian.templater-binding.test.ts index 9f385738..1a97f9f8 100644 --- a/src/utilityObsidian.templater-binding.test.ts +++ b/src/utilityObsidian.templater-binding.test.ts @@ -1,6 +1,18 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { App, TFile } from "obsidian"; -import { jumpToNextTemplaterCursorIfPossible, templaterParseTemplate } from "./utilityObsidian"; +import { IntegrationRegistry } from "./integrations/IntegrationRegistry"; +import { registerIntegrationRegistry } from "./integrations/IntegrationRegistry"; +import type { TemplaterIntegration } from "./integrations/TemplaterIntegration"; +import { + getTemplater, + getTemplaterPlugin, + isTemplaterTriggerOnCreateEnabled, + jumpToNextTemplaterCursorIfPossible, + overwriteTemplaterOnce, + templaterParseTemplate, + waitForTemplaterTriggerOnCreateToComplete, + withTemplaterFileCreationSuppressed, +} from "./utilityObsidian"; describe("templaterParseTemplate", () => { it("calls parse_template with the correct `this` context", async () => { @@ -28,6 +40,78 @@ describe("templaterParseTemplate", () => { }); }); +describe("legacy Templater wrappers", () => { + it("delegate through the registered integration adapter", async () => { + const app = new App(); + const file = new TFile(); + file.path = "QA.md"; + file.extension = "md"; + + const rawPlugin = { ok: true }; + const plugin = { settings: { trigger_on_file_creation: true } }; + const fakeIntegration: TemplaterIntegration = { + id: "templater-obsidian" as const, + getRawPlugin: vi.fn(() => rawPlugin), + getPlugin: vi.fn(() => plugin), + getCapabilityReport: vi.fn(() => ({ + pluginId: "templater-obsidian" as const, + installed: true, + capabilities: { + triggerOnFileCreation: true, + pendingTemplates: false, + overwriteFileCommands: true, + parseTemplate: true, + createRunningConfig: false, + cursorAutoJump: false, + cursorJump: true, + teardown: false, + }, + missingCapabilities: [], + })), + hasCapability: vi.fn(() => true), + isTriggerOnCreateEnabled: vi.fn(() => true), + waitForTriggerOnCreateToComplete: vi.fn(async () => undefined), + withFileCreationSuppressed: vi.fn(async (_path, fn) => await fn()), + overwriteFileOnce: vi.fn(async () => undefined), + parseTemplate: vi.fn(async (content) => `wrapped:${content}`), + jumpToNextCursorIfPossible: vi.fn(async () => undefined), + }; + registerIntegrationRegistry( + app as any, + new IntegrationRegistry({ templater: fakeIntegration }), + ); + + expect(getTemplater(app as any)).toBe(rawPlugin); + expect(getTemplaterPlugin(app as any)).toBe(plugin); + expect(isTemplaterTriggerOnCreateEnabled(app as any)).toBe(true); + expect(await templaterParseTemplate(app as any, "hello", file as any)).toBe( + "wrapped:hello", + ); + expect( + await withTemplaterFileCreationSuppressed( + app as any, + "QA.md", + async () => "suppressed", + ), + ).toBe("suppressed"); + await overwriteTemplaterOnce(app as any, file as any, { + skipIfNoTags: false, + }); + await waitForTemplaterTriggerOnCreateToComplete(app as any, file as any); + await jumpToNextTemplaterCursorIfPossible(app as any, file as any); + + expect(fakeIntegration.parseTemplate).toHaveBeenCalledWith("hello", file); + expect(fakeIntegration.withFileCreationSuppressed).toHaveBeenCalled(); + expect(fakeIntegration.overwriteFileOnce).toHaveBeenCalledWith(file, { + skipIfNoTags: false, + }); + expect( + fakeIntegration.waitForTriggerOnCreateToComplete, + ).toHaveBeenCalledWith(file, {}); + expect(fakeIntegration.jumpToNextCursorIfPossible).toHaveBeenCalledWith(file); + }); +}); + describe("jumpToNextTemplaterCursorIfPossible", () => { it("calls jump_to_next_cursor_location with the correct `this` context", async () => { const app = new App(); diff --git a/src/utilityObsidian.ts b/src/utilityObsidian.ts index 955b64a8..17a17761 100644 --- a/src/utilityObsidian.ts +++ b/src/utilityObsidian.ts @@ -21,36 +21,11 @@ import type { import type { AppendLinkOptions, LinkPlacement } from "./types/linkPlacement"; import { placementSupportsEmbed } from "./types/linkPlacement"; import type { IUserScript } from "./types/macros/IUserScript"; -import { reportError } from "./utils/errorUtils"; import { deepClone } from "./utils/deepClone"; +import { getIntegrationRegistry } from "./integrations/IntegrationRegistry"; +import type { TemplaterPluginLike } from "./integrations/TemplaterIntegration"; -export type TemplaterPluginLike = { - settings?: { - trigger_on_file_creation?: boolean; - auto_jump_to_cursor?: boolean; - }; - templater?: { - overwrite_file_commands?: (f: TFile) => Promise; - parse_template?: ( - opt: { target_file: TFile; run_mode: number; frontmatter?: Record }, - content: string, - ) => Promise; - create_running_config?: ( - template_file: TFile | undefined, - target_file: TFile, - run_mode: number, - ) => { target_file: TFile; run_mode: number; frontmatter: Record }; - files_with_pending_templates?: Set; - functions_generator?: { teardown?: () => Promise }; - }; - editor_handler?: { - plugin?: unknown; - jump_to_next_cursor_location?: ( - file?: TFile | null, - auto_jump?: boolean, - ) => Promise; - }; -}; +export type { TemplaterPluginLike } from "./integrations/TemplaterIntegration"; /** * Wait until the filesystem reports a stable mtime for the file or the timeout elapses. @@ -84,18 +59,16 @@ export async function waitForFileSettle(app: App, file: TFile, timeoutMs = 500) } } -export function getTemplater(app: App) { - return app.plugins.plugins["templater-obsidian"]; +export function getTemplater(app: App): unknown | null { + return getIntegrationRegistry(app).templater.getRawPlugin(); } export function getTemplaterPlugin(app: App): TemplaterPluginLike | null { - const plugin = getTemplater(app); - if (!plugin) return null; - return plugin as unknown as TemplaterPluginLike; + return getIntegrationRegistry(app).templater.getPlugin(); } export function isTemplaterTriggerOnCreateEnabled(app: App): boolean { - return !!getTemplaterPlugin(app)?.settings?.trigger_on_file_creation; + return getIntegrationRegistry(app).templater.isTriggerOnCreateEnabled(); } function sleep(ms: number): Promise { @@ -107,84 +80,10 @@ export async function waitForTemplaterTriggerOnCreateToComplete( file: TFile, opts: { timeoutMs?: number; appearTimeoutMs?: number } = {}, ): Promise { - if (file.extension !== "md") return; - if (!isTemplaterTriggerOnCreateEnabled(app)) return; - - const plugin = getTemplaterPlugin(app); - const pendingFiles = plugin?.templater?.files_with_pending_templates; - if (!(pendingFiles instanceof Set)) { - await waitForFileToStopChanging(app, file, { - timeoutMs: opts.timeoutMs ?? 5000, - gracePeriodMs: opts.appearTimeoutMs ?? 2500, - quietPeriodMs: 200, - }); - return; - } - - const { timeoutMs = 5000, appearTimeoutMs = 2500 } = opts; - const start = Date.now(); - - while (Date.now() - start < appearTimeoutMs) { - if (pendingFiles.has(file.path)) break; - await sleep(50); - } - - while (Date.now() - start < timeoutMs) { - if (!pendingFiles.has(file.path)) break; - await sleep(50); - } - - await waitForFileSettle(app, file, 800); -} - -type TemplaterFileCreationSuppressionState = { - count: number; - hadPathInitially: boolean; -}; - -const templaterFileCreationSuppressions = new Map< - string, - TemplaterFileCreationSuppressionState ->(); -let activeTemplaterFileCreationSuppressions = 0; -let templaterSuppressionTeardownLock: Promise | null = null; - -// Templater waits ~300ms before checking `files_with_pending_templates` in its -// on-create handler. We hold the entry slightly longer to ensure the bypass is -// observed. -// Tested with templater-obsidian v2.x; may need adjustment if Templater internals -// change. -const TEMPLATER_PENDING_CHECK_BUFFER_MS = 350; - -async function maybeTeardownTemplaterAfterSuppression( - app: App, - plugin: TemplaterPluginLike, - pendingFiles: Set, -): Promise { - if (activeTemplaterFileCreationSuppressions > 0) return; - if (pendingFiles.size !== 0) return; - - if (templaterSuppressionTeardownLock) { - await templaterSuppressionTeardownLock; - return; - } - - templaterSuppressionTeardownLock = (async () => { - try { - app.workspace.trigger("templater:all-templates-executed"); - await plugin.templater?.functions_generator?.teardown?.(); - } catch (err) { - log.logWarning( - `withTemplaterFileCreationSuppressed: teardown failed – ${(err as Error).message}`, - ); - } - })(); - - try { - await templaterSuppressionTeardownLock; - } finally { - templaterSuppressionTeardownLock = null; - } + await getIntegrationRegistry(app).templater.waitForTriggerOnCreateToComplete( + file, + opts, + ); } export async function withTemplaterFileCreationSuppressed( @@ -192,62 +91,10 @@ export async function withTemplaterFileCreationSuppressed( filePath: string, fn: () => Promise, ): Promise { - const plugin = getTemplaterPlugin(app); - const pendingFiles = plugin?.templater?.files_with_pending_templates; - if ( - !plugin || - !isTemplaterTriggerOnCreateEnabled(app) || - !(pendingFiles instanceof Set) - ) { - return await fn(); - } - - activeTemplaterFileCreationSuppressions++; - - let state = templaterFileCreationSuppressions.get(filePath); - if (!state) { - state = { - count: 0, - hadPathInitially: pendingFiles.has(filePath), - }; - templaterFileCreationSuppressions.set(filePath, state); - - if (!state.hadPathInitially) { - pendingFiles.add(filePath); - } - } - - state.count++; - - let fnSucceeded = false; - try { - const result = await fn(); - fnSucceeded = true; - return result; - } finally { - state.count--; - activeTemplaterFileCreationSuppressions--; - - if (state.count <= 0) { - templaterFileCreationSuppressions.delete(filePath); - - if (!state.hadPathInitially) { - if (fnSucceeded) { - const minHoldMs = TEMPLATER_PENDING_CHECK_BUFFER_MS; - await sleep(minHoldMs); - } - - pendingFiles.delete(filePath); - - // By temporarily adding entries to Templater's internal Set, we can - // prevent its own teardown from firing when other tasks finish. - // When the Set is empty again (and no suppressions are active), - // emulate the "all templates executed" teardown to avoid leaving - // internal state around. - await maybeTeardownTemplaterAfterSuppression(app, plugin, pendingFiles); - } - } - } + return await getIntegrationRegistry(app).templater.withFileCreationSuppressed( + filePath, + fn, + ); } export async function waitForFileToStopChanging( @@ -307,161 +154,30 @@ export async function waitForFileToStopChanging( } } -const templaterRenderLocks = new Map>(); - -async function withTemplaterFileLock( - filePath: string, - fn: () => Promise, -): Promise { - const previous = templaterRenderLocks.get(filePath) ?? Promise.resolve(); - let release!: () => void; - const current = new Promise((resolve) => { - release = () => resolve(); - }); - - const chain = previous - .catch(() => undefined) - .then(() => current); - - templaterRenderLocks.set(filePath, chain); - - chain - .finally(() => { - if (templaterRenderLocks.get(filePath) === chain) { - templaterRenderLocks.delete(filePath); - } - }) - .catch(() => undefined); - - await previous.catch(() => undefined); - try { - return await fn(); - } finally { - release(); - } -} - export async function overwriteTemplaterOnce( app: App, file: TFile, opts: { skipIfNoTags?: boolean; postWait?: boolean } = {}, ): Promise { - if (file.extension !== "md") return; - - const plugin = getTemplaterPlugin(app); - const templater = plugin?.templater; - const overwrite = templater?.overwrite_file_commands; - if (!plugin || !templater || typeof overwrite !== "function") return; - - const { skipIfNoTags = true, postWait = true } = opts; - - await withTemplaterFileLock(file.path, async () => { - // Ensure the initial QuickAdd write is flushed & stable on disk. - await waitForFileSettle(app, file); - - let original: string; - try { - original = await app.vault.read(file); - } catch (err) { - reportError( - err as Error, - `overwriteTemplaterOnce: failed to read ${file.path} before render`, - ); - return; - } - - if (skipIfNoTags && !original.includes("<%")) { - return; - } - - try { - // Preserve Templater's internal `this` context. - await overwrite.call(templater, file); - if (postWait) { - await waitForFileSettle(app, file, 800); - } - } catch (err) { - // Roll back to original content to avoid partial renders - try { - await app.vault.modify(file, original); - } catch (rollbackErr) { - log.logWarning( - `Failed to rollback ${file.path} after Templater error: ${(rollbackErr as Error).message}`, - ); - } - reportError( - err as Error, - `Templater failed on ${file.path}. Rolled back to pre-render state.`, - ); - } - }); + await getIntegrationRegistry(app).templater.overwriteFileOnce(file, opts); } export async function templaterParseTemplate( app: App, templateContent: string, targetFile: TFile, -) { - if (targetFile.extension !== "md") return templateContent; - - const plugin = getTemplaterPlugin(app); - const templater = plugin?.templater; - const parseTemplate = templater?.parse_template; - if (!plugin || !templater || typeof parseTemplate !== "function") - return templateContent; - - // Use Templater's create_running_config if available for forward compatibility. - // This ensures we get a properly initialized config object with all required fields, - // even if Templater adds new required fields in future versions. - // Fallback to manual config for older Templater versions. - const createConfig = templater.create_running_config; - const config = - typeof createConfig === "function" - ? createConfig.call(templater, undefined, targetFile, 4) - : // `run_mode: 4` = RunMode.DynamicProcessor - // `frontmatter: {}` required since Templater 2.18.0 - { target_file: targetFile, run_mode: 4, frontmatter: {} }; - - return await parseTemplate.call(templater, config, templateContent); +): Promise { + return await getIntegrationRegistry(app).templater.parseTemplate( + templateContent, + targetFile, + ); } export async function jumpToNextTemplaterCursorIfPossible( app: App, file: TFile, ): Promise { - if (file.extension !== "md") return; - if (app.workspace.getActiveFile()?.path !== file.path) return; - - const plugin = getTemplaterPlugin(app); - const autoJumpEnabled = !!plugin?.settings?.auto_jump_to_cursor; - const editorHandler = plugin?.editor_handler; - const jump = editorHandler?.jump_to_next_cursor_location; - - if (!autoJumpEnabled) return; - - if (typeof jump === "function") { - try { - // Preserve Templater's internal `this` context. - await jump.call(editorHandler, file, true); - return; - } catch (err) { - log.logWarning( - `jumpToNextTemplaterCursorIfPossible: API failed – ${(err as Error).message}`, - ); - } - } - - try { - ( - app.commands as unknown as { - executeCommandById?: (commandId: string) => boolean; - } - ).executeCommandById?.( - "templater-obsidian:jump-to-next-cursor-location", - ); - } catch { - // no-op - } + await getIntegrationRegistry(app).templater.jumpToNextCursorIfPossible(file); } export function getNaturalLanguageDates() {