fix: preserve macro user script variable updates

src/engine/MacroChoiceEngine.entry.test.ts

@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { MacroChoiceEngine } from "./MacroChoiceEngine";
 import type QuickAdd from "../main";
 import type { IChoiceExecutor } from "../IChoiceExecutor";
@@ -211,6 +211,122 @@ describe("MacroChoiceEngine user script entry handling", () => {
 	});
 });
 
+describe("MacroChoiceEngine user script variable propagation", () => {
+	const app = {} as App;
+	const plugin = {} as unknown as QuickAdd;
+
+	let choiceExecutor: IChoiceExecutor;
+	let variables: Map<string, unknown>;
+	let macroChoice: IMacroChoice;
+	let logs: Array<number | undefined>;
+	let getApiSpy: ReturnType<typeof vi.spyOn>;
+
+	beforeEach(() => {
+		vi.clearAllMocks();
+		mockGetUserScript.mockReset();
+		mockInitializeUserScriptSettings.mockReset();
+		mockSuggest.mockReset();
+
+		getApiSpy = vi
+			.spyOn(QuickAddApi, "GetApi")
+			.mockReturnValue({} as unknown as ReturnType<typeof QuickAddApi.GetApi>);
+
+		logs = [];
+		variables = new Map<string, unknown>();
+		choiceExecutor = {
+			execute: vi.fn(),
+			variables,
+		};
+
+		const makeScript = (index: number): IUserScript => ({
+			id: `script-${index}`,
+			name: `Script ${index}`,
+			type: CommandType.UserScript,
+			path: `script-${index}.js`,
+			settings: {},
+		});
+
+		macroChoice = {
+			id: "macro-sequence",
+			name: "Macro sequence",
+			type: "Macro",
+			command: false,
+			runOnStartup: false,
+			macro: {
+				id: "macro-sequence",
+				name: "Macro sequence",
+				commands: [
+					makeScript(1),
+					makeScript(2),
+					makeScript(3),
+					makeScript(4),
+				],
+			} as IMacro,
+		};
+
+		mockGetUserScript.mockImplementation((command: IUserScript) => {
+			const nextValueByPath: Record<string, number> = {
+				"script-1.js": 1,
+				"script-2.js": 2,
+				"script-3.js": 3,
+				"script-4.js": 3,
+			};
+
+			const nextValue = nextValueByPath[command.path ?? ""] ?? 0;
+
+			return Promise.resolve(async (params: { variables: Record<string, unknown> }) => {
+				logs.push(params.variables.target as number | undefined);
+				if (nextValue !== 0) {
+					params.variables.target = nextValue;
+				}
+			});
+		});
+	});
+
+	afterEach(() => {
+		getApiSpy.mockRestore();
+	});
+
+	it("propagates variable mutations across sequential user scripts", async () => {
+		const engine = new MacroChoiceEngine(
+			app,
+			plugin,
+			macroChoice,
+			choiceExecutor,
+			variables,
+		);
+
+		await engine.run();
+
+		expect(logs).toEqual([undefined, 1, 2, 3]);
+		expect(choiceExecutor.variables.get("target")).toBe(3);
+	});
+
+	it("merges existing executor variables into provided map without losing state", async () => {
+		const executorVariables = new Map<string, unknown>([
+			["keep", "executor"],
+		]);
+		const providedVariables = new Map<string, unknown>([
+			["override", 1],
+		]);
+
+		const engine = new MacroChoiceEngine(
+			app,
+			plugin,
+			macroChoice,
+			{
+				...choiceExecutor,
+				variables: executorVariables,
+			},
+			providedVariables,
+		);
+
+		expect(engine["params"].variables.keep).toBe("executor");
+		expect(engine["params"].variables.override).toBe(1);
+		expect(engine["choiceExecutor"].variables).toBe(providedVariables);
+	});
+});
+
 describe("MacroChoiceEngine choice command cancellation", () => {
 	const app = {} as App;
 	let plugin: QuickAdd & { getChoiceById: ReturnType<typeof vi.fn> };

src/engine/MacroChoiceEngine.ts

@@ -47,6 +47,7 @@ import type { ScriptCondition } from "../types/macros/Conditional/types";
 import { evaluateCondition } from "./helpers/conditionalEvaluator";
 import { handleMacroAbort } from "../utils/macroAbortHandler";
 import { buildOpenFileOptions } from "./helpers/openFileOptions";
+import { createVariablesProxy } from "../utils/variablesProxy";
 
 type ConditionalScriptRunner = () => Promise<unknown>;
 
@@ -93,19 +94,32 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
 		this.macro = choice?.macro;
 		this.choiceExecutor = choiceExecutor;
 		this.preloadedUserScripts = preloadedUserScripts ?? new Map();
+		const existingVariables = this.choiceExecutor.variables;
+		const sharedVariables =
+			variables ?? existingVariables ?? new Map<string, unknown>();
+
+		// If a fresh map was provided, merge any existing executor variables
+		// so we don't accidentally drop state the caller wanted to keep.
+		if (
+			variables &&
+			existingVariables &&
+			variables !== existingVariables
+		) {
+			existingVariables.forEach((value, key) => {
+				if (!variables.has(key)) variables.set(key, value);
+			});
+		}
+
+		this.choiceExecutor.variables = sharedVariables;
 		this.params = {
 			app: this.app,
 			quickAddApi: QuickAddApi.GetApi(app, plugin, choiceExecutor),
-			variables: {},
+			variables: createVariablesProxy(this.choiceExecutor.variables),
 			obsidian,
 			abort: (message?: string) => {
 				throw new MacroAbortError(message);
 			},
 		};
-
-		variables?.forEach((value, key) => {
-			this.params.variables[key] = value;
-		});
 	}
 
 	async run(): Promise<void> {
@@ -151,14 +165,6 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
 				if (command?.type === CommandType.Conditional) {
 					await this.executeConditional(command as IConditionalCommand);
 				}
-
-				this.pullExecutorVariablesIntoParams();
-				Object.keys(this.params.variables).forEach((key) => {
-					this.choiceExecutor.variables.set(
-						key,
-						this.params.variables[key]
-					);
-				});
 			}
 		} catch (error) {
 			if (
@@ -462,7 +468,6 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
 	}
 
 	private async executeConditional(command: IConditionalCommand) {
-		this.pullExecutorVariablesIntoParams();
 		const shouldRunThenBranch = await evaluateCondition(command.condition, {
 			variables: this.params.variables,
 			evaluateScriptCondition: async (condition: ScriptCondition) =>
@@ -489,12 +494,6 @@ export class MacroChoiceEngine extends QuickAddChoiceEngine {
 		this.output = value;
 	}
 
-	private pullExecutorVariablesIntoParams() {
-		this.choiceExecutor.variables.forEach((value, key) => {
-			this.params.variables[key] = value;
-		});
-	}
-
 	private async evaluateScriptCondition(
 		condition: ScriptCondition
 	): Promise<boolean> {

src/engine/SingleMacroEngine.ts

@@ -288,11 +288,6 @@ export class SingleMacroEngine {
 		engine: MacroChoiceEngine,
 		settings: Record<string, unknown>,
 	): Promise<unknown> {
-		// Ensure params reflect latest shared variables before executing
-		this.choiceExecutor.variables.forEach((value, key) => {
-			engine.params.variables[key] = value;
-		});
-
 		if (typeof member === "function") {
 			return await (
 				member as (

src/utils/variablesProxy.test.ts

@@ -0,0 +1,68 @@
+import { describe, expect, it } from "vitest";
+import { createVariablesProxy } from "./variablesProxy";
+
+describe("createVariablesProxy", () => {
+	it("reads and writes through to the backing map", () => {
+		const backing = new Map<string, unknown>([["a", 1]]);
+		const proxy = createVariablesProxy(backing);
+
+		expect(proxy.a).toBe(1);
+		proxy.b = 2;
+		expect(backing.get("b")).toBe(2);
+
+		delete proxy.a;
+		expect(backing.has("a")).toBe(false);
+	});
+
+	it("enumerates keys like a plain object", () => {
+		const backing = new Map<string, unknown>([
+			["foo", "bar"],
+			["num", 3],
+		]);
+		const proxy = createVariablesProxy(backing);
+
+		expect(Object.keys(proxy)).toEqual(["foo", "num"]);
+		expect(JSON.stringify(proxy)).toBe('{"foo":"bar","num":3}');
+	});
+
+	it("reflects external map mutations immediately", () => {
+		const backing = new Map<string, unknown>();
+		const proxy = createVariablesProxy(backing);
+
+		backing.set("outside", 42);
+		expect(proxy.outside).toBe(42);
+
+		backing.delete("outside");
+		expect(proxy.outside).toBeUndefined();
+		expect(Object.keys(proxy)).toEqual([]);
+	});
+
+	it("ignores symbol property access and does not throw", () => {
+		const backing = new Map<string, unknown>();
+		const proxy = createVariablesProxy(backing);
+		const sym = Symbol("test");
+
+		// @ts-expect-error accessing symbol
+		expect(proxy[sym]).toBeUndefined();
+		// @ts-expect-error setting symbol
+		proxy[sym] = "value";
+		expect(backing.size).toBe(0);
+	});
+
+	it("works with for...in and Object.assign", () => {
+		const backing = new Map<string, unknown>([
+			["x", 1],
+			["y", 2],
+		]);
+		const proxy = createVariablesProxy(backing);
+
+		const seen: string[] = [];
+		for (const key in proxy) {
+			seen.push(key);
+		}
+		expect(seen).toEqual(["x", "y"]);
+
+		const copy = Object.assign({}, proxy);
+		expect(copy).toEqual({ x: 1, y: 2 });
+	});
+});

src/utils/variablesProxy.ts

@@ -0,0 +1,41 @@
+/**
+ * Creates a plain-object style proxy over a Map so scripts can read/write
+ * `params.variables.foo` while the underlying source of truth remains the Map.
+ */
+export function createVariablesProxy(
+	store: Map<string, unknown>
+): Record<string, unknown> {
+	const target = Object.create(null);
+
+	return new Proxy(target, {
+		get(_t, prop: string | symbol) {
+			if (typeof prop !== "string") return undefined;
+			return store.get(prop);
+		},
+		set(_t, prop: string | symbol, value) {
+			if (typeof prop !== "string") return true;
+			store.set(prop, value);
+			return true;
+		},
+		deleteProperty(_t, prop: string | symbol) {
+			if (typeof prop !== "string") return true;
+			store.delete(prop);
+			return true;
+		},
+		has(_t, prop: string | symbol) {
+			return typeof prop === "string" && store.has(prop);
+		},
+		ownKeys() {
+			return Array.from(store.keys());
+		},
+	getOwnPropertyDescriptor(_t, prop: string | symbol) {
+		if (typeof prop !== "string" || !store.has(prop)) return undefined;
+		return {
+			value: store.get(prop),
+			writable: true,
+			configurable: true,
+			enumerable: true,
+		};
+	},
+});
+}