diff --git a/docs/src/content/docs/docs/Advanced/scriptsWithSettings.md b/docs/src/content/docs/docs/Advanced/scriptsWithSettings.md index 9b8334d3d..9ffe178b4 100644 --- a/docs/src/content/docs/docs/Advanced/scriptsWithSettings.md +++ b/docs/src/content/docs/docs/Advanced/scriptsWithSettings.md @@ -82,6 +82,18 @@ How the pieces fit together: object describing the field. Add a `description` to any field to show help text beneath it. +### Scripts with several exports {#settings-with-member-access} + +A script can export more functions next to `entry` and `settings`. A macro +command named with `::`, such as `my-script::otherExport`, runs that export +instead of `entry`. The export still receives `settings` as its second +argument, and the gear still shows the script's settings. QuickAdd looks for +`settings` on the selected export first, then on each parent up to +`module.exports`, and uses the nearest one it finds - so an export without its +own `settings` uses the script's. +Settings are saved per macro command, so two commands for the same script keep +separate values. + ## The field types {#setting-types} Set each field's `type` to one of these: diff --git a/docs/src/content/docs/docs/Examples/Capture_FetchTasksFromTodoist.md b/docs/src/content/docs/docs/Examples/Capture_FetchTasksFromTodoist.md index bb54ae370..54fa7c375 100644 --- a/docs/src/content/docs/docs/Examples/Capture_FetchTasksFromTodoist.md +++ b/docs/src/content/docs/docs/Examples/Capture_FetchTasksFromTodoist.md @@ -20,7 +20,7 @@ The Todoist Script has three exports, `SelectFromAllTasks`, `GetAllTasksFromProj - `GetAllTasksFromProject` will prompt you for a project and get all tasks from that project, and - `GetAllTasksFromSection` will prompt you for a section and get all tasks from that section. -If you run the macro as-is, the script asks you which one to run. To always run one of them, reference it in the Capture format with `::`, for example `{{MACRO:Todoist::GetAllTasksFromProject}}`. +If you run the macro as-is, the script asks you which one to run. To always run one of them, reference it in the Capture format with `::`, for example `{{MACRO:Todoist::GetAllTasksFromProject}}`, or name it in the macro's script command, for example `todoistTaskSync::GetAllTasksFromProject`. :::caution[Imported tasks are completed in Todoist] By default, the script completes every task it imports, so the same task isn't imported twice. Recurring tasks move to their next occurrence instead. To keep the tasks open in Todoist, untick **Complete imported tasks in Todoist** in the script's settings. @@ -29,7 +29,7 @@ By default, the script completes every task it imports, so the same task isn't i ## Setup 1. Save the Todoist Script to your vault, for example as `scripts/todoistTaskSync.js`. -2. In **Settings → QuickAdd**, add a [Macro choice](/docs/Choices/MacroChoice/) named `Todoist`, and add the script to its command list. Add the script by its file name only - don't add `::GetAllTasksFromProject` there, because the script's settings are only available when the command points at the whole script. +2. In **Settings → QuickAdd**, add a [Macro choice](/docs/Choices/MacroChoice/) named `Todoist`, and add the script to its command list. Add it by its file name (`todoistTaskSync`) to pick an export when the macro runs, or append an export (`todoistTaskSync::GetAllTasksFromProject`) to always run that one. Either way, the script's settings apply. 3. Click the gear (⚙️) next to the script command, and paste your Todoist API token into **Todoist API token**. QuickAdd keeps it in Obsidian's secret storage, not in `data.json`. Leave **Complete imported tasks in Todoist** ticked, or untick it to leave tasks open in Todoist. ![Todoist script settings](../Images/Todoist-ScriptSettings.png) @@ -52,7 +52,7 @@ If there isn't a date set for the task, they'll simply be entered as `- [ ] Buy ## Troubleshooting -- **"Add your Todoist API token in the Todoist script's settings"**: open the macro, click the gear next to the script command, and paste the token. If the script command's name ends in `::SomeExport`, remove that part so the gear shows the settings. +- **"Add your Todoist API token in the Todoist script's settings"**: open the macro, click the gear next to the script command, and paste the token. Settings belong to each script command, so if the macro has several commands for this script, set the token on the one that runs. - **"Todoist rejected the API token (HTTP 401)"**: the token is wrong or was reset. Copy it again from Todoist's Developer settings. - **"Secret setting ... is unavailable. Re-enter it on this device."**: secrets are stored per device. Paste the token again on this device. diff --git a/docs/src/content/docs/docs/UserScripts.md b/docs/src/content/docs/docs/UserScripts.md index c8be06d13..de29652dd 100644 --- a/docs/src/content/docs/docs/UserScripts.md +++ b/docs/src/content/docs/docs/UserScripts.md @@ -160,7 +160,10 @@ The `app` object provides access to the entire Obsidian API, including: ### The `settings` object {#settings-object} `settings` holds the user-configured values for your script's options. It's only -passed when you use the object structure with a `settings` block. +populated when your script defines a `settings` block, as in the object +structure above. An export picked with `::` (for example `my-script::start`) +gets the values too: QuickAdd uses the nearest `settings` block on the export +or its parents, up to `module.exports`. ## Let users configure your script {#configurable-options} diff --git a/src/IChoiceExecutor.ts b/src/IChoiceExecutor.ts index 6037cccc4..6662cde41 100644 --- a/src/IChoiceExecutor.ts +++ b/src/IChoiceExecutor.ts @@ -10,6 +10,7 @@ import type { PromptProvider } from "./interactive/promptProvider"; import type IMacroChoice from "./types/choices/IMacroChoice"; import type { ICommand } from "./types/macros/ICommand"; import type { PreparedChoiceInputState } from "./preflight/preparedChoiceInputs"; +import type { LoadedUserScript } from "./utils/userScript"; export interface IChoiceExecutor { execute(choice: IChoice): Promise; @@ -110,13 +111,14 @@ export interface IChoiceExecutor { * User-script modules already loaded (and therefore already EXECUTED - loading * a CommonJS user script runs its top-level code) by a requirement-collection * pass, keyed by `getUserScriptPreloadKey` (`command.path ?? command.id` plus - * any `::` member-drill suffix - stored values are DRILLED exports, so keying - * by path alone would collide different members of one file). MacroChoiceEngine + * any `::` member-drill suffix - stored values hold the DRILLED export plus the + * module's settings definition, so keying by path alone would collide + * different members of one file). MacroChoiceEngine * consumes an entry (delete-on-use) instead of re-loading the script, so * introspecting `quickadd.inputs` in the one-page preflight / non-interactive * CLI does not make a script's top-level side effects run twice per trigger. * Optional so existing stubs are unaffected; absent means "no preloaded * modules". */ - preloadedUserScripts?: Map; + preloadedUserScripts?: Map; } diff --git a/src/choiceExecutor.preload.test.ts b/src/choiceExecutor.preload.test.ts index 957fdcbd4..e2c66c734 100644 --- a/src/choiceExecutor.preload.test.ts +++ b/src/choiceExecutor.preload.test.ts @@ -41,7 +41,10 @@ describe("ChoiceExecutor preloadedUserScripts lifecycle", () => { // ran (cancelled modal / aborted macro): the entry must not survive to // the NEXT trigger on a long-lived executor, where it would hand the // engine a module loaded before the user's latest edits. - executor.preloadedUserScripts.set("stale.js", { entry: () => {} }); + executor.preloadedUserScripts.set("stale.js", { + script: { entry: () => {} }, + settings: undefined, + }); await executor.execute({ id: "unknown", @@ -57,7 +60,7 @@ describe("ChoiceExecutor preloadedUserScripts lifecycle", () => { { workspace: { getActiveFile: () => null } } as never, {} as never, ); - executor.preloadedUserScripts.set("outer.js", {}); + executor.preloadedUserScripts.set("outer.js", { script: {}, settings: undefined }); // Depth 2 -> 1: a nested execute() ending must NOT wipe the outer // run's preloaded modules. diff --git a/src/choiceExecutor.ts b/src/choiceExecutor.ts index e5bb9d179..4f19b5184 100644 --- a/src/choiceExecutor.ts +++ b/src/choiceExecutor.ts @@ -36,12 +36,13 @@ import VDateInputPrompt from "./gui/VDateInputPrompt/VDateInputPrompt"; import { planDateOrigin, dateFromStoredValue } from "./utils/resolveDateOrigin"; import { log } from "./logger/logManager"; import type { ICommand } from "./types/macros/ICommand"; +import type { LoadedUserScript } from "./utils/userScript"; import { withPreparedChoiceInputs, clearPreparedChoiceInputs, createPreparedChoiceInputState, getPreparedTemplateNoteSelection } from "./preflight/preparedChoiceInputs"; import { isTemplateChoice } from "./preflight/macroCommandRole"; import { shouldRunTemplateNoteDiscovery } from "./utils/templateNoteDiscoveryEligibility"; export class ChoiceExecutor implements IChoiceExecutor { - public variables: Map = new Map(); + public variables: Map = new Map(); public readonly preparedInputs = createPreparedChoiceInputState(); // Default to interactive so every GUI entry point (command palette, ribbon, // suggester) keeps its current prompt behaviour. Non-interactive callers (CLI @@ -55,7 +56,7 @@ export class ChoiceExecutor implements IChoiceExecutor { // consumed once by MacroChoiceEngine so a script's top-level code runs a // single time per trigger instead of once for introspection plus once for // execution (see IChoiceExecutor.preloadedUserScripts). - public readonly preloadedUserScripts = new Map(); + public readonly preloadedUserScripts = new Map(); public focusedProperty: FrontmatterPropertyTarget | null = null; public triggerContext: QuickAddTriggerContext | null = null; public clocks?: RunClocks; diff --git a/src/engine/MacroChoiceEngine.aiPromptContext.test.ts b/src/engine/MacroChoiceEngine.aiPromptContext.test.ts index 8c72b93d9..e5fc12997 100644 --- a/src/engine/MacroChoiceEngine.aiPromptContext.test.ts +++ b/src/engine/MacroChoiceEngine.aiPromptContext.test.ts @@ -47,6 +47,7 @@ vi.mock("../formatters/completeFormatter", () => ({ })); vi.mock("../utilityObsidian", () => ({ getUserScript: vi.fn(), + loadUserScript: vi.fn(), openFile: vi.fn(), })); vi.mock("../quickAddInstance", () => ({ diff --git a/src/engine/MacroChoiceEngine.entry.test.ts b/src/engine/MacroChoiceEngine.entry.test.ts index 495c2629a..ee57bf19b 100644 --- a/src/engine/MacroChoiceEngine.entry.test.ts +++ b/src/engine/MacroChoiceEngine.entry.test.ts @@ -13,10 +13,12 @@ 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 type * as UserScriptModule from "../utils/userScript"; +import type { LoadedUserScript } from "../utils/userScript"; -const { mockGetUserScript, mockInitializeUserScriptSettings, mockSuggest, mockGetApi, mockInputPrompt } = +const { mockLoadModuleExports, mockInitializeUserScriptSettings, mockSuggest, mockGetApi, mockInputPrompt } = vi.hoisted(() => ({ - mockGetUserScript: vi.fn(), + mockLoadModuleExports: vi.fn(), mockInitializeUserScriptSettings: vi.fn(), mockSuggest: vi.fn(), mockGetApi: vi.fn(() => ({})), @@ -27,10 +29,23 @@ vi.mock("../utilityObsidian", async () => { const actual = await vi.importActual>( "../utilityObsidian", ); + const { getUserScriptMemberAccess, selectUserScriptMember } = + await vi.importActual( + "../utils/userScript", + ); return { ...actual, - getUserScript: mockGetUserScript, + // Fake only reading + evaluating the file (mockLoadModuleExports returns + // `module.exports`); the `::` drill and settings lookup run for real. + loadUserScript: async (command: IUserScript) => { + const moduleExports: unknown = await mockLoadModuleExports(command); + if (!moduleExports) return undefined; + return selectUserScriptMember( + moduleExports, + getUserScriptMemberAccess(command.name).memberAccess ?? [], + ); + }, }; }); @@ -133,7 +148,7 @@ describe("MacroChoiceEngine user script entry handling", () => { beforeEach(() => { vi.clearAllMocks(); - mockGetUserScript.mockReset(); + mockLoadModuleExports.mockReset(); mockInitializeUserScriptSettings.mockReset(); mockSuggest.mockReset(); mockGetApi.mockReset(); @@ -174,7 +189,7 @@ describe("MacroChoiceEngine user script entry handling", () => { { name: "clears previous output when the script returns undefined", callable: true, expected: undefined }, ])("$name", async ({ callable, expected }) => { const script = vi.fn().mockResolvedValue(undefined); - mockGetUserScript.mockResolvedValue(callable ? script : undefined); + mockLoadModuleExports.mockResolvedValue(callable ? script : undefined); const engine = new MacroChoiceEngine(app, plugin, macroChoice, choiceExecutor, variables); engine.setOutput("previous"); await engine["executeUserScript"](userScriptCommand); @@ -185,7 +200,7 @@ describe("MacroChoiceEngine user script entry handling", () => { it("runs the entry export without prompting when no settings are defined", async () => { const entryFn = vi.fn().mockResolvedValue("entry-result"); - mockGetUserScript.mockResolvedValue({ + mockLoadModuleExports.mockResolvedValue({ entry: entryFn, }); @@ -214,8 +229,8 @@ describe("MacroChoiceEngine user script entry handling", () => { // ONCE so a later run of the same command loads fresh. it("consumes a preloaded user-script module instead of re-loading it", async () => { const entryFn = vi.fn().mockResolvedValue("entry-result"); - const preloaded = new Map([ - ["script.js", { entry: entryFn }], + const preloaded = new Map([ + ["script.js", { script: { entry: entryFn }, settings: undefined }], ]); const engine = new MacroChoiceEngine( @@ -229,27 +244,27 @@ describe("MacroChoiceEngine user script entry handling", () => { await engine["executeUserScript"](userScriptCommand); - expect(mockGetUserScript).not.toHaveBeenCalled(); + expect(mockLoadModuleExports).not.toHaveBeenCalled(); expect(entryFn).toHaveBeenCalledTimes(1); // Delete-on-use: the preloaded execution is spent. expect(preloaded.has("script.js")).toBe(false); // A second execution of the same command loads (and thus runs) fresh. - mockGetUserScript.mockResolvedValue({ entry: entryFn }); + mockLoadModuleExports.mockResolvedValue({ entry: entryFn }); await engine["executeUserScript"](userScriptCommand); - expect(mockGetUserScript).toHaveBeenCalledTimes(1); + expect(mockLoadModuleExports).toHaveBeenCalledTimes(1); }); - // Preloaded values are member-DRILLED exports, so a command drilling a + // Preloaded values hold member-DRILLED exports, so a command drilling a // different `::` member of the same file must NOT consume another // member's entry (path-only keying executed the wrong function). it("does not consume a preloaded entry cached for a different :: member", async () => { const fooEntry = vi.fn().mockResolvedValue("foo-result"); const barEntry = vi.fn().mockResolvedValue("bar-result"); - const preloaded = new Map([ - ["script.js::foo", { entry: fooEntry }], + const preloaded = new Map([ + ["script.js::foo", { script: { entry: fooEntry }, settings: undefined }], ]); - mockGetUserScript.mockResolvedValue({ entry: barEntry }); + mockLoadModuleExports.mockResolvedValue({ bar: { entry: barEntry } }); const engine = new MacroChoiceEngine( app, @@ -264,7 +279,7 @@ describe("MacroChoiceEngine user script entry handling", () => { await engine["executeUserScript"](barCommand); // bar must load fresh (and run barEntry), leaving foo's entry intact. - expect(mockGetUserScript).toHaveBeenCalledTimes(1); + expect(mockLoadModuleExports).toHaveBeenCalledTimes(1); expect(barEntry).toHaveBeenCalledTimes(1); expect(fooEntry).not.toHaveBeenCalled(); expect(preloaded.has("script.js::foo")).toBe(true); @@ -272,7 +287,7 @@ describe("MacroChoiceEngine user script entry handling", () => { const fooCommand = { ...userScriptCommand, name: "Script::foo" }; await engine["executeUserScript"](fooCommand); expect(fooEntry).toHaveBeenCalledTimes(1); - expect(mockGetUserScript).toHaveBeenCalledTimes(1); + expect(mockLoadModuleExports).toHaveBeenCalledTimes(1); expect(preloaded.has("script.js::foo")).toBe(false); }); @@ -285,7 +300,7 @@ describe("MacroChoiceEngine user script entry handling", () => { }, }; - mockGetUserScript.mockResolvedValue({ + mockLoadModuleExports.mockResolvedValue({ entry: entryFn, settings, }); @@ -327,7 +342,7 @@ describe("MacroChoiceEngine user script entry handling", () => { "API Key": "legacy-secret", }; - mockGetUserScript.mockResolvedValue({ + mockLoadModuleExports.mockResolvedValue({ entry: entryFn, settings: { options: { @@ -364,7 +379,7 @@ describe("MacroChoiceEngine user script entry handling", () => { it("ignores malformed primitive settings exports at the user-script boundary", async () => { const entryFn = vi.fn().mockResolvedValue("entry-result"); - mockGetUserScript.mockResolvedValue({ + mockLoadModuleExports.mockResolvedValue({ entry: entryFn, settings: "not-settings", }); @@ -389,7 +404,7 @@ describe("MacroChoiceEngine user script entry handling", () => { it("prompts the user when no entry export is defined", async () => { const optionFn = vi.fn().mockResolvedValue("option-result"); - mockGetUserScript.mockResolvedValue({ + mockLoadModuleExports.mockResolvedValue({ option1: optionFn, }); mockSuggest.mockResolvedValueOnce("option1"); @@ -422,7 +437,7 @@ describe("MacroChoiceEngine user script variable propagation", () => { beforeEach(() => { vi.clearAllMocks(); - mockGetUserScript.mockReset(); + mockLoadModuleExports.mockReset(); mockInitializeUserScriptSettings.mockReset(); mockSuggest.mockReset(); @@ -464,7 +479,7 @@ describe("MacroChoiceEngine user script variable propagation", () => { } as IMacro, }; - mockGetUserScript.mockImplementation((command: IUserScript) => { + mockLoadModuleExports.mockImplementation((command: IUserScript) => { const nextValueByPath: Record = { "script-1.js": 1, "script-2.js": 2, @@ -548,7 +563,7 @@ describe("MacroChoiceEngine user script variable propagation", () => { } as IMacro, }; - mockGetUserScript.mockImplementationOnce(() => { + mockLoadModuleExports.mockImplementationOnce(() => { return Promise.resolve(async (params: { variables: Record }) => { params.variables = { foo: "bar" }; }); @@ -592,7 +607,7 @@ describe("MacroChoiceEngine user script variable propagation", () => { } as IMacro, }; - mockGetUserScript.mockImplementationOnce(() => { + mockLoadModuleExports.mockImplementationOnce(() => { return Promise.resolve(async (params: { variables: Record }) => { params.variables = params.variables; params.variables.added = 2; @@ -637,7 +652,7 @@ describe("MacroChoiceEngine user script variable propagation", () => { } as IMacro, }; - mockGetUserScript.mockImplementationOnce(() => { + mockLoadModuleExports.mockImplementationOnce(() => { return Promise.resolve(async (params: { variables: any }) => { params.variables = 123; params.variables.added = "ok"; @@ -682,7 +697,7 @@ describe("MacroChoiceEngine user script variable propagation", () => { } as IMacro, }; - mockGetUserScript.mockImplementationOnce(() => { + mockLoadModuleExports.mockImplementationOnce(() => { return Promise.resolve(async (params: { variables: any }) => { params.variables = new Map([ [1, "nope"], diff --git a/src/engine/MacroChoiceEngine.openFilePath.audit-macro.test.ts b/src/engine/MacroChoiceEngine.openFilePath.audit-macro.test.ts index e295f5189..46da0d2fe 100644 --- a/src/engine/MacroChoiceEngine.openFilePath.audit-macro.test.ts +++ b/src/engine/MacroChoiceEngine.openFilePath.audit-macro.test.ts @@ -49,6 +49,7 @@ vi.mock("../formatters/completeFormatter", () => ({ })); vi.mock("../utilityObsidian", () => ({ getUserScript: vi.fn(), + loadUserScript: vi.fn(), openFile: openFileMock, })); vi.mock("../quickAddInstance", () => ({ diff --git a/src/engine/MacroChoiceEngine.ts b/src/engine/MacroChoiceEngine.ts index 5da7cdaab..ae630c7cb 100644 --- a/src/engine/MacroChoiceEngine.ts +++ b/src/engine/MacroChoiceEngine.ts @@ -10,6 +10,7 @@ import { reportError } from "../utils/errorUtils"; import { CommandType } from "../types/macros/CommandType"; import { QuickAddApi } from "../quickAddApi"; import type { ICommand } from "../types/macros/ICommand"; +import type { LoadedUserScript } from "../utils/userScript"; import { executeUserScript, type ScriptParameters } from "./userScriptExecution"; import { QuickAddChoiceEngine } from "./QuickAddChoiceEngine"; import type { IMacro } from "../types/macros/IMacro"; @@ -111,7 +112,7 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { protected choiceExecutor: IChoiceExecutor; protected readonly plugin: QuickAdd; private conditionalScriptCache = new Map(); - private readonly preloadedUserScripts: Map; + private readonly preloadedUserScripts: Map; private readonly promptLabel?: string; private buildParams( app: App, @@ -185,7 +186,7 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine { choice: IMacroChoice, choiceExecutor: IChoiceExecutor, variables: Map, - preloadedUserScripts?: Map, + preloadedUserScripts?: Map, promptLabel?: string, private readonly originLeaf: WorkspaceLeaf | null = null, ) { diff --git a/src/engine/userScriptExecution.test.ts b/src/engine/userScriptExecution.test.ts new file mode 100644 index 000000000..3dcf6aac2 --- /dev/null +++ b/src/engine/userScriptExecution.test.ts @@ -0,0 +1,120 @@ +import { TFile, type App } from "obsidian"; +import { describe, expect, it, vi } from "vitest"; +import type { IChoiceExecutor } from "../IChoiceExecutor"; +import type QuickAdd from "../main"; +import { CommandType } from "../types/macros/CommandType"; +import type { IUserScript } from "../types/macros/IUserScript"; +import { executeUserScript, type ScriptParameters } from "./userScriptExecution"; + +// Same shape as docs/public/scripts/TodoistScript.js: `settings` lives on +// module.exports next to `entry` and the member functions. +const SCRIPT_SOURCE = ` +const API_TOKEN = "Todoist API token"; +const COMPLETE_TASKS = "Complete imported tasks in Todoist"; +module.exports = { + entry: async () => "entry ran", + settings: { + name: "Todoist", + options: { + [API_TOKEN]: { type: "secret", id: "todoist-api-token" }, + [COMPLETE_TASKS]: { type: "checkbox", defaultValue: true }, + }, + }, + GetAllTasksFromProject: async (params, settings) => { + params.received.push({ ...settings }); + return "member ran"; + }, +}; +`; + +function createApp(secrets: Map): App { + const file = new TFile(); + file.path = "scripts/todoist.js"; + file.extension = "js"; + return { + vault: { + getAbstractFileByPath: vi.fn(() => file), + read: vi.fn(async () => SCRIPT_SOURCE), + }, + secretStorage: { + getSecret: vi.fn((id: string) => secrets.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => void secrets.set(id, value)), + }, + } as unknown as App; +} + +async function run(command: IUserScript, app: App) { + const received: Record[] = []; + const result = await executeUserScript(command, { + app, + plugin: { saveSettings: vi.fn() } as unknown as QuickAdd, + choiceName: "Todoist", + params: { received } as unknown as ScriptParameters, + executor: {} as IChoiceExecutor, + preloadedUserScripts: new Map(), + }); + return { result, received }; +} + +describe("executeUserScript settings with `Script::Export` member access", () => { + function createCommand(settings: Record): IUserScript { + return { + id: "todoist-command", + name: "todoistTaskSync::GetAllTasksFromProject", + type: CommandType.UserScript, + path: "scripts/todoist.js", + settings, + }; + } + + // Regression: the drilled export is a bare function without `.settings`, so + // defaults were never initialized and the member got `{}`. + it("initializes defaults from the module's settings for a drilled member", async () => { + const command = createCommand({}); + + const { result, received } = await run(command, createApp(new Map())); + + expect(result).toEqual({ output: "member ran" }); + expect(received).toEqual([ + { "Complete imported tasks in Todoist": true }, + ]); + expect(command.settings).toEqual({ + "Complete imported tasks in Todoist": true, + }); + }); + + it("passes a resolved secret and keeps a user-changed checkbox", async () => { + const secrets = new Map([["stored-token-id", "fake-token"]]); + const command = createCommand({ + "Todoist API token": { + __quickaddSecret: true, + secretRef: "stored-token-id", + }, + "Complete imported tasks in Todoist": false, + }); + + const { received } = await run(command, createApp(secrets)); + + expect(received).toEqual([ + { + "Todoist API token": "fake-token", + "Complete imported tasks in Todoist": false, + }, + ]); + }); + + // Secret migration needs the definition to know which settings are secret; + // without it a legacy plaintext token stayed in data.json. + it("migrates a legacy plaintext secret for a drilled member", async () => { + const secrets = new Map(); + const command = createCommand({ "Todoist API token": "legacy-token" }); + + const { received } = await run(command, createApp(secrets)); + + expect(received[0]["Todoist API token"]).toBe("legacy-token"); + expect(command.settings["Todoist API token"]).toMatchObject({ + __quickaddSecret: true, + }); + expect([...secrets.values()]).toEqual(["legacy-token"]); + }); +}); diff --git a/src/engine/userScriptExecution.ts b/src/engine/userScriptExecution.ts index 25145d0a9..390f35e85 100644 --- a/src/engine/userScriptExecution.ts +++ b/src/engine/userScriptExecution.ts @@ -4,8 +4,8 @@ import type { QuickAddApi } from "../quickAddApi"; import type QuickAdd from "../main"; import type { IChoiceExecutor } from "../IChoiceExecutor"; import type { IUserScript } from "../types/macros/IUserScript"; -import { getUserScript } from "../utilityObsidian"; -import { getUserScriptPreloadKey } from "../utils/userScript"; +import { loadUserScript } from "../utilityObsidian"; +import { getUserScriptPreloadKey, type LoadedUserScript } from "../utils/userScript"; import { initializeUserScriptSettings } from "../utils/userScriptSettings"; import { resolveScriptSettings } from "./userScriptSettings"; import { log } from "../logger/logManager"; @@ -31,7 +31,7 @@ type ScriptContext = { choiceName: string; params: ScriptParameters; executor: IChoiceExecutor; - preloadedUserScripts: Map; + preloadedUserScripts: Map; promptLabel?: string; }; @@ -53,17 +53,19 @@ export async function executeUserScript( const { app, plugin, choiceName, params, executor, preloadedUserScripts, promptLabel } = context; // Preloaded exports are member-specific and consumed once. const cacheKey = getUserScriptPreloadKey(command); - let userScript = cacheKey === undefined ? undefined : preloadedUserScripts.get(cacheKey); - if (cacheKey !== undefined && userScript !== undefined) preloadedUserScripts.delete(cacheKey); - if (userScript === undefined) userScript = await getUserScript(command, app); + let loaded = cacheKey === undefined ? undefined : preloadedUserScripts.get(cacheKey); + if (cacheKey !== undefined && loaded !== undefined) preloadedUserScripts.delete(cacheKey); + if (loaded === undefined) loaded = await loadUserScript(command, app); + const userScript = loaded?.script; if (!userScript) { log.logError(`failed to load user script ${command.path}.`); return; } if (!command.settings) command.settings = {}; - const settingsExport = isRecord(userScript) ? userScript.settings : undefined; - const definition = isRecord(settingsExport) ? settingsExport : undefined; + // Read from the module, not the `::`-drilled export (a bare function for + // `Script::Export`), so defaults and secrets apply to member access too. + const definition = loaded?.settings; if (definition) initializeUserScriptSettings(command.settings, definition); async function invoke(fn: UserScriptFunction): Promise { diff --git a/src/gui/MacroGUIs/CommandList.svelte b/src/gui/MacroGUIs/CommandList.svelte index 34e5c87a0..4e083d4b1 100644 --- a/src/gui/MacroGUIs/CommandList.svelte +++ b/src/gui/MacroGUIs/CommandList.svelte @@ -21,7 +21,7 @@ import UserScriptCommand from "./Components/UserScriptCommand.svelte"; import type { IUserScript } from "../../types/macros/IUserScript"; import { UserScriptSettingsModal } from "./UserScriptSettingsModal"; import { log } from "../../logger/logManager"; -import { getUserScript } from "src/utilityObsidian"; +import { loadUserScript } from "src/utilityObsidian"; import type { IAIAssistantCommand } from "src/types/macros/QuickCommands/IAIAssistantCommand"; import AIAssistantCommand from "./Components/AIAssistantCommand.svelte"; import { AIAssistantCommandSettingsModal } from "./AIAssistantCommandSettingsModal"; @@ -239,14 +239,15 @@ function getChoiceBuilder(choice: IChoice) { } async function configureScript(command: IUserScript) { - const userScript = await getUserScript(command, app); - if (!userScript) { + const loaded = await loadUserScript(command, app); + if (!loaded?.script) { log.logWarning(`${command.name} could not be loaded.`); return; } - const scriptSettings = - (userScript as { settings?: { [key: string]: unknown } }).settings ?? {}; + // The settings definition lives on the module; `Script::Export` drills + // to the export that runs, which usually has no `settings` of its own. + const scriptSettings = loaded.settings ?? {}; new UserScriptSettingsModal( app, diff --git a/src/preflight/collectChoiceRequirements.audit-preflight-suggesters.test.ts b/src/preflight/collectChoiceRequirements.audit-preflight-suggesters.test.ts index 9f2443cda..813ebfa93 100644 --- a/src/preflight/collectChoiceRequirements.audit-preflight-suggesters.test.ts +++ b/src/preflight/collectChoiceRequirements.audit-preflight-suggesters.test.ts @@ -8,7 +8,7 @@ vi.mock("src/utilityObsidian", () => ({ getMarkdownFilesMatchingFilter: vi.fn(() => []), getMarkdownFilesWithTag: vi.fn(() => []), getMarkdownFilesWithProperty: vi.fn(() => []), - getUserScript: vi.fn(), + loadUserScript: vi.fn(), getTemplateFile: vi.fn(() => null), isFolder: vi.fn(() => false), })); diff --git a/src/preflight/collectChoiceRequirements.test.ts b/src/preflight/collectChoiceRequirements.test.ts index 4aaef7514..638bb8553 100644 --- a/src/preflight/collectChoiceRequirements.test.ts +++ b/src/preflight/collectChoiceRequirements.test.ts @@ -13,6 +13,7 @@ import { CommandType } from "src/types/macros/CommandType"; import type { IChoiceCommand } from "src/types/macros/IChoiceCommand"; import type { ICommand } from "src/types/macros/ICommand"; import type { IUserScript } from "src/types/macros/IUserScript"; +import type { LoadedUserScript } from "src/utils/userScript"; import type { IConditionalCommand } from "src/types/macros/Conditional/IConditionalCommand"; import type { INestedChoiceCommand } from "src/types/macros/QuickCommands/INestedChoiceCommand"; import type IChoice from "src/types/choices/IChoice"; @@ -54,7 +55,12 @@ vi.mock("src/utilityObsidian", () => ({ getMarkdownFilesMatchingFilter: getMarkdownFilesMatchingFilterMock, getMarkdownFilesWithTag: getMarkdownFilesWithTagMock, getMarkdownFilesWithProperty: getMarkdownFilesWithPropertyMock, - getUserScript: getUserScriptMock, + // getUserScriptMock returns the `::`-drilled export; the collector only + // reads quickadd.inputs from it, so the settings definition is irrelevant. + loadUserScript: async (...args: unknown[]) => { + const script: unknown = await getUserScriptMock(...args); + return script === undefined ? undefined : { script, settings: undefined }; + }, getTemplateFile: getTemplateFileMock, isFolder: isFolderMock, })); @@ -558,12 +564,12 @@ describe("collectChoiceRequirements - macro script metadata", () => { }, }; getUserScriptMock.mockResolvedValue(exported); - const preloadedUserScripts = new Map(); + const preloadedUserScripts = new Map(); await collect(createMacroChoice(scriptCommand), choiceExecutor, { preloadedUserScripts }); expect(getUserScriptMock).toHaveBeenCalledTimes(1); - expect(preloadedUserScripts.get("script.js")).toBe(exported); + expect(preloadedUserScripts.get("script.js")?.script).toBe(exported); // A second collection pass (e.g. CLI collect followed by the one-page // preflight) must reuse the cached module, not execute it again. @@ -573,7 +579,7 @@ describe("collectChoiceRequirements - macro script metadata", () => { expectCollectedFields(requirements, { id: "project" }); }); - // getUserScript returns the `::`-member-DRILLED export, so the cache key + // The cached `script` is the `::`-member-DRILLED export, so the cache key // must include the drill: two commands sharing a path but drilling // different members hold different functions with different inputs, and // caching by path alone made the second command reuse the first member's @@ -589,7 +595,7 @@ describe("collectChoiceRequirements - macro script metadata", () => { getUserScriptMock .mockResolvedValueOnce(fooExport) .mockResolvedValueOnce(barExport); - const preloadedUserScripts = new Map(); + const preloadedUserScripts = new Map(); const macroChoice = createMacroChoice(scriptCommand); macroChoice.macro.commands = [ @@ -600,8 +606,8 @@ describe("collectChoiceRequirements - macro script metadata", () => { const requirements = await collect(macroChoice, choiceExecutor, { preloadedUserScripts }); expect(getUserScriptMock).toHaveBeenCalledTimes(2); - expect(preloadedUserScripts.get("script.js::foo")).toBe(fooExport); - expect(preloadedUserScripts.get("script.js::bar")).toBe(barExport); + expect(preloadedUserScripts.get("script.js::foo")?.script).toBe(fooExport); + expect(preloadedUserScripts.get("script.js::bar")?.script).toBe(barExport); expectCollectedFields(requirements, { id: "fooInput" }, { id: "barInput" }); }); diff --git a/src/preflight/collectChoiceRequirements.ts b/src/preflight/collectChoiceRequirements.ts index 7bc5fa137..ac50fe897 100644 --- a/src/preflight/collectChoiceRequirements.ts +++ b/src/preflight/collectChoiceRequirements.ts @@ -22,13 +22,14 @@ import type { IUserScript } from "src/types/macros/IUserScript"; import { shouldLeaveTemplateTitleForDiscovery } from "src/utils/templateNoteDiscoveryEligibility"; import { getTemplateFile, - getUserScript, isFolder, + loadUserScript, } from "src/utilityObsidian"; import { log } from "src/logger/logManager"; import { getUserScriptPreloadKey, isUserScriptLoadError, + type LoadedUserScript, } from "src/utils/userScript"; import { hasTemplatePathSyntax } from "src/utils/templatePathSyntax"; import { @@ -70,7 +71,7 @@ interface CollectChoiceRequirementsOptions { * body twice: once here for introspection and once in MacroChoiceEngine. * The engine consumes these entries instead of re-loading (delete-on-use). */ - preloadedUserScripts?: Map; + preloadedUserScripts?: Map; } async function readTemplate(app: App, path: string): Promise { @@ -382,29 +383,29 @@ async function collectForCaptureChoice( async function collectUserScriptRequirements( app: App, userScriptCommand: IUserScript, - preloadedUserScripts?: Map, + preloadedUserScripts?: Map, ): Promise { const requirements: FieldRequirement[] = []; try { // Reuse an already-loaded module (loading executes the script's // top-level code); cache what we load so the runtime engine consumes // this execution instead of running the module body a second time. - // The key is member-aware (path + `::` drill) because getUserScript - // returns the drilled export. + // The key is member-aware (path + `::` drill) because the cached + // `script` is the drilled export. const cacheKey = getUserScriptPreloadKey(userScriptCommand); - let exported = + let loaded = cacheKey !== undefined ? preloadedUserScripts?.get(cacheKey) : undefined; - if (exported === undefined) { - exported = await getUserScript(userScriptCommand, app, { + if (loaded === undefined) { + loaded = await loadUserScript(userScriptCommand, app, { reportLoadErrors: false, }); - if (cacheKey !== undefined && exported !== undefined) { - preloadedUserScripts?.set(cacheKey, exported); + if (cacheKey !== undefined && loaded !== undefined) { + preloadedUserScripts?.set(cacheKey, loaded); } } - const scriptInputs = getQuickAddScriptInputs(exported); + const scriptInputs = getQuickAddScriptInputs(loaded?.script); for (const input of scriptInputs) { const requirement = toFieldRequirement(input); if (requirement) requirements.push(requirement); diff --git a/src/preflight/runOnePagePreflight.fallback.test.ts b/src/preflight/runOnePagePreflight.fallback.test.ts index c9c24fe17..b1401d8a7 100644 --- a/src/preflight/runOnePagePreflight.fallback.test.ts +++ b/src/preflight/runOnePagePreflight.fallback.test.ts @@ -52,7 +52,7 @@ vi.mock("src/utilityObsidian", async () => { return { getMarkdownFilesInFolder: vi.fn(() => []), getMarkdownFilesWithTag: vi.fn(() => []), - getUserScript: vi.fn(), + loadUserScript: vi.fn(), isFolder: vi.fn(() => false), getTemplateFile: vi.fn((app: App, path: string) => { const f = app.vault.getAbstractFileByPath(path); diff --git a/src/preflight/runOnePagePreflight.filenamePreview.test.ts b/src/preflight/runOnePagePreflight.filenamePreview.test.ts index c55da2d71..adc202bc0 100644 --- a/src/preflight/runOnePagePreflight.filenamePreview.test.ts +++ b/src/preflight/runOnePagePreflight.filenamePreview.test.ts @@ -51,7 +51,7 @@ vi.mock("src/utilityObsidian", async () => { return { getMarkdownFilesInFolder: vi.fn(() => []), getMarkdownFilesWithTag: vi.fn(() => []), - getUserScript: vi.fn(), + loadUserScript: vi.fn(), isFolder: vi.fn(() => false), // A configured folder can hold {{DATE:}}, which the requirement scan // resolves through this helper; the preview renders {{DATE}} with it too. diff --git a/src/preflight/runOnePagePreflight.selection.test.ts b/src/preflight/runOnePagePreflight.selection.test.ts index 69190a13b..891fdd669 100644 --- a/src/preflight/runOnePagePreflight.selection.test.ts +++ b/src/preflight/runOnePagePreflight.selection.test.ts @@ -54,7 +54,7 @@ vi.mock("src/utilityObsidian", async () => { return { getMarkdownFilesInFolder: vi.fn(() => []), getMarkdownFilesWithTag: vi.fn(() => []), - getUserScript: vi.fn(), + loadUserScript: vi.fn(), isFolder: vi.fn(() => false), // Faithful to the real resolver: trim, strip a leading slash, append .md // only when no template extension is present, then resolve to a TFile. diff --git a/src/utilityObsidian.test.ts b/src/utilityObsidian.test.ts index f84aa9f24..5057777e7 100644 --- a/src/utilityObsidian.test.ts +++ b/src/utilityObsidian.test.ts @@ -11,6 +11,7 @@ import { areSameVaultFilePath, getAllFolderPathsInVault, getUserScript, + loadUserScript, normalizeVaultFilePath, getOpenFileOriginLeaf, openFile, @@ -248,6 +249,72 @@ describe("getUserScript", () => { ).rejects.toThrow("script rejected"); }); + // `Script::Export` drills to what runs, but the settings definition belongs + // to the module; the gear and execution both read it from here. + describe("loadUserScript settings definition", () => { + const rootSettings = { options: { Token: { type: "secret" } } }; + + it("reads settings from the module root when the drilled export has none", async () => { + const app = createUserScriptApp(` + module.exports = { + settings: ${JSON.stringify(rootSettings)}, + Export: async () => "ran", + }; + `); + + const loaded = await loadUserScript( + createUserScriptCommand({ name: "Script::Export" }), + app, + ); + + expect(typeof loaded?.script).toBe("function"); + expect(loaded?.settings).toEqual(rootSettings); + }); + + it("prefers the nearest settings along the drill path", async () => { + const app = createUserScriptApp(` + module.exports = { + settings: ${JSON.stringify(rootSettings)}, + group: { + settings: { options: { Nested: { type: "text" } } }, + run: () => "ran", + }, + }; + `); + + const nested = await loadUserScript( + createUserScriptCommand({ name: "Script::group::run" }), + app, + ); + const group = await loadUserScript( + createUserScriptCommand({ name: "Script::group" }), + app, + ); + + expect(nested?.settings).toEqual({ options: { Nested: { type: "text" } } }); + expect(group?.settings).toEqual({ options: { Nested: { type: "text" } } }); + }); + + it("reads settings attached to a function export and ignores non-object settings", async () => { + const functionRoot = await loadUserScript( + createUserScriptCommand(), + createUserScriptApp(` + const run = () => "ran"; + run.settings = ${JSON.stringify(rootSettings)}; + module.exports = run; + `), + ); + const primitiveSettings = await loadUserScript( + createUserScriptCommand({ name: "Script::run" }), + createUserScriptApp(`module.exports = { settings: "nope", run: () => 1 };`), + ); + + expect(functionRoot?.settings).toEqual(rootSettings); + expect(primitiveSettings?.settings).toBeUndefined(); + expect(typeof primitiveSettings?.script).toBe("function"); + }); + }); + it("loads a user script from a note's ```js code block (#1065)", async () => { const app = createUserScriptApp( [ diff --git a/src/utilityObsidian.ts b/src/utilityObsidian.ts index 1de72b19c..8677ba37f 100644 --- a/src/utilityObsidian.ts +++ b/src/utilityObsidian.ts @@ -59,7 +59,11 @@ export { openExistingFileTab, } from "./utils/fileOpening"; -export { getUserScript, getUserScriptMemberAccess } from "./utils/userScript"; +export { + getUserScript, + getUserScriptMemberAccess, + loadUserScript, +} from "./utils/userScript"; export { getAllFolderPathsInVault, diff --git a/src/utils/userScript.ts b/src/utils/userScript.ts index 87aee7454..be8c16c05 100644 --- a/src/utils/userScript.ts +++ b/src/utils/userScript.ts @@ -127,12 +127,13 @@ export function getUserScriptMemberAccess(fullMemberPath: string): { } /** - * Cache key for a preloaded user-script module (the map shared between the - * requirement collector and MacroChoiceEngine). It must include the `::` - * member drill from `command.name`, because getUserScript returns the - * DRILLED value: two commands sharing one path but drilling different - * members (`lib::foo` vs `lib::bar`) hold different functions and must - * never consume each other's preloaded entry. + * Cache key for a preloaded user script (the map shared between the + * requirement collector and MacroChoiceEngine; values are + * {@link LoadedUserScript}). It must include the `::` member drill from + * `command.name`, because the cached `script` is the DRILLED value: two + * commands sharing one path but drilling different members (`lib::foo` vs + * `lib::bar`) hold different functions and must never consume each other's + * preloaded entry. */ export function getUserScriptPreloadKey( command: IUserScript, @@ -145,13 +146,58 @@ export function getUserScriptPreloadKey( : base; } -// Slightly modified version of Templater's user script import implementation -// Source: https://github.com/SilentVoid13/Templater +/** + * A loaded user script: `script` is the value selected by the `::` member + * drill in `command.name` (what runs), and `settings` is the script's settings + * definition. The definition belongs to the module, not to the drilled + * export: `Script::Export` usually drills to a bare function, while + * `settings` lives on `module.exports`. So `settings` is taken from the + * nearest value along the drill path - the drilled export itself first, then + * each parent, ending at the module root - that exports a `settings` object. + */ +export type LoadedUserScript = { + script: unknown; + settings: Record | undefined; +}; + +function getOwnSettingsDefinition( + value: unknown, +): Record | undefined { + if (!isRecord(value) && typeof value !== "function") return undefined; + const settings = (value as { settings?: unknown }).settings; + return isRecord(settings) ? settings : undefined; +} + +export function selectUserScriptMember( + moduleExports: unknown, + memberAccess: readonly string[], +): LoadedUserScript { + let script = moduleExports; + let settings = getOwnSettingsDefinition(script); + for (const member of memberAccess) { + // Untyped CommonJS exports: a missing intermediate member throws, as before. + script = (script as Record)[member]; + settings = getOwnSettingsDefinition(script) ?? settings; + } + return { script, settings }; +} + +/** The drilled export only; use {@link loadUserScript} when settings matter. */ export async function getUserScript( command: IUserScript, app: App, options: GetUserScriptOptions = {}, ) { + return (await loadUserScript(command, app, options))?.script; +} + +// Slightly modified version of Templater's user script import implementation +// Source: https://github.com/SilentVoid13/Templater +export async function loadUserScript( + command: IUserScript, + app: App, + options: GetUserScriptOptions = {}, +): Promise { // @ts-ignore const file: TAbstractFile = app.vault.getAbstractFileByPath(command.path); if (!file) { @@ -218,24 +264,15 @@ export async function getUserScript( const userScript = exp["default"] || mod.exports; if (!userScript) return; - let script = userScript; const usesExplicitDefaultExport = Boolean(exp["default"]); - const { memberAccess } = getUserScriptMemberAccess(command.name); - const hasMemberAccess = Boolean(memberAccess && memberAccess.length > 0); - if (memberAccess && memberAccess.length > 0) { - let member: string; - while ((member = memberAccess.shift() as string)) { - //@ts-ignore - - script = script[member]; - } - } + const memberAccess = getUserScriptMemberAccess(command.name).memberAccess ?? []; + const loaded = selectUserScriptMember(userScript, memberAccess); if ( usesExplicitDefaultExport && - !hasMemberAccess && - !isRunnableUserScriptExport(script) + memberAccess.length === 0 && + !isRunnableUserScriptExport(loaded.script) ) { reportAndThrowUserScriptLoadError( defaultExportMessage(command.path), @@ -243,6 +280,6 @@ export async function getUserScript( ); } - return script; + return loaded; } } diff --git a/tests/e2e/macro-member-access.test.ts b/tests/e2e/macro-member-access.test.ts index 432382707..2b1a35ad7 100644 --- a/tests/e2e/macro-member-access.test.ts +++ b/tests/e2e/macro-member-access.test.ts @@ -242,3 +242,55 @@ describe("issue 964: member access across macro user scripts", () => { expect(content.trim()).toBe("SECOND_BETA"); }); }); + +describe("`Script::Export` macro commands read the module's settings", () => { + const choiceId = `${TEST_PREFIX}member-settings-macro`; + + beforeAll(async () => { + const outputPath = sandbox.path("member-settings-output.md"); + // Mirrors docs/public/scripts/TodoistScript.js: `settings` lives on + // module.exports, beside the member the command drills to. + await seedFile( + "member-settings-script.js", + [ + "module.exports = {", + " settings: { name: 'Member settings', options: {", + " 'Complete tasks': { type: 'checkbox', defaultValue: true },", + " 'Label': { type: 'text', defaultValue: 'from-default' },", + " } },", + " entry: async () => 'ENTRY_SHOULD_NOT_RUN',", + " Export: async (params, settings) => {", + ` await params.app.vault.create(${JSON.stringify(outputPath)}, JSON.stringify(settings));`, + " },", + "};", + ].join("\n"), + ); + + await qa.data().patch((data) => { + data.choices = data.choices.filter((choice) => choice.id !== choiceId); + data.choices.push( + macroChoice(choiceId, [ + { + path: sandbox.path("member-settings-script.js"), + name: "member-settings-script::Export", + }, + ]), + ); + }); + + await qa.reload({ waitUntilReady: true }); + }, 15_000); + + it("passes the module's default settings to the drilled export", async () => { + const content = await runChoiceAndWaitForContent( + choiceId, + "member-settings-output.md", + "Complete tasks", + ); + + expect(JSON.parse(content)).toEqual({ + "Complete tasks": true, + Label: "from-default", + }); + }); +});