From 9a8656ca0fb57163da748dbef7b9f403496c7439 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Wed, 17 Jun 2026 10:53:26 +0200 Subject: [PATCH 1/2] fix(ai): keep OpenAI assistant content as a string OpenAI rejects assistant messages whose content is null, including tool-call messages. Preserve an empty string when the normalized message has no content so the request body stays valid. --- src/ai/tools/providerToolMapping.test.ts | 1 + src/ai/tools/providerToolMapping.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ai/tools/providerToolMapping.test.ts b/src/ai/tools/providerToolMapping.test.ts index 569546c3..1c41bdd6 100644 --- a/src/ai/tools/providerToolMapping.test.ts +++ b/src/ai/tools/providerToolMapping.test.ts @@ -57,6 +57,7 @@ describe("OpenAI mapping", () => { const body = buildChatBody("openai", "gpt-4o", req) as Record; const msgs = body.messages as Array>; const assistant = msgs[1]; + expect(assistant.content).toBe(""); expect((assistant.tool_calls as Array>)[0]).toMatchObject({ id: "call_1", type: "function", diff --git a/src/ai/tools/providerToolMapping.ts b/src/ai/tools/providerToolMapping.ts index 2eea1d35..ed9c7537 100644 --- a/src/ai/tools/providerToolMapping.ts +++ b/src/ai/tools/providerToolMapping.ts @@ -79,7 +79,7 @@ function openaiMessages(messages: NormalizedMessage[]): Body[] { if (m.role === "system") out.push({ role: "system", content: m.content }); else if (m.role === "user") out.push({ role: "user", content: m.content }); else if (m.role === "assistant") { - const msg: Body = { role: "assistant", content: m.content || null }; + const msg: Body = { role: "assistant", content: m.content ?? "" }; if (m.toolCalls && m.toolCalls.length > 0) { msg.tool_calls = m.toolCalls.map((c) => ({ id: c.id, From 0b26d7655e6505c70b79d0934e47ed45f06b8eb7 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Wed, 17 Jun 2026 10:53:38 +0200 Subject: [PATCH 2/2] feat(user-scripts): support stable secret option ids Allow user-script secret options to provide an optional id for SecretStorage references. This keeps saved secrets stable across visible setting label changes while retaining generated IDs as the fallback. --- docs/docs/Advanced/scriptsWithSettings.md | 3 +- docs/docs/UserScripts.md | 3 + .../MacroGUIs/UserScriptSettingsModal.test.ts | 34 ++++- src/gui/MacroGUIs/UserScriptSettingsModal.ts | 3 +- src/utils/userScriptSecrets.test.ts | 123 +++++++++++++++++ src/utils/userScriptSecrets.ts | 130 ++++++++++++++++-- 6 files changed, 280 insertions(+), 16 deletions(-) diff --git a/docs/docs/Advanced/scriptsWithSettings.md b/docs/docs/Advanced/scriptsWithSettings.md index c72c1f78..5c1a7c4e 100644 --- a/docs/docs/Advanced/scriptsWithSettings.md +++ b/docs/docs/Advanced/scriptsWithSettings.md @@ -48,6 +48,7 @@ module.exports = { }, "API Key": { type: "secret", + id: "api-key", placeholder: "Paste API key", description: "Stored securely with Obsidian SecretStorage.", }, @@ -76,7 +77,7 @@ module.exports = { ## Setting types - `text` and `input`: A text field. -- `secret`: A password-style input stored with Obsidian SecretStorage. QuickAdd stores only a reference in `data.json`; package exports omit secret values and local secret references. The older `text` / `input` plus `secret: true` form is still treated as a secret setting. +- `secret`: A password-style input stored with Obsidian SecretStorage. QuickAdd stores only a reference in `data.json`; package exports omit secret values and local secret references. Add an optional `id` to give the stored secret a stable key if the visible setting label changes later. The older `text` / `input` plus `secret: true` form is still treated as a secret setting. - `textarea`: A multi-line text area. - `checkbox` and `toggle`: A checkbox. - `dropdown` and `select`: A dropdown. diff --git a/docs/docs/UserScripts.md b/docs/docs/UserScripts.md index afbfcf32..f9503880 100644 --- a/docs/docs/UserScripts.md +++ b/docs/docs/UserScripts.md @@ -233,6 +233,7 @@ For API keys, access tokens, and other sensitive values. Secrets are stored in O options: { "API Key": { type: "secret", + id: "api-key", // Optional stable storage id placeholder: "Paste API key", description: "Your API key" } @@ -243,6 +244,8 @@ options: { Secret values are local to the Obsidian app profile. They are not included in QuickAdd package exports and are not synced through the plugin's `data.json`; users must enter them on each device where the script runs. +The optional `id` controls the SecretStorage reference used for the setting. If omitted, QuickAdd uses the option name. Set `id` when you want to rename the visible setting label later without creating a new saved secret. + #### Toggle/Checkbox For boolean on/off settings. diff --git a/src/gui/MacroGUIs/UserScriptSettingsModal.test.ts b/src/gui/MacroGUIs/UserScriptSettingsModal.test.ts index 2a0e092a..85f1618a 100644 --- a/src/gui/MacroGUIs/UserScriptSettingsModal.test.ts +++ b/src/gui/MacroGUIs/UserScriptSettingsModal.test.ts @@ -31,7 +31,7 @@ function createCommand(settings: Record = {}): IUserScript { }; } -function createSettings() { +function createSettings(optionOverrides: Record = {}) { return { name: "Script Settings", options: { @@ -40,6 +40,7 @@ function createSettings() { defaultValue: "must-not-persist", placeholder: "Paste API key", description: "API key", + ...optionOverrides, }, }, }; @@ -94,6 +95,37 @@ describe("UserScriptSettingsModal secret settings", () => { expect(input.value).toBe(""); }); + it("uses script-defined secret option IDs when saving", async () => { + const app = new App(); + const command = createCommand(); + const modal = new UserScriptSettingsModal( + app, + command, + createSettings({ id: "readwise-api-key" }), + ); + await flushPromises(); + + const input = modal.contentEl.querySelector("input") as HTMLInputElement; + inputValue(input, "secret-value"); + + const save = modal.contentEl.querySelector( + 'button[aria-label="Save API Key"]', + ) as HTMLButtonElement; + save.click(); + await flushPromises(); + + expect( + app.secretStorage.getSecret( + "quickadd-user-script-command-1-readwise-api-key", + ), + ).toBe("secret-value"); + expect(command.settings["API Key"]).toEqual( + createUserScriptSecretRef( + "quickadd-user-script-command-1-readwise-api-key", + ), + ); + }); + it("migrates existing plaintext secret settings after opening", async () => { const app = new App(); const command = createCommand({ diff --git a/src/gui/MacroGUIs/UserScriptSettingsModal.ts b/src/gui/MacroGUIs/UserScriptSettingsModal.ts index b48ac1f6..03f0b2ac 100644 --- a/src/gui/MacroGUIs/UserScriptSettingsModal.ts +++ b/src/gui/MacroGUIs/UserScriptSettingsModal.ts @@ -15,7 +15,7 @@ import { storeUserScriptSecret, } from "../../utils/userScriptSecrets"; -type Option = { description?: string } & ( +type Option = { description?: string; id?: string } & ( | { type: "text" | "input"; value: string; @@ -224,6 +224,7 @@ export class UserScriptSettingsModal extends Modal { name, pendingValue, getSecretRefFromCommandSetting(this.command, name), + this.settings.options?.[name], ); if (!secretRef) { diff --git a/src/utils/userScriptSecrets.test.ts b/src/utils/userScriptSecrets.test.ts index 5f0734f4..48ce6c8c 100644 --- a/src/utils/userScriptSecrets.test.ts +++ b/src/utils/userScriptSecrets.test.ts @@ -179,6 +179,59 @@ describe("userScriptSecrets", () => { ); }); + it("uses script-defined secret option IDs when provided", () => { + const command = createCommand(); + const option = { type: "secret", id: "readwise-api-key" }; + + expect( + buildUserScriptSecretId(command, "Readwise API key", option), + ).toBe("quickadd-user-script-command-1-readwise-api-key"); + expect(buildUserScriptSecretId(command, "API token", option)).toBe( + "quickadd-user-script-command-1-readwise-api-key", + ); + }); + + it("keeps SecretStorage IDs within Obsidian's 64 character limit", () => { + const command = createCommand(); + command.id = "4e700b0f-1e0c-47c2-aafc-417421efcc73"; + + const secretId = buildUserScriptSecretId(command, "Readwise API key"); + + expect(secretId.length).toBeLessThanOrEqual(64); + expect(secretId).toMatch(/^[a-z0-9-]+$/); + expect(secretId).toMatch(/^quickadd-user-script-/); + expect(secretId).toBe(buildUserScriptSecretId(command, "Readwise API key")); + }); + + it("keeps collision fallback SecretStorage IDs within Obsidian's 64 character limit", async () => { + const command = createCommand(); + command.id = "4e700b0f-1e0c-47c2-aafc-417421efcc73"; + const existingSecretRef = buildUserScriptSecretId( + command, + "Readwise API key", + ); + const storedSecrets = new Map([[existingSecretRef, "old-secret"]]); + const setSecret = vi.fn((name: string, value: string) => { + storedSecrets.set(name, value); + }); + const app = createApp({ + getSecret: vi.fn((name: string) => storedSecrets.get(name) ?? null), + setSecret, + }); + + const secretRef = await storeUserScriptSecret( + app, + command, + "Readwise API key", + "new-secret", + ); + + expect(secretRef).not.toBe(existingSecretRef); + expect(secretRef?.length).toBeLessThanOrEqual(64); + expect(secretRef).toMatch(/^[a-z0-9-]+$/); + expect(setSecret).toHaveBeenCalledWith(secretRef, "new-secret"); + }); + it("stores a secret and creates a marker without exposing the value", async () => { const setSecret = vi.fn().mockResolvedValue(undefined); const getSecret = vi.fn().mockResolvedValue(null); @@ -321,6 +374,76 @@ describe("userScriptSecrets", () => { expect(JSON.stringify(command.settings)).not.toContain("legacy-secret"); }); + it("uses script-defined secret option IDs when migrating legacy plaintext settings", async () => { + const command = createCommand({ + "Readwise API key": "legacy-secret", + }); + const setSecret = vi.fn().mockResolvedValue(undefined); + const app = createApp({ + getSecret: vi.fn().mockResolvedValue(null), + setSecret, + }); + + const changed = await migrateUserScriptSecretSettings(app, command, { + options: { + "Readwise API key": { + type: "secret", + id: "readwise-api-key", + }, + }, + }); + + expect(changed).toBe(true); + expect(setSecret).toHaveBeenCalledWith( + "quickadd-user-script-command-1-readwise-api-key", + "legacy-secret", + ); + expect(command.settings["Readwise API key"]).toEqual( + createUserScriptSecretRef( + "quickadd-user-script-command-1-readwise-api-key", + ), + ); + }); + + it("reattaches script-defined secret IDs when option labels change", async () => { + const stableRef = "quickadd-user-script-command-1-readwise-api-key"; + const command = createCommand({ + "Readwise API key": createUserScriptSecretRef(stableRef), + }); + const app = createApp({ + getSecret: vi.fn((name: string) => + name === stableRef ? "stored-secret" : null, + ), + setSecret: vi.fn(), + }); + const renamedSettings = { + options: { + "API token": { + type: "secret", + id: "readwise-api-key", + }, + }, + }; + + const changed = await migrateUserScriptSecretSettings( + app, + command, + renamedSettings, + ); + const resolved = await resolveUserScriptSettings( + app, + command, + renamedSettings, + ); + + expect(changed).toBe(true); + expect(command.settings["Readwise API key"]).toBeUndefined(); + expect(command.settings["API token"]).toEqual( + createUserScriptSecretRef(stableRef), + ); + expect(resolved["API token"]).toBe("stored-secret"); + }); + it("does not migrate or clear legacy plaintext when SecretStorage is unavailable", async () => { const command = createCommand({ "API Key": "legacy-secret", diff --git a/src/utils/userScriptSecrets.ts b/src/utils/userScriptSecrets.ts index e3f128c9..639bac19 100644 --- a/src/utils/userScriptSecrets.ts +++ b/src/utils/userScriptSecrets.ts @@ -6,6 +6,8 @@ import { extractScriptFromMarkdown } from "./extractScriptFromMarkdown"; const USER_SCRIPT_SECRET_PREFIX = "quickadd-user-script"; const SECRET_MARKER = "__quickaddSecret"; const MARKDOWN_FILE_EXTENSION_REGEX = /\.md$/i; +const MAX_SECRET_ID_LENGTH = 64; +const SECRET_ID_HASH_LENGTH = 8; type SecretStorageLike = { getSecret?: (id: string) => string | null | Promise; @@ -17,6 +19,7 @@ type SecretStorageLike = { }; export type UserScriptOptionDefinition = { + id?: unknown; type?: unknown; secret?: unknown; defaultValue?: unknown; @@ -63,6 +66,65 @@ function normalizeSecretIdPart(value: string): string { return normalized || "setting"; } +function hashSecretId(value: string): string { + let hash = 5381; + for (let index = 0; index < value.length; index += 1) { + hash = (hash * 33) ^ value.charCodeAt(index); + } + + return (hash >>> 0).toString(36).slice(0, SECRET_ID_HASH_LENGTH); +} + +function truncateSecretIdPart(value: string, maxLength: number): string { + if (value.length <= maxLength) return value; + return value.slice(0, maxLength).replace(/-+$/g, "") || "setting"; +} + +function getSecretSettingId( + settingName: string, + option?: UserScriptOptionDefinition, +): string { + const optionId = option?.id; + if (typeof optionId === "string" && optionId.trim().length > 0) { + return optionId; + } + + return settingName; +} + +function fitSecretId( + commandPart: string, + settingPart: string, + suffixPart?: string, +): string { + const rawParts = suffixPart + ? [USER_SCRIPT_SECRET_PREFIX, commandPart, settingPart, suffixPart] + : [USER_SCRIPT_SECRET_PREFIX, commandPart, settingPart]; + const raw = rawParts.join("-"); + if (raw.length <= MAX_SECRET_ID_LENGTH) return raw; + + const hash = hashSecretId( + [commandPart, settingPart, suffixPart].filter(Boolean).join(":"), + ); + const separatorBudget = suffixPart ? 4 : 3; + const partBudget = + MAX_SECRET_ID_LENGTH - + USER_SCRIPT_SECRET_PREFIX.length - + hash.length - + (suffixPart?.length ?? 0) - + separatorBudget; + const commandBudget = Math.max(8, Math.floor(partBudget * 0.6)); + const settingBudget = Math.max(8, partBudget - commandBudget); + + return [ + USER_SCRIPT_SECRET_PREFIX, + truncateSecretIdPart(commandPart, commandBudget), + truncateSecretIdPart(settingPart, settingBudget), + ...(suffixPart ? [suffixPart] : []), + hash, + ].join("-"); +} + function formatSecretError(error: unknown): string { return (error as Error)?.message ?? String(error); } @@ -466,13 +528,13 @@ export function createUserScriptSecretRef(secretRef: string): UserScriptSecretRe export function buildUserScriptSecretId( command: IUserScript, settingName: string, + option?: UserScriptOptionDefinition, ): string { const commandId = command.id?.trim() || command.path || command.name || "script"; - return [ - USER_SCRIPT_SECRET_PREFIX, + return fitSecretId( normalizeSecretIdPart(commandId), - normalizeSecretIdPart(settingName), - ].join("-"); + normalizeSecretIdPart(getSecretSettingId(settingName, option)), + ); } export function getSecretOptionNames( @@ -486,6 +548,21 @@ export function getSecretOptionNames( .map(([name]) => name); } +function getSecretOptionEntries( + userScriptSettings: UserScriptSettingsDefinition | undefined, +): Array<[string, UserScriptOptionDefinition]> { + const options = userScriptSettings?.options; + if (!options) return []; + + return Object.entries(options).filter(([, option]) => + isSecretUserScriptOption(option), + ); +} + +function optionDefinesStableSecretId(option: UserScriptOptionDefinition): boolean { + return typeof option.id === "string" && option.id.trim().length > 0; +} + async function readSecretStorageEntry( app: App | undefined, secretRef: string, @@ -527,9 +604,12 @@ async function buildAvailableSecretRef( command: IUserScript, settingName: string, value: string, + option?: UserScriptOptionDefinition, ): Promise { - const base = buildUserScriptSecretId(command, settingName); - let candidate = base; + const commandId = command.id?.trim() || command.path || command.name || "script"; + const commandPart = normalizeSecretIdPart(commandId); + const settingPart = normalizeSecretIdPart(getSecretSettingId(settingName, option)); + let candidate = fitSecretId(commandPart, settingPart); let suffix = 1; while (true) { @@ -537,7 +617,7 @@ async function buildAvailableSecretRef( if (!existing || existing === value) return candidate; suffix += 1; - candidate = `${base}-${suffix}`; + candidate = fitSecretId(commandPart, settingPart, String(suffix)); } } @@ -547,12 +627,13 @@ export async function storeUserScriptSecret( settingName: string, value: string, existingRef?: string, + option?: UserScriptOptionDefinition, ): Promise { if (value.length === 0) return null; const secretRef = existingRef?.trim() || - (await buildAvailableSecretRef(app, command, settingName, value)); + (await buildAvailableSecretRef(app, command, settingName, value, option)); const stored = await writeSecretStorageEntry(app, secretRef, value); return stored ? secretRef : null; @@ -627,12 +708,12 @@ export async function migrateUserScriptSecretSettings( command: IUserScript, userScriptSettings: UserScriptSettingsDefinition | undefined, ): Promise { - const secretOptionNames = getSecretOptionNames(userScriptSettings); - if (secretOptionNames.length === 0) return false; + const secretOptionEntries = getSecretOptionEntries(userScriptSettings); + if (secretOptionEntries.length === 0) return false; const secretStorage = getSecretStorage(app); if (!secretStorage?.getSecret || !secretStorage?.setSecret) { - const hasLegacySecrets = secretOptionNames.some((name) => { + const hasLegacySecrets = secretOptionEntries.some(([name]) => { const value = command.settings?.[name]; return typeof value === "string" && value.length > 0; }); @@ -648,16 +729,39 @@ export async function migrateUserScriptSecretSettings( let migrated = false; - for (const settingName of secretOptionNames) { + for (const [settingName, option] of secretOptionEntries) { const value = command.settings?.[settingName]; if (isUserScriptSecretRef(value)) continue; - if (typeof value !== "string" || value.length === 0) continue; + if (typeof value !== "string" || value.length === 0) { + if (!optionDefinesStableSecretId(option)) continue; + + const secretRef = buildUserScriptSecretId(command, settingName, option); + const existingSecret = await readSecretStorageEntry(app, secretRef); + if (!existingSecret) continue; + + command.settings[settingName] = createUserScriptSecretRef(secretRef); + for (const [existingName, existingValue] of Object.entries( + command.settings, + )) { + if ( + existingName !== settingName && + isUserScriptSecretRef(existingValue) && + existingValue.secretRef === secretRef + ) { + delete command.settings[existingName]; + } + } + migrated = true; + continue; + } const secretRef = await storeUserScriptSecret( app, command, settingName, value, + undefined, + option, ); if (!secretRef) continue;