From bc14305459fc1f1ccb84ca46c2c52dbfbb13bd5b Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Fri, 20 Mar 2026 20:00:15 +0100 Subject: [PATCH 1/3] fix(macros): resolve member access across macro scripts --- member-access-owner-plan.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 member-access-owner-plan.md diff --git a/member-access-owner-plan.md b/member-access-owner-plan.md new file mode 100644 index 00000000..1a6fb759 --- /dev/null +++ b/member-access-owner-plan.md @@ -0,0 +1,36 @@ +# Member Access Namespace Pivot Plan + +## Tasks +- [x] T1 Remove owner-model schema and cleanup +- [x] T2 Runtime merged-namespace resolution +- [x] T3 Parser and conflict-selector rules +- [x] T4 UI/doc cleanup and new guidance +- [x] T5 Tests and verification + +## Work Log +- Removed the unshipped `exposeForMemberAccess` field from user-script types and deleted the related migration wiring. +- Reworked `SingleMacroEngine` so `Macro::member` searches all user scripts, errors on zero or multiple matches, and supports `Macro::Script Name::member` disambiguation. +- Preserved pre/post macro execution around the selected script, including selector-based execution on later scripts. +- Removed the owner toggle/badge UI and updated the unreleased macro docs to describe the merged-namespace model. +- Replaced the focused regression tests with unique-match, conflict, selector, duplicate-name, and selector-fallback coverage. +- Updated the vault-backed e2e fixtures to cover unique later-script lookup, ambiguous conflict failure, and qualified selector success. +- Rebuilt the plugin, copied the generated bundle to `/Users/christian/Developer/quickadd/`, reloaded QuickAdd in the `dev` vault, and reran focused e2e/CLI verification. + +## Files Modified Or Created +- `docs/docs/Choices/MacroChoice.md` +- `src/engine/SingleMacroEngine.ts` +- `src/engine/SingleMacroEngine.member-access.test.ts` +- `src/gui/MacroGUIs/CommandList.svelte` +- `src/gui/MacroGUIs/Components/UserScriptCommand.svelte` +- `src/gui/MacroGUIs/UserScriptSettingsModal.ts` +- `src/migrations/migrate.ts` +- `src/settings.ts` +- `src/types/macros/IUserScript.ts` +- `src/types/macros/UserScript.ts` +- `tests/e2e/macro-member-access.test.ts` +- `member-access-owner-plan.md` + +## Errors / Gotchas +- The `dev` vault still loads `/Users/christian/Developer/quickadd/main.js`, so verification required copying this worktree's built `main.js` and `styles.css` into that checkout before reloading the plugin. +- The first version of the qualified-selector e2e fixture used an object-export script as a pre-command, which opened the legacy object-picker prompt and blocked the test; the fixture was updated so the earlier ambiguous script is directly runnable. +- A live settings-modal DOM check in Obsidian kept surfacing a stale pre-reload modal instance that still contained the removed toggle text, even though the rebuilt bundle no longer contains the `Expose for member access` string. Functional CLI verification and source/bundle inspection matched the new implementation, but that one UI probe remained unreliable in the running app session. From 2276a9ee7002f0ef4f8a0e2717c8ca513aed10b7 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Fri, 20 Mar 2026 20:16:03 +0100 Subject: [PATCH 2/3] codex: remove plan file from PR (#1155) --- member-access-owner-plan.md | 36 ------------------------------------ 1 file changed, 36 deletions(-) delete mode 100644 member-access-owner-plan.md diff --git a/member-access-owner-plan.md b/member-access-owner-plan.md deleted file mode 100644 index 1a6fb759..00000000 --- a/member-access-owner-plan.md +++ /dev/null @@ -1,36 +0,0 @@ -# Member Access Namespace Pivot Plan - -## Tasks -- [x] T1 Remove owner-model schema and cleanup -- [x] T2 Runtime merged-namespace resolution -- [x] T3 Parser and conflict-selector rules -- [x] T4 UI/doc cleanup and new guidance -- [x] T5 Tests and verification - -## Work Log -- Removed the unshipped `exposeForMemberAccess` field from user-script types and deleted the related migration wiring. -- Reworked `SingleMacroEngine` so `Macro::member` searches all user scripts, errors on zero or multiple matches, and supports `Macro::Script Name::member` disambiguation. -- Preserved pre/post macro execution around the selected script, including selector-based execution on later scripts. -- Removed the owner toggle/badge UI and updated the unreleased macro docs to describe the merged-namespace model. -- Replaced the focused regression tests with unique-match, conflict, selector, duplicate-name, and selector-fallback coverage. -- Updated the vault-backed e2e fixtures to cover unique later-script lookup, ambiguous conflict failure, and qualified selector success. -- Rebuilt the plugin, copied the generated bundle to `/Users/christian/Developer/quickadd/`, reloaded QuickAdd in the `dev` vault, and reran focused e2e/CLI verification. - -## Files Modified Or Created -- `docs/docs/Choices/MacroChoice.md` -- `src/engine/SingleMacroEngine.ts` -- `src/engine/SingleMacroEngine.member-access.test.ts` -- `src/gui/MacroGUIs/CommandList.svelte` -- `src/gui/MacroGUIs/Components/UserScriptCommand.svelte` -- `src/gui/MacroGUIs/UserScriptSettingsModal.ts` -- `src/migrations/migrate.ts` -- `src/settings.ts` -- `src/types/macros/IUserScript.ts` -- `src/types/macros/UserScript.ts` -- `tests/e2e/macro-member-access.test.ts` -- `member-access-owner-plan.md` - -## Errors / Gotchas -- The `dev` vault still loads `/Users/christian/Developer/quickadd/main.js`, so verification required copying this worktree's built `main.js` and `styles.css` into that checkout before reloading the plugin. -- The first version of the qualified-selector e2e fixture used an object-export script as a pre-command, which opened the legacy object-picker prompt and blocked the test; the fixture was updated so the earlier ambiguous script is directly runnable. -- A live settings-modal DOM check in Obsidian kept surfacing a stale pre-reload modal instance that still contained the removed toggle text, even though the rebuilt bundle no longer contains the `Expose for member access` string. Functional CLI verification and source/bundle inspection matched the new implementation, but that one UI probe remained unreliable in the running app session. From 825890fa42d5dda93b2fe23504917a3d2dae4886 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Fri, 20 Mar 2026 20:24:24 +0100 Subject: [PATCH 3/3] codex: address PR review feedback (#1155) --- .../SingleMacroEngine.member-access.test.ts | 52 +++++++++++++++- src/engine/SingleMacroEngine.ts | 59 +++---------------- tests/e2e/macro-member-access.test.ts | 3 +- 3 files changed, 60 insertions(+), 54 deletions(-) diff --git a/src/engine/SingleMacroEngine.member-access.test.ts b/src/engine/SingleMacroEngine.member-access.test.ts index e187773d..b747e765 100644 --- a/src/engine/SingleMacroEngine.member-access.test.ts +++ b/src/engine/SingleMacroEngine.member-access.test.ts @@ -250,7 +250,7 @@ describe("SingleMacroEngine member access", () => { expect(engineInstance.runSubset).toHaveBeenNthCalledWith(2, [postCommand]); expect(engineInstance.setOutput).toHaveBeenCalledWith("export-result"); expect(result).toBe("export-result"); - expect(mockGetUserScript).toHaveBeenCalledTimes(2); + expect(mockGetUserScript).toHaveBeenCalledTimes(3); expect(mockGetUserScript).toHaveBeenCalledWith(firstScript, app); expect(mockGetUserScript).toHaveBeenCalledWith(secondScript, app); }); @@ -328,6 +328,52 @@ describe("SingleMacroEngine member access", () => { expect(mockGetUserScript).toHaveBeenCalledWith(scriptB, app); }); + it("loads a selector-targeted script after pre-commands run", async () => { + const preCommand = { + id: "wait-1", + name: "Wait", + type: CommandType.Wait, + } as ICommand; + const scriptA = createUserScript("script-a", "a.js", { + name: "Alpha Script", + }); + const scriptB = createUserScript("script-b", "b.js", { + name: "Beta Script", + }); + + let ready = false; + const engineInstance = macroEngineFactory(); + engineInstance.runSubset = vi.fn().mockImplementation(async () => { + ready = true; + }); + macroEngineFactory = () => engineInstance; + + mockGetUserScript.mockImplementation(async (command: IUserScript) => { + if (command.id !== "script-b") { + return { alpha: vi.fn() }; + } + + return ready + ? { beta: () => "late-bound-result" } + : { alpha: vi.fn() }; + }); + + const engine = new SingleMacroEngine( + app, + plugin, + [baseMacroChoice([preCommand, scriptA, scriptB])], + choiceExecutor, + ); + + const result = await engine.runAndGetOutput( + "My Macro::Beta Script::beta", + ); + + expect(result).toBe("late-bound-result"); + expect(mockGetUserScript).toHaveBeenCalledTimes(1); + expect(mockGetUserScript).toHaveBeenCalledWith(scriptB, app); + }); + it("aborts when a selector matches duplicate script names", async () => { const scriptA = createUserScript("script-a", "a.js", { name: "Shared Script", @@ -383,7 +429,7 @@ describe("SingleMacroEngine member access", () => { const result = await engine.runAndGetOutput("My Macro::NotAScript::beta"); expect(result).toBe("nested-result"); - expect(mockGetUserScript).toHaveBeenCalledTimes(2); + expect(mockGetUserScript).toHaveBeenCalledTimes(3); }); it("aborts when a selected script does not export the requested member", async () => { @@ -411,7 +457,7 @@ describe("SingleMacroEngine member access", () => { await expect( engine.runAndGetOutput("My Macro::Alpha Script::beta"), - ).rejects.toThrow("targeted script 'Alpha Script'"); + ).rejects.toThrow("routes member access to 'Alpha Script'"); }); it("propagates aborts when the export aborts", async () => { diff --git a/src/engine/SingleMacroEngine.ts b/src/engine/SingleMacroEngine.ts index f7c8ae75..a4d18d48 100644 --- a/src/engine/SingleMacroEngine.ts +++ b/src/engine/SingleMacroEngine.ts @@ -16,8 +16,6 @@ import { MacroAbortError } from "../errors/MacroAbortError"; type UserScriptCandidate = { command: IUserScript; index: number; - exportsRef?: unknown; - resolvedMember: { found: boolean; value?: unknown }; }; type MemberAccessSelection = { @@ -98,8 +96,6 @@ export class SingleMacroEngine { throw new Error(`macro '${macroName}' does not exist.`); } - const preloadedScripts = new Map(); - // Create a dedicated engine for this macro const engine = new MacroChoiceEngine( this.app, @@ -107,7 +103,7 @@ export class SingleMacroEngine { macroChoice, this.choiceExecutor, this.variables, - preloadedScripts, + undefined, context?.label, ); @@ -116,7 +112,6 @@ export class SingleMacroEngine { engine, macroChoice, memberAccess, - preloadedScripts, ); this.ensureNotAborted(); @@ -154,7 +149,6 @@ export class SingleMacroEngine { engine: MacroChoiceEngine, macroChoice: IMacroChoice, memberAccess: string[], - preloadedScripts: Map, ): Promise<{ executed: boolean; result?: unknown }> { const originalCommands = macroChoice.macro?.commands; if (!originalCommands?.length) { @@ -176,7 +170,6 @@ export class SingleMacroEngine { macroChoice, userScriptCommands, memberAccess, - preloadedScripts, ); const preCommands = originalCommands.slice(0, selection.candidate.index); @@ -199,10 +192,7 @@ export class SingleMacroEngine { userScriptCommand.settings = {}; } - const exportsRef = - selection.candidate.exportsRef !== undefined - ? selection.candidate.exportsRef - : await getUserScript(userScriptCommand, this.app); + const exportsRef = await getUserScript(userScriptCommand, this.app); if (exportsRef === undefined || exportsRef === null) { throw new MacroAbortError( @@ -210,11 +200,6 @@ export class SingleMacroEngine { ); } - const cacheKey = userScriptCommand.path ?? userScriptCommand.id; - if (cacheKey && exportsRef !== undefined && exportsRef !== null) { - preloadedScripts.set(cacheKey, exportsRef); - } - const settingsExport = typeof exportsRef === "object" || typeof exportsRef === "function" ? (exportsRef as Record).settings @@ -227,10 +212,10 @@ export class SingleMacroEngine { ); } - const resolvedMember = - selection.candidate.exportsRef !== undefined - ? selection.candidate.resolvedMember - : this.resolveMemberAccess(exportsRef, selection.memberAccess); + const resolvedMember = this.resolveMemberAccess( + exportsRef, + selection.memberAccess, + ); if (!resolvedMember.found) { throw new MacroAbortError( @@ -327,14 +312,12 @@ export class SingleMacroEngine { macroChoice: IMacroChoice, userScriptCommands: Array<{ command: IUserScript; index: number }>, memberAccess: string[], - preloadedScripts: Map, ): Promise { if (userScriptCommands.length === 1) { return { candidate: { command: userScriptCommands[0].command, index: userScriptCommands[0].index, - resolvedMember: { found: false }, }, memberAccess, }; @@ -347,48 +330,24 @@ export class SingleMacroEngine { ); if (selectorMatch) { - const exportsRef = await getUserScript(selectorMatch.command, this.app); - const cacheKey = selectorMatch.command.path ?? selectorMatch.command.id; - if (cacheKey && exportsRef !== undefined && exportsRef !== null) { - preloadedScripts.set(cacheKey, exportsRef); - } - - const resolvedMember = this.resolveMemberAccess( - exportsRef, - selectorMatch.memberAccess, - ); - if (!resolvedMember.found) { - throw new MacroAbortError( - `Macro '${macroChoice.name}' targeted script '${selectorMatch.command.name}', but that script does not export '${selectorMatch.memberAccess.join( - "::", - )}'.`, - ); - } - return { candidate: { command: selectorMatch.command, index: selectorMatch.index, - exportsRef, - resolvedMember, }, memberAccess: selectorMatch.memberAccess, }; } - const candidates: UserScriptCandidate[] = []; + const candidates: Array< + UserScriptCandidate & { resolvedMember: { found: boolean; value?: unknown } } + > = []; for (const entry of userScriptCommands) { const exportsRef = await getUserScript(entry.command, this.app); - const cacheKey = entry.command.path ?? entry.command.id; - if (cacheKey && exportsRef !== undefined && exportsRef !== null) { - preloadedScripts.set(cacheKey, exportsRef); - } - candidates.push({ command: entry.command, index: entry.index, - exportsRef, resolvedMember: this.resolveMemberAccess(exportsRef, memberAccess), }); } diff --git a/tests/e2e/macro-member-access.test.ts b/tests/e2e/macro-member-access.test.ts index 7a4bdfeb..5bb1bc65 100644 --- a/tests/e2e/macro-member-access.test.ts +++ b/tests/e2e/macro-member-access.test.ts @@ -15,7 +15,8 @@ import type { const VAULT = "dev"; const PLUGIN_ID = "quickadd"; -const WAIT_OPTS = { timeoutMs: 10_000, intervalMs: 200 }; +const waitTimeoutMs = Number(process.env.E2E_TIMEOUT_MS) || 15_000; +const WAIT_OPTS = { timeoutMs: waitTimeoutMs, intervalMs: 200 }; const TEST_PREFIX = "__qa-test-964-"; let obsidian: ObsidianClient;