diff --git a/docs/docs/commands.md b/docs/docs/commands.md index 599080b9..06af7c12 100644 --- a/docs/docs/commands.md +++ b/docs/docs/commands.md @@ -1,7 +1,7 @@ ## Show player Opens the PodNotes pane and brings it into focus. -If the pane already exists but is hidden — for example in a collapsed sidebar or out of view because the right sidebar has too many icons — this command reveals it. If it does not exist yet, the command creates it in the right sidebar. You can run it from the command palette or bind it to a hotkey. +If the pane already exists but is hidden - for example in a collapsed sidebar or out of view because the right sidebar has too many icons - this command reveals it. If it does not exist yet, the command creates it in the right sidebar. You can run it from the command palette or bind it to a hotkey. PodNotes also adds a **podcast icon to the left ribbon** as a reliable way to reopen the pane. On mobile it appears in the ribbon menu, and you can hide it via Obsidian's *Manage ribbon actions* if you prefer to use the command instead. @@ -25,7 +25,7 @@ This will skip the current episode forward by the amount of seconds specified in ## Download Playing Episode This will download the currently playing episode. -Downloads are stored in the location specified by the **Episode download path** setting. This path is a template and **must include a per-episode token** such as `{{title}}` (the default is `PodNotes/{{podcast}}/{{title}}`). A path without `{{title}}` makes every episode resolve to the same file, so downloads overwrite each other or fail. The file extension is added automatically — do not include one. +Downloads are stored in the location specified by the **Episode download path** setting. This path is a template and **must include a per-episode token** such as `{{title}}` (the default is `PodNotes/{{podcast}}/{{title}}`). A path without `{{title}}` makes every episode resolve to the same file, so downloads overwrite each other or fail. The file extension is added automatically - do not include one. ## Capture Timestamp This will capture the current timestamp of the currently playing episode. @@ -85,8 +85,16 @@ Episode links look like this: [https://pod.link/1138055739/episode/1732808e781cc They can be used to share the episode with others, no matter what podcast app they use. ## Transcribe current episode -This command will transcribe the currently playing episode using OpenAI's Whisper model. -The transcription will be saved in the location specified in the transcript settings. +This command transcribes the currently playing audio episode and saves the +result at the path configured in **Transcript settings**. -Note: This feature requires an OpenAI API key to be set in the settings. +Plain Whisper transcription and OpenAI speaker diarization require an available +OpenAI secret on the current device. Deepgram speaker diarization requires an +available Deepgram secret instead. In **Settings -> PodNotes -> Transcript +settings**, select an existing Obsidian secret or create one for the provider +you use. + +The command remains available when a required secret is missing so PodNotes can +tell you which provider needs to be configured. If a selected secret is not +available on this device, open PodNotes settings and select or create it again. diff --git a/docs/docs/import_export.md b/docs/docs/import_export.md index cd9a890d..4e076ce9 100644 --- a/docs/docs/import_export.md +++ b/docs/docs/import_export.md @@ -19,33 +19,79 @@ First designate a file path to save to (or use the default), and click _Export_. ## Settings & templates -Under the **Settings & templates** heading you can move your PodNotes +Under the **Settings & templates** heading, you can move your PodNotes configuration between vaults or back it up. This covers your preferences, note/timestamp/transcript templates, file paths, saved feeds, and playlists. ![Settings & templates import/export](resources/settings_import_export.png) -Playback progress, downloaded-episode bookkeeping, the currently-playing +Playback progress, downloaded-episode bookkeeping, the currently playing episode, and the episode-to-note mapping are **not** included, because they are specific to a single vault. +PodNotes settings exports use format v2. API key values and the names of the +Obsidian secrets selected in PodNotes are omitted by default. This keeps a +destination vault's existing OpenAI and Deepgram selections unchanged when you +import the file. + ### Exporting settings -1. (Optional) Enable **Include OpenAI API key** if you also want your key in the - file. The key is written in plaintext, so only do this for files you keep - private — the file lives in your vault and may sync to other devices or be - read by other plugins. +1. Leave **Include API keys** off for a normal settings export. To transfer the + OpenAI and Deepgram keys that are available on this device, enable it + explicitly. 2. Set a file name (or keep the default `PodNotes_Settings.json`). 3. Click **Export**. The settings file is written to your vault. +When **Include API keys** is enabled, available values are added in plaintext +under a separate top-level `secrets` payload: + +```json +{ + "type": "podnotes-settings", + "version": 2, + "settings": {}, + "secrets": { + "openAI": "your OpenAI API key", + "deepgram": "your Deepgram API key" + } +} +``` + +Only configured keys are included. If a selected secret name exists in PodNotes +settings but its value is unavailable on this device, the export stops instead +of silently creating an incomplete credential backup. Open **Transcript +settings**, select or create the missing secret on this device, and export +again. + +An export containing `secrets` is sensitive. The values are plaintext in a file +inside your vault, where they may sync to other devices or be read by other +plugins. Keep the file private and delete it when it is no longer needed. + ### Importing settings -1. Click **Import** next to _Import settings_ and choose a settings file. Both a - PodNotes export file and a raw `data.json` are accepted. +1. Click **Import** next to **Import settings** and choose a settings file. + PodNotes accepts current v2 exports, legacy v1 exports, and a raw PodNotes + `data.json`. 2. Confirm the import. Your current preferences, templates, feeds, and playlists are replaced with the imported values; playback progress and downloads are kept. +An import without API key values preserves the OpenAI and Deepgram secret names +already selected in the destination vault. For a legacy v1 export or raw +`data.json`, PodNotes recognizes the old plaintext `openAIApiKey` and +`diarizationApiKey` fields and imports them as secrets instead of putting them +back into plugin settings. + +When an import contains API keys, the confirmation names the affected +providers. PodNotes stores each value under a PodNotes-owned name in Obsidian's +vault-local secret storage, then updates PodNotes to reference it. Existing +Obsidian secrets are never overwritten. If a PodNotes-owned name already holds +a different value, the imported key is stored under a new suffixed name. + +Imported secret values are available only in the current vault on the current +device. Repeat the import or select/create the appropriate secrets separately +on another device. + A file exported by a newer version of PodNotes is rejected until you update the -plugin. If an episode is already open when you import, a changed default playback -rate applies to the next episode you open. \ No newline at end of file +plugin. If an episode is already open when you import, a changed default +playback rate applies to the next episode you open. diff --git a/docs/docs/resources/diarization_settings.png b/docs/docs/resources/diarization_settings.png index 8228fb42..59baab79 100644 Binary files a/docs/docs/resources/diarization_settings.png and b/docs/docs/resources/diarization_settings.png differ diff --git a/docs/docs/resources/settings_import_export.png b/docs/docs/resources/settings_import_export.png index 40436014..5c5f7650 100644 Binary files a/docs/docs/resources/settings_import_export.png and b/docs/docs/resources/settings_import_export.png differ diff --git a/docs/docs/transcripts.md b/docs/docs/transcripts.md index 917981bd..96e3903e 100644 --- a/docs/docs/transcripts.md +++ b/docs/docs/transcripts.md @@ -1,25 +1,74 @@ # Transcripts -PodNotes allows you to create transcripts of podcast episodes using OpenAI's Whisper model. +PodNotes can create transcript notes from podcast episodes. Plain transcription +uses OpenAI's Whisper model, while optional speaker diarization can use OpenAI +or Deepgram. ## Setting Up -Before you can use the transcription feature, you need to set up a few things: - -1. **OpenAI API Key**: You need to have an OpenAI API key. You can get one by signing up at [OpenAI's website](https://openai.com/). Once you have the key, enter it in the PodNotes settings under the "Transcript settings" section. - -2. **Transcript File Path**: In the settings, you can specify where you want the transcript files to be saved. You can use placeholders like `{{podcast}}` and `{{title}}` in the path. - -3. **Transcript Template**: You can also customize how the transcript content is formatted using a template. +Before you can use transcription, set up the following: + +1. **OpenAI API key**: Create a key at [OpenAI's website](https://openai.com/). + In **Settings -> PodNotes -> Transcript settings**, use **OpenAI API key** to + select an existing Obsidian secret or create one. Plain Whisper transcription + and OpenAI diarization both use this secret. + +2. **Transcript file path**: Choose where transcript files are saved. You can + use placeholders such as `{{podcast}}` and `{{title}}` in the path. + +3. **Transcript template**: Customize how the transcript note is formatted. + +### How API keys are stored + +PodNotes uses Obsidian's native secret picker. The API key value is kept in +Obsidian's vault-local secret storage; PodNotes stores only a provider-specific, +PodNotes-owned secret name in its `data.json`, through the `openAISecretId` and +`deepgramSecretId` references. If you explicitly select a shared Obsidian +secret, PodNotes copies its value into a collision-safe name owned by the chosen +provider. Persisted OpenAI references therefore cannot read Deepgram or foreign +secrets, and persisted Deepgram references cannot read OpenAI or foreign +secrets. + +PodNotes requires Obsidian 1.11.5 or newer because that release encrypts secret +storage on disk using encryption provided by the operating system. Secret +storage keeps keys out of PodNotes files and normal settings exports, but it is +shared infrastructure rather than plugin isolation: other installed plugins can +access the same vault-local secret collection. + +Secret values are local to the current vault on the current device. A selected +secret name can sync with the rest of the vault configuration while its value +does not. If PodNotes says that a selected secret is unavailable on this device, +open **Transcript settings** on that device and select or create the secret +again. + +When upgrading from a version that stored API keys directly in `data.json`, +PodNotes moves existing OpenAI and Deepgram keys into Obsidian's secret storage +before rewriting the settings file. The plaintext fields are then removed. If +that migration cannot be completed and verified, PodNotes leaves the existing +settings file unchanged and shows a persistent notice so you can restart the +plugin and retry. + +The migration cannot erase copies that already exist in backups, Obsidian Sync +version history, filesystem snapshots, or old plaintext settings exports. If a +key may have reached an untrusted device or backup, rotate it with the provider +and remove any old export files you no longer need. ## Creating a Transcript To create a transcript: -1. Start playing the podcast episode you want to transcribe. -2. Use the "Transcribe current episode" command in Obsidian. -3. PodNotes will fetch the audio for the episode you are playing (reusing an already-downloaded copy when one exists), split it into chunks, and send these chunks to OpenAI for transcription. The transcription always uses the currently playing episode's own audio, regardless of your download path settings. -4. Once the transcription is complete, a new file will be created at the specified location with the transcribed content. +1. Start playing the audio episode you want to transcribe. +2. Run **PodNotes: Transcribe current episode** from the command palette. +3. PodNotes fetches the currently playing episode's audio, reusing an existing + download when available. Plain Whisper and OpenAI diarization split large + audio into chunks; Deepgram diarization sends the episode in one request. +4. When transcription finishes, PodNotes creates a note at the configured + transcript path. + +The command always uses the currently playing episode's own audio, regardless +of your episode download path setting. It remains available when a required +secret is missing so it can tell you which provider must be configured on the +current device. Generated transcript notes are also available to workflow plugins through the [PodNotes API](./api.md#transcript), so tools such as QuickAdd or Templater can @@ -27,11 +76,15 @@ read the text and send it to the AI provider configured in your own macro. ## Transcript Template -The transcript template works similarly to the [note template](./templates.md#note-template), but with the added `{{transcript}}` placeholder. +The transcript template works similarly to the +[note template](./templates.md#note-template), with the additional +`{{transcript}}` placeholder. ## Speaker Diarization -By default the transcription uses OpenAI's Whisper model, which produces plain text with **no speaker labels**. Speaker diarization is an opt-in setting that instead labels each segment of the transcript by speaker, turning a wall of text into a readable, speaker-by-speaker conversation: +By default, transcription uses OpenAI's Whisper model, which produces plain +text with **no speaker labels**. Speaker diarization is an opt-in setting that +labels each transcript segment by speaker: ![Transcript with speaker labels](resources/transcript_diarization.png) @@ -41,8 +94,16 @@ In the **Transcript settings** section, turn on **Speaker diarization** and choo ![Speaker diarization settings](resources/diarization_settings.png) -- **OpenAI** (`gpt-4o-transcribe-diarize`): reuses the OpenAI API key you already entered above, so there is nothing else to configure. Because each request is capped at ~20 MB (a conservative margin under OpenAI's 25 MB request cap), a long episode is split into chunks that are diarized independently — so on long episodes the speaker labels can change across chunk boundaries (the same person may be labelled `A` in one chunk and `B` in the next). A typical-length episode fits in a single request and is fully consistent. -- **Deepgram**: sends the whole episode in one request, so speaker labels stay consistent across the entire episode. This requires a separate **Deepgram API key**, which you can create at [deepgram.com](https://deepgram.com) (new accounts include free credit). Your Deepgram key is stored separately from your OpenAI key and is only used for diarization. +- **OpenAI** (`gpt-4o-transcribe-diarize`) reuses the OpenAI secret selected + above. Because each request is capped at about 20 MB, a conservative margin + under OpenAI's 25 MB request cap, a long episode is diarized in independent + chunks. Speaker labels can therefore reset across chunk boundaries. A typical + episode fits in one request and keeps consistent labels. +- **Deepgram** sends the whole episode in one request, so speaker labels remain + consistent throughout the episode. Create a separate key at + [deepgram.com](https://deepgram.com), then select an existing Obsidian secret + or create one under **Deepgram API key**. The Deepgram secret is used only for + Deepgram diarization and is local to the current vault and device. Diarization is off by default, so existing transcripts and the plain-Whisper workflow are unchanged unless you enable it. @@ -53,7 +114,9 @@ The **Speaker label format** setting controls the prefix added before each speak - OpenAI labels speakers `A`, `B`, `C`, … - Deepgram labels speakers `1`, `2`, `3`, … -The default is `**{{speaker}}:** `, which renders as `**A:**`-style bold prefixes. To spell out the word "Speaker", set it to `**Speaker {{speaker}}:** ` (rendering `**Speaker A:**`); `> {{speaker}}: ` would instead put each turn in a blockquote. +The default is `**{{speaker}}:** `, which renders as an `**A:**`-style bold +prefix. To spell out the word "Speaker", use `**Speaker {{speaker}}:** `. To put +each turn in a blockquote, use `> {{speaker}}: `. The labelled transcript replaces the usual `{{transcript}}` value in your [transcript template](#transcript-template), so you don't need to change your template to use diarization. diff --git a/manifest.json b/manifest.json index 8ef84133..5ff3c9b2 100644 --- a/manifest.json +++ b/manifest.json @@ -2,7 +2,7 @@ "id": "podnotes", "name": "PodNotes", "version": "2.18.4", - "minAppVersion": "0.15.9", + "minAppVersion": "1.11.5", "description": "Helps you write notes on podcasts.", "author": "Christian B. B. Houmann", "authorUrl": "https://bagerbach.com", diff --git a/scripts/provision-obsidian-e2e-vault.mjs b/scripts/provision-obsidian-e2e-vault.mjs index fbf8fc6a..4a0dc5fb 100644 --- a/scripts/provision-obsidian-e2e-vault.mjs +++ b/scripts/provision-obsidian-e2e-vault.mjs @@ -27,7 +27,7 @@ export const PODNOTES_READY_EVAL = `Boolean(app.plugins.plugins[${JSON.stringify // constants.ts. (currentEpisode is intentionally omitted - DEFAULT_SETTINGS sets it to // undefined, which JSON cannot represent and PodNotes treats as absent.) export const DEFAULT_PODNOTES_DATA = { - schemaVersion: 1, + schemaVersion: 2, savedFeeds: {}, podNotes: {}, defaultPlaybackRate: 1, @@ -104,8 +104,8 @@ export const DEFAULT_PODNOTES_DATA = { shouldRepeat: false, episodes: [], }, - openAIApiKey: "", - diarizationApiKey: "", + openAISecretId: "", + deepgramSecretId: "", transcript: { path: "transcripts/{{podcast}}/{{title}}.md", template: "# {{title}}\n\nPodcast: {{podcast}}\nDate: {{date}}\n\n{{transcript}}", diff --git a/scripts/provision-obsidian-e2e-vault.test.ts b/scripts/provision-obsidian-e2e-vault.test.ts index 556e9db6..40889e2e 100644 --- a/scripts/provision-obsidian-e2e-vault.test.ts +++ b/scripts/provision-obsidian-e2e-vault.test.ts @@ -49,13 +49,13 @@ describe("provision-obsidian-e2e-vault", () => { expect(options.vaultPath).toBe("/tmp/podnotes-repo/vaults/podnotes-a"); }); - it("seeds schema v1 data that mirrors the real DEFAULT_SETTINGS", () => { + it("seeds schema v2 data that mirrors the real DEFAULT_SETTINGS", () => { // JSON cannot represent currentEpisode: undefined, so compare the // serialized forms - this is exactly what lands in the vault's data.json // and it fails if a new setting is added to src/constants.ts without // updating DEFAULT_PODNOTES_DATA. expect(JSON.parse(JSON.stringify(DEFAULT_PODNOTES_DATA))).toEqual({ - schemaVersion: 1, + schemaVersion: 2, ...JSON.parse(JSON.stringify(DEFAULT_SETTINGS)), }); }); diff --git a/src/constants.ts b/src/constants.ts index d11f9793..5ac40069 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -154,8 +154,8 @@ export const DEFAULT_SETTINGS: IPodNotesSettings = { ...LOCAL_FILES_SETTINGS, episodes: [], }, - openAIApiKey: "", - diarizationApiKey: "", + openAISecretId: "", + deepgramSecretId: "", transcript: { path: "transcripts/{{podcast}}/{{title}}.md", template: "# {{title}}\n\nPodcast: {{podcast}}\nDate: {{date}}\n\n{{transcript}}", diff --git a/src/main.activateView.test.ts b/src/main.activateView.test.ts index e4ef0a60..863e311e 100644 --- a/src/main.activateView.test.ts +++ b/src/main.activateView.test.ts @@ -254,6 +254,11 @@ describe("PodNotes onload wiring (#55)", () => { storeUnsubscribers: [], views: new Set(), app: { + secretStorage: { + getSecret: vi.fn(() => null), + setSecret: vi.fn(), + listSecrets: vi.fn(() => []), + }, workspace: { onLayoutReady: vi.fn(), on: vi.fn(() => ({})), @@ -398,7 +403,7 @@ describe("PodNotes onload wiring (#55)", () => { mediaType: "video", }; const { commands } = await loadPlugin({ - openAIApiKey: "sk-test", + openAISecretId: "podnotes-openai-api-key", currentEpisode: videoEpisode, }); diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index 52a32e74..36952ea0 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -1,12 +1,27 @@ import { afterEach, describe, expect, it, vi } from "vitest"; +import type { SecretStorage } from "obsidian"; import { DEFAULT_SETTINGS } from "./constants"; import PodNotes from "./main"; +import { CredentialRepository } from "./services/CredentialRepository"; + +function memorySecretStorage(initial: Record = {}) { + const values = new Map(Object.entries(initial)); + const storage = { + getSecret: vi.fn((id: string) => values.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => values.set(id, value)), + listSecrets: vi.fn(() => [...values.keys()]), + } as unknown as SecretStorage; + return { storage, values }; +} function makePlugin( saveData: (data: unknown) => Promise = vi.fn().mockResolvedValue(undefined), ): PodNotes { + const { storage } = memorySecretStorage(); const plugin = Object.create(PodNotes.prototype) as PodNotes; Object.assign(plugin, { + app: { secretStorage: storage }, + credentials: new CredentialRepository(storage), isReady: true, settings: structuredClone(DEFAULT_SETTINGS), pendingSave: null, @@ -51,15 +66,15 @@ describe("PodNotes persistence integration", () => { const saveData = vi.fn().mockResolvedValue(undefined); const plugin = makePlugin(saveData); Object.assign(plugin, { - loadData: vi.fn().mockResolvedValue({ schemaVersion: 2 }), + loadData: vi.fn().mockResolvedValue({ schemaVersion: 3 }), }); vi.spyOn(console, "error").mockImplementation(() => undefined); - await expect(plugin.loadSettings()).rejects.toThrow(/schema v2/); + await expect(plugin.loadSettings()).rejects.toThrow(/schema v3/); expect(saveData).not.toHaveBeenCalled(); }); - it("writes schema v1, canonical dates, and preserved unknown fields", async () => { + it("writes schema v2, canonical dates, and preserved unknown fields", async () => { const saveData = vi.fn().mockResolvedValue(undefined); const plugin = makePlugin(saveData); plugin.settings.currentEpisode = { @@ -77,7 +92,7 @@ describe("PodNotes persistence integration", () => { expect(saveData).toHaveBeenCalledWith( expect.objectContaining({ - schemaVersion: 1, + schemaVersion: 2, retained: { enabled: true }, currentEpisode: expect.objectContaining({ episodeDate: "2024-03-01T10:05:03.000Z", @@ -86,6 +101,176 @@ describe("PodNotes persistence integration", () => { ); }); + it("moves legacy credentials into SecretStorage before writing schema v2", async () => { + const saveData = vi.fn().mockResolvedValue(undefined); + const { storage, values } = memorySecretStorage(); + const plugin = makePlugin(saveData); + Object.assign(plugin, { + app: { secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue({ + schemaVersion: 1, + openAIApiKey: "sk-legacy", + diarizationApiKey: "dg-legacy", + retained: true, + }), + }); + + await plugin.loadSettings(); + + expect(values.get("podnotes-openai-api-key")).toBe("sk-legacy"); + expect(values.get("podnotes-deepgram-api-key")).toBe("dg-legacy"); + expect(plugin.settings.openAISecretId).toBe("podnotes-openai-api-key"); + expect(plugin.settings.deepgramSecretId).toBe("podnotes-deepgram-api-key"); + expect(saveData).toHaveBeenCalledTimes(1); + const persisted = saveData.mock.calls[0][0] as Record; + expect(persisted).toEqual( + expect.objectContaining({ + schemaVersion: 2, + openAISecretId: "podnotes-openai-api-key", + deepgramSecretId: "podnotes-deepgram-api-key", + retained: true, + }), + ); + expect(persisted).not.toHaveProperty("openAIApiKey"); + expect(persisted).not.toHaveProperty("diarizationApiKey"); + }); + + it("leaves legacy data untouched when SecretStorage migration fails", async () => { + const saveData = vi.fn().mockResolvedValue(undefined); + const raw = { schemaVersion: 1, openAIApiKey: "sk", defaultVolume: 0.3 }; + const storage = { + getSecret: vi.fn(() => null), + setSecret: vi.fn(() => { + throw new Error("keychain unavailable"); + }), + listSecrets: vi.fn(() => []), + } as unknown as SecretStorage; + const plugin = makePlugin(saveData); + Object.assign(plugin, { + app: { secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue(raw), + }); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await expect(plugin.loadSettings()).rejects.toThrow("keychain unavailable"); + expect(saveData).not.toHaveBeenCalled(); + expect(raw).toEqual({ schemaVersion: 1, openAIApiKey: "sk", defaultVolume: 0.3 }); + }); + + it("does not save v2 when the second credential write fails", async () => { + const saveData = vi.fn().mockResolvedValue(undefined); + const values = new Map(); + const storage = { + getSecret: vi.fn((id: string) => values.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => { + if (id.startsWith("podnotes-deepgram")) throw new Error("Deepgram write failed"); + values.set(id, value); + }), + listSecrets: vi.fn(() => [...values.keys()]), + } as unknown as SecretStorage; + const plugin = makePlugin(saveData); + Object.assign(plugin, { + app: { secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue({ + schemaVersion: 1, + openAIApiKey: "sk-created", + diarizationApiKey: "dg-fails", + }), + }); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await expect(plugin.loadSettings()).rejects.toThrow("Deepgram write failed"); + expect(values.get("podnotes-openai-api-key")).toBe("sk-created"); + expect(saveData).not.toHaveBeenCalled(); + }); + + it("reuses an already-created secret after the v2 save fails and is retried", async () => { + const { storage, values } = memorySecretStorage(); + const raw = { schemaVersion: 1, openAIApiKey: "sk-retry" }; + const first = makePlugin(vi.fn().mockRejectedValue(new Error("disk full"))); + Object.assign(first, { + app: { secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue(raw), + }); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await expect(first.loadSettings()).rejects.toThrow("disk full"); + expect(values.get("podnotes-openai-api-key")).toBe("sk-retry"); + + const retrySave = vi.fn().mockResolvedValue(undefined); + const retry = makePlugin(retrySave); + Object.assign(retry, { + app: { secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue(raw), + }); + await retry.loadSettings(); + + expect(retry.settings.openAISecretId).toBe("podnotes-openai-api-key"); + expect(values.has("podnotes-openai-api-key-2")).toBe(false); + expect(retrySave).toHaveBeenCalledWith( + expect.objectContaining({ + schemaVersion: 2, + openAISecretId: "podnotes-openai-api-key", + }), + ); + }); + + it("does not rewrite data that is already schema v2", async () => { + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData); + Object.assign(plugin, { + loadData: vi.fn().mockResolvedValue({ schemaVersion: 2, defaultVolume: 0.4 }), + }); + + await plugin.loadSettings(); + + expect(plugin.settings.defaultVolume).toBe(0.4); + expect(saveData).not.toHaveBeenCalled(); + }); + + it("scrubs retired plaintext fields from schema v2 without importing them", async () => { + const saveData = vi.fn().mockResolvedValue(undefined); + const { storage, values } = memorySecretStorage(); + const plugin = makePlugin(saveData); + Object.assign(plugin, { + app: { secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue({ + schemaVersion: 2, + openAIApiKey: "must-not-import", + defaultVolume: 0.4, + }), + }); + + await plugin.loadSettings(); + + expect(values.size).toBe(0); + expect(plugin.settings.openAISecretId).toBe(""); + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.calls[0][0]).toEqual( + expect.objectContaining({ schemaVersion: 2, defaultVolume: 0.4 }), + ); + expect(saveData.mock.calls[0][0]).not.toHaveProperty("openAIApiKey"); + }); + + it("fails closed if a v2 retired-field scrub cannot be persisted", async () => { + const plugin = makePlugin(vi.fn().mockRejectedValue(new Error("disk full"))); + Object.assign(plugin, { + loadData: vi + .fn() + .mockResolvedValue({ schemaVersion: 2, openAIApiKey: "must-stay-unread" }), + }); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await expect(plugin.loadSettings()).rejects.toThrow("disk full"); + expect(plugin.settings).toEqual(DEFAULT_SETTINGS); + }); + it("rejects a strict caller when saveData fails", async () => { const failure = new Error("disk full"); const plugin = makePlugin(vi.fn().mockRejectedValue(failure)); diff --git a/src/main.ts b/src/main.ts index 15e4ea2f..8b450b89 100644 --- a/src/main.ts +++ b/src/main.ts @@ -37,8 +37,10 @@ import { normalizePlaybackRate } from "./utility/playbackRate"; import { decodePodNotesData, encodePodNotesData, + PODNOTES_DATA_SCHEMA_VERSION, PodNotesDataError, } from "./persistence/podNotesData"; +import { CredentialRepository } from "./services/CredentialRepository"; type MediaSessionActionName = | "previoustrack" @@ -56,10 +58,21 @@ interface SaveWaiter { reject: (error: unknown) => void; } +class PodNotesSchemaMigrationError extends Error { + constructor( + public readonly originalCause: unknown, + public readonly includedCredentials: boolean, + ) { + super(originalCause instanceof Error ? originalCause.message : String(originalCause)); + this.name = "PodNotesSchemaMigrationError"; + } +} + export default class PodNotes extends Plugin implements IPodNotes { public api!: IAPI; public override settings!: IPodNotesSettings; public override app!: PartialAppExtension; + public credentials!: CredentialRepository; private views = new Set(); @@ -88,6 +101,7 @@ export default class PodNotes extends Plugin implements IPodNotes { this.isUnloaded = false; this.podcastViewMountEnabled = !this.isMobileRuntime(); plugin.set(this); + this.credentials = new CredentialRepository(this.app.secretStorage); await this.loadSettings(); @@ -290,6 +304,10 @@ export default class PodNotes extends Plugin implements IPodNotes { return this.transcriptionService; } + invalidateTranscriptionCredentialCache(): void { + this.transcriptionService?.clearCredentialCache(); + } + captureTimestamp(editor: Editor | null | undefined): boolean { if (!editor || !this.api.podcast || !this.settings.timestamp.template) { return false; @@ -401,6 +419,8 @@ export default class PodNotes extends Plugin implements IPodNotes { this.storeUnsubscribers = []; this.volumeUnsubscribe?.(); this.localFilesMirrorUnsubscribe?.(); + this.transcriptionService?.dispose(); + this.transcriptionService = undefined; this.views.clear(); // Intentionally do NOT detach the view's leaves here. Obsidian persists and @@ -419,7 +439,43 @@ export default class PodNotes extends Plugin implements IPodNotes { async loadSettings() { try { const decoded = decodePodNotesData(await this.loadData()); - this.settings = decoded.settings; + let settings = decoded.settings; + this.credentials ??= new CredentialRepository(this.app.secretStorage); + + if ( + decoded.sourceVersion < PODNOTES_DATA_SCHEMA_VERSION || + decoded.retiredPlaintextPresent + ) { + // SecretStorage writes are synchronous. Store every legacy value and + // verify it first, then write the v2 snapshot directly because the normal + // save queue intentionally stays dormant until onload finishes. A v2 file + // carrying retired fields is scrubbed without importing their values. If + // any step fails, the old data.json is left untouched and loading stops. + try { + const references = + decoded.sourceVersion < PODNOTES_DATA_SCHEMA_VERSION + ? this.credentials.storeValues(decoded.legacySecrets) + : {}; + settings = { ...settings, ...references }; + await this.saveData(encodePodNotesData(settings, decoded.unknownFields)); + } catch (error) { + throw new PodNotesSchemaMigrationError( + error, + Boolean(decoded.legacySecrets.openAI || decoded.legacySecrets.deepgram), + ); + } + const migratedProviders = [ + decoded.legacySecrets.openAI ? "OpenAI" : null, + decoded.legacySecrets.deepgram ? "Deepgram" : null, + ].filter((provider): provider is string => provider !== null); + if (migratedProviders.length > 0) { + new Notice( + `Moved your ${migratedProviders.join(" and ")} API ${migratedProviders.length === 1 ? "key" : "keys"} into Obsidian SecretStorage.`, + ); + } + } + + this.settings = settings; this.persistenceUnknownFields = decoded.unknownFields; if (decoded.warnings.length > 0) { @@ -429,6 +485,14 @@ export default class PodNotes extends Plugin implements IPodNotes { if (error instanceof PodNotesDataError) { new Notice(error.message, 0); console.error("PodNotes refused to load unsafe persisted data", error); + } else if (error instanceof PodNotesSchemaMigrationError) { + new Notice( + error.includedCredentials + ? "PodNotes could not securely migrate its API keys. Your existing data was kept unchanged. Restart PodNotes to retry." + : "PodNotes could not upgrade its settings data. Your existing data was kept unchanged. Restart PodNotes to retry.", + 0, + ); + console.error("PodNotes failed to migrate persisted settings", error.originalCause); } throw error; } diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index 6aa1f61c..a46a878f 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -89,7 +89,7 @@ describe("PodNotes data schema", () => { expect((persisted.currentEpisode as Record).episodeDate).toBe( episodeDate.toISOString(), ); - expect(decoded.sourceVersion).toBe(1); + expect(decoded.sourceVersion).toBe(2); expect(decoded.changed).toBe(false); expect(decoded.settings.currentEpisode?.episodeDate).toEqual(episodeDate); expect(decoded.settings.queue.episodes[0].episodeDate).toEqual(episodeDate); @@ -134,7 +134,7 @@ describe("PodNotes data schema", () => { }); }); - it("preserves an intentionally disabled note feature in schema v1 and on save", () => { + it("preserves an intentionally disabled note feature from schema v1 and on v2 save", () => { const decoded = decodePodNotesData({ schemaVersion: 1, note: { path: "", template: "" }, @@ -145,6 +145,61 @@ describe("PodNotes data schema", () => { expect(persisted.note).toEqual({ path: "", template: "" }); }); + it("extracts legacy plaintext credentials without putting them in runtime settings", () => { + const decoded = decodePodNotesData({ + schemaVersion: 1, + openAIApiKey: " sk-legacy ", + diarizationApiKey: "dg-legacy", + }); + + expect(decoded.legacySecrets).toEqual({ openAI: "sk-legacy", deepgram: "dg-legacy" }); + expect(decoded.settings).not.toHaveProperty("openAIApiKey"); + expect(decoded.settings).not.toHaveProperty("diarizationApiKey"); + expect(decoded.unknownFields).not.toHaveProperty("openAIApiKey"); + expect(decoded.unknownFields).not.toHaveProperty("diarizationApiKey"); + }); + + it("retires plaintext fields even if they appear in v2 data", () => { + const decoded = decodePodNotesData({ + schemaVersion: 2, + openAIApiKey: "must-not-survive", + diarizationApiKey: "must-not-survive", + }); + const persisted = encodePodNotesData(decoded.settings, decoded.unknownFields); + + expect(decoded.legacySecrets).toEqual({}); + expect(decoded.retiredPlaintextPresent).toBe(true); + expect(decoded.changed).toBe(true); + expect(decoded.unknownFields).not.toHaveProperty("openAIApiKey"); + expect(decoded.unknownFields).not.toHaveProperty("diarizationApiKey"); + expect(JSON.stringify(persisted)).not.toContain("must-not-survive"); + expect(persisted).not.toHaveProperty("openAIApiKey"); + expect(persisted).not.toHaveProperty("diarizationApiKey"); + }); + + it("preserves provider-owned SecretStorage IDs and removes tampered v2 references", () => { + const valid = decodePodNotesData({ + schemaVersion: 2, + openAISecretId: "podnotes-openai-api-key-2", + }); + const foreign = decodePodNotesData({ + schemaVersion: 2, + openAISecretId: "shared-global-api-key", + deepgramSecretId: "podnotes-openai-api-key", + }); + + expect(valid.settings.openAISecretId).toBe("podnotes-openai-api-key-2"); + expect(foreign.settings.openAISecretId).toBe(""); + expect(foreign.settings.deepgramSecretId).toBe(""); + expect(foreign.changed).toBe(true); + expect(foreign.warnings).toContain( + "openAISecretId: foreign or wrong-provider SecretStorage ID; reference was removed", + ); + expect(foreign.warnings).toContain( + "deepgramSecretId: foreign or wrong-provider SecretStorage ID; reference was removed", + ); + }); + it("salvages valid fields and only drops individually invalid collection entries", () => { const validEpisode = { ...episode("Valid"), @@ -211,7 +266,7 @@ describe("PodNotes data schema", () => { expect(decoded.settings.downloadedEpisodes["Old Podcast"][0].size).toBe(0); }); - it("preserves safe unknown root and nested fields across a v1 save", () => { + it("preserves safe unknown root and nested fields across a schema upgrade", () => { const decoded = decodePodNotesData({ schemaVersion: 1, futureRootField: { enabled: true }, @@ -250,8 +305,8 @@ describe("PodNotes data schema", () => { }); it("fails closed for a future schema version", () => { - expect(() => decodePodNotesData({ schemaVersion: 2 })).toThrowError( - /schema v2 requires a newer version/, + expect(() => decodePodNotesData({ schemaVersion: 3 })).toThrowError( + /schema v3 requires a newer version/, ); }); }); diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index 873373a9..ad3a7fbc 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -8,12 +8,22 @@ import { type PersistedPlaylist, } from "./episodeCodec"; import { decodeSettings } from "./settingsCodec"; +import type { CredentialValues } from "src/types/Credentials"; export { decodeEpisode, decodePlaylist, encodeEpisode, encodePlaylist } from "./episodeCodec"; -export const PODNOTES_DATA_SCHEMA_VERSION = 1; +export const PODNOTES_DATA_SCHEMA_VERSION = 2; -const KNOWN_TOP_LEVEL_KEYS = new Set([...Object.keys(DEFAULT_SETTINGS), "schemaVersion"]); +/** Plaintext fields accepted only as migration input and never persisted again. */ +export const RETIRED_PLAINTEXT_SECRET_KEYS = new Set(["openAIApiKey", "diarizationApiKey"]); + +const KNOWN_TOP_LEVEL_KEYS = new Set([ + ...Object.keys(DEFAULT_SETTINGS), + ...RETIRED_PLAINTEXT_SECRET_KEYS, + "schemaVersion", +]); + +export type PodNotesDataSchemaVersion = 0 | 1 | typeof PODNOTES_DATA_SCHEMA_VERSION; export class PodNotesDataError extends Error { constructor( @@ -27,10 +37,14 @@ export class PodNotesDataError extends Error { export interface DecodedPodNotesData { settings: IPodNotesSettings; - sourceVersion: 0 | typeof PODNOTES_DATA_SCHEMA_VERSION; + sourceVersion: PodNotesDataSchemaVersion; changed: boolean; warnings: string[]; - /** Unknown v0/v1 root fields retained so a normal save does not erase them. */ + /** Plaintext v0/v1 values that must move into SecretStorage before a v2 save. */ + legacySecrets: CredentialValues; + /** Retired fields found in v2 must be scrubbed without importing their values. */ + retiredPlaintextPresent: boolean; + /** Unknown supported-schema root fields retained so a normal save does not erase them. */ unknownFields: UnknownRecord; } @@ -46,7 +60,7 @@ export type PersistedPodNotesSettings = Omit< downloadedEpisodes: Record; }; -export type PersistedPodNotesDataV1 = PersistedPodNotesSettings & +export type PersistedPodNotesDataV2 = PersistedPodNotesSettings & UnknownRecord & { schemaVersion: typeof PODNOTES_DATA_SCHEMA_VERSION; }; @@ -54,7 +68,7 @@ export type PersistedPodNotesDataV1 = PersistedPodNotesSettings & /** * Decode and validate plugin data before any store sees it. * - * Missing `schemaVersion` is the legacy v0 shape. V1 deliberately remains flat + * Missing `schemaVersion` is the legacy v0 shape. Persisted data remains flat * so existing data files, rollback paths, and E2E tooling stay compatible. A * newer version fails closed so an older plugin can never overwrite data it * does not understand. @@ -72,13 +86,22 @@ export function decodePodNotesData(value: unknown): DecodedPodNotesData { const sourceVersion = readSchemaVersion(value); const warnings = new Set(); const settings = decodeSettings(value, warnings, sourceVersion); + const legacySecrets = decodeLegacySecrets(value, sourceVersion, warnings); + const retiredPlaintextPresent = [...RETIRED_PLAINTEXT_SECRET_KEYS].some((key) => + Object.prototype.hasOwnProperty.call(value, key), + ); const unknownFields = copyUnknownRootFields(value); return { settings, sourceVersion, - changed: sourceVersion !== PODNOTES_DATA_SCHEMA_VERSION || warnings.size > 0, + changed: + sourceVersion !== PODNOTES_DATA_SCHEMA_VERSION || + retiredPlaintextPresent || + warnings.size > 0, warnings: [...warnings], + legacySecrets, + retiredPlaintextPresent, unknownFields, }; } @@ -87,8 +110,12 @@ export function decodePodNotesData(value: unknown): DecodedPodNotesData { export function encodePodNotesData( settings: IPodNotesSettings, unknownFields: UnknownRecord = {}, -): PersistedPodNotesDataV1 { - const validated = decodeSettings(settings as unknown as UnknownRecord, new Set(), 1); +): PersistedPodNotesDataV2 { + const validated = decodeSettings( + settings as unknown as UnknownRecord, + new Set(), + PODNOTES_DATA_SCHEMA_VERSION, + ); return { ...copyUnknownRootFields(unknownFields), @@ -104,15 +131,19 @@ export function encodePodNotesData( downloadedEpisodes: mapRecord(validated.downloadedEpisodes, (episodes) => episodes.map((episode) => encodeEpisode(episode)), ), - } as PersistedPodNotesDataV1; + } as PersistedPodNotesDataV2; } -function readSchemaVersion(root: UnknownRecord): 0 | 1 { +function readSchemaVersion(root: UnknownRecord): PodNotesDataSchemaVersion { if (!Object.prototype.hasOwnProperty.call(root, "schemaVersion")) return 0; const version = root.schemaVersion; - if (version === PODNOTES_DATA_SCHEMA_VERSION) return version; - if (typeof version === "number" && Number.isInteger(version) && version > 1) { + if (version === 1 || version === PODNOTES_DATA_SCHEMA_VERSION) return version; + if ( + typeof version === "number" && + Number.isInteger(version) && + version > PODNOTES_DATA_SCHEMA_VERSION + ) { throw new PodNotesDataError( `PodNotes data schema v${version} requires a newer version of PodNotes. The file was not modified.`, "unsupported-version", @@ -125,6 +156,37 @@ function readSchemaVersion(root: UnknownRecord): 0 | 1 { ); } +function decodeLegacySecrets( + root: UnknownRecord, + sourceVersion: PodNotesDataSchemaVersion, + warnings: Set, +): CredentialValues { + if (sourceVersion >= PODNOTES_DATA_SCHEMA_VERSION) return {}; + + const values: CredentialValues = {}; + readLegacySecret(root, "openAIApiKey", "openAI", values, warnings); + readLegacySecret(root, "diarizationApiKey", "deepgram", values, warnings); + return values; +} + +function readLegacySecret( + root: UnknownRecord, + legacyKey: "openAIApiKey" | "diarizationApiKey", + credentialKey: keyof CredentialValues, + values: CredentialValues, + warnings: Set, +): void { + const value = root[legacyKey]; + if (value === undefined || value === null || value === "") return; + if (typeof value !== "string") { + warnings.add(`${legacyKey}: expected a string; value was ignored`); + return; + } + + const normalized = value.trim(); + if (normalized) values[credentialKey] = normalized; +} + function copyUnknownRootFields(root: UnknownRecord): UnknownRecord { const copy: UnknownRecord = {}; for (const [key, value] of Object.entries(root)) { diff --git a/src/persistence/settingsCodec.ts b/src/persistence/settingsCodec.ts index 7e1c9ebd..0378fd5f 100644 --- a/src/persistence/settingsCodec.ts +++ b/src/persistence/settingsCodec.ts @@ -7,6 +7,7 @@ import { migrateTranscriptSettings, } from "src/settingsMigrations"; import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; +import { isPodNotesSecretId, type CredentialKind } from "src/types/Credentials"; import { sanitizeEpisodeListLimit } from "src/utility/episodeListLimit"; import { normalizePlaybackRate } from "src/utility/playbackRate"; import { @@ -29,11 +30,12 @@ import { warn, } from "./codecUtils"; import { decodeEpisode, decodePlaylist } from "./episodeCodec"; +import type { PodNotesDataSchemaVersion } from "./podNotesData"; export function decodeSettings( root: UnknownRecord, warnings: Set, - sourceVersion: 0 | 1 = 1, + sourceVersion: PodNotesDataSchemaVersion = 2, ): IPodNotesSettings { const defaultPlaybackRate = normalizePlaybackRate(root.defaultPlaybackRate); if ( @@ -109,18 +111,26 @@ export function decodeSettings( feedNote: decodeFeedNote(root.feedNote, warnings), download: decodeDownload(root.download, warnings), downloadedEpisodes: decodeDownloadedEpisodes(root.downloadedEpisodes, warnings), - openAIApiKey: readString(root, "openAIApiKey", DEFAULT_SETTINGS.openAIApiKey, warnings), - diarizationApiKey: readString( - root, - "diarizationApiKey", - DEFAULT_SETTINGS.diarizationApiKey, - warnings, - ), + openAISecretId: decodeSecretId(root, "openAISecretId", warnings), + deepgramSecretId: decodeSecretId(root, "deepgramSecretId", warnings), transcript: decodeTranscript(root.transcript, warnings), feedCache: decodeFeedCache(root.feedCache, warnings), }; } +function decodeSecretId( + root: UnknownRecord, + key: "openAISecretId" | "deepgramSecretId", + warnings: Set, +): string { + const value = readString(root, key, DEFAULT_SETTINGS[key], warnings); + const kind: CredentialKind = key === "openAISecretId" ? "openai" : "deepgram"; + if (!value || isPodNotesSecretId(kind, value)) return value; + + warn(warnings, key, "foreign or wrong-provider SecretStorage ID; reference was removed"); + return ""; +} + function decodeTimestamp(value: unknown, warnings: Set): IPodNotesSettings["timestamp"] { const record = optionalRecord(value, warnings, "timestamp"); return { @@ -145,7 +155,7 @@ function decodeTimestamp(value: unknown, warnings: Set): IPodNotesSettin function decodeNote( value: unknown, warnings: Set, - sourceVersion: 0 | 1, + sourceVersion: PodNotesDataSchemaVersion, ): IPodNotesSettings["note"] { const record = optionalRecord(value, warnings, "note"); const path = readNullableString(record, "path", warnings, "note"); diff --git a/src/services/CredentialRepository.test.ts b/src/services/CredentialRepository.test.ts new file mode 100644 index 00000000..3b4df3d9 --- /dev/null +++ b/src/services/CredentialRepository.test.ts @@ -0,0 +1,181 @@ +import { describe, expect, it, vi } from "vitest"; +import type { SecretStorage } from "obsidian"; +import { DEFAULT_SETTINGS } from "src/constants"; +import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; +import { CredentialRepository } from "./CredentialRepository"; + +function storage(initial: Record = {}) { + const values = new Map(Object.entries(initial)); + return { + values, + api: { + getSecret: vi.fn((id: string) => values.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => { + values.set(id, value); + }), + listSecrets: vi.fn(() => [...values.keys()]), + } as unknown as SecretStorage, + }; +} + +function settings(overrides: Partial = {}): IPodNotesSettings { + return { ...structuredClone(DEFAULT_SETTINGS), ...overrides }; +} + +describe("CredentialRepository", () => { + it("stores credentials under fixed PodNotes IDs and verifies them", () => { + const { api, values } = storage(); + const repository = new CredentialRepository(api); + + expect(repository.storeValues({ openAI: " sk-openai ", deepgram: "dg-key" })).toEqual({ + openAISecretId: "podnotes-openai-api-key", + deepgramSecretId: "podnotes-deepgram-api-key", + }); + expect(values.get("podnotes-openai-api-key")).toBe("sk-openai"); + expect(values.get("podnotes-deepgram-api-key")).toBe("dg-key"); + expect(api.getSecret).toHaveBeenCalledWith("podnotes-openai-api-key"); + }); + + it("reuses an exact value so a migration retry is idempotent", () => { + const { api } = storage({ "podnotes-openai-api-key": "sk-existing" }); + const repository = new CredentialRepository(api); + + expect(repository.storeValues({ openAI: "sk-existing" })).toEqual({ + openAISecretId: "podnotes-openai-api-key", + }); + expect(api.setSecret).not.toHaveBeenCalled(); + }); + + it("uses a collision-safe suffix and never overwrites another value", () => { + const { api, values } = storage({ + "podnotes-openai-api-key": "someone-else", + "podnotes-openai-api-key-2": "also-taken", + }); + const repository = new CredentialRepository(api); + + expect(repository.storeValues({ openAI: "sk-new" })).toEqual({ + openAISecretId: "podnotes-openai-api-key-3", + }); + expect(values.get("podnotes-openai-api-key")).toBe("someone-else"); + expect(values.get("podnotes-openai-api-key-3")).toBe("sk-new"); + }); + + it("fails when SecretStorage cannot read back the written value", () => { + const api = { + getSecret: vi.fn().mockReturnValueOnce(null).mockReturnValueOnce(null), + setSecret: vi.fn(), + listSecrets: vi.fn(() => []), + } as unknown as SecretStorage; + const repository = new CredentialRepository(api); + + expect(() => repository.storeValues({ openAI: "sk" })).toThrow( + /SecretStorage did not retain/, + ); + }); + + it("runtime reads fail closed without retaining values in the repository", () => { + const { api } = storage({ "podnotes-openai-api-key": " sk-runtime " }); + const repository = new CredentialRepository(api); + const configured = settings({ openAISecretId: "podnotes-openai-api-key" }); + + expect(repository.get(configured, "openai")).toBe("sk-runtime"); + expect(repository.has(configured, "openai")).toBe(true); + expect(repository.get(settings(), "openai")).toBeNull(); + + (api.getSecret as ReturnType).mockImplementation(() => { + throw new Error("storage unavailable"); + }); + vi.spyOn(console, "error").mockImplementation(() => undefined); + expect(repository.get(configured, "openai")).toBeNull(); + }); + + it("never dereferences foreign or cross-provider persisted references", () => { + const { api } = storage({ + "shared-api-key": "foreign-value", + "podnotes-deepgram-api-key": "deepgram-value", + "podnotes-openai-api-key": "openai-value", + }); + const repository = new CredentialRepository(api); + + expect(repository.get(settings({ openAISecretId: "shared-api-key" }), "openai")).toBeNull(); + expect( + repository.get(settings({ openAISecretId: "podnotes-deepgram-api-key" }), "openai"), + ).toBeNull(); + expect( + repository.get(settings({ deepgramSecretId: "podnotes-openai-api-key" }), "deepgram"), + ).toBeNull(); + expect(api.getSecret).not.toHaveBeenCalled(); + }); + + it("copies an explicitly selected foreign secret into a provider-owned ID", () => { + const { api, values } = storage({ + "shared-api-key": " sk-shared ", + "podnotes-openai-api-key": "existing-value", + }); + const repository = new CredentialRepository(api); + + expect(repository.adoptReference("openai", "shared-api-key")).toBe( + "podnotes-openai-api-key-2", + ); + expect(values.get("podnotes-openai-api-key-2")).toBe("sk-shared"); + expect(api.getSecret).toHaveBeenLastCalledWith("podnotes-openai-api-key-2"); + }); + + it("copies an explicitly selected cross-provider secret into the selected provider", () => { + const { api, values } = storage({ "podnotes-deepgram-api-key": "shared-by-user" }); + const repository = new CredentialRepository(api); + + expect(repository.adoptReference("openai", "podnotes-deepgram-api-key")).toBe( + "podnotes-openai-api-key", + ); + expect(values.get("podnotes-openai-api-key")).toBe("shared-by-user"); + }); + + it("rejects a missing explicit selection without creating a reference", () => { + const { api, values } = storage(); + const repository = new CredentialRepository(api); + + expect(() => repository.adoptReference("openai", "missing-secret")).toThrow( + "not available on this device", + ); + expect(values.size).toBe(0); + }); + + it("exports only values whose referenced secrets still exist", () => { + const { api } = storage({ + "podnotes-openai-api-key": "sk", + "podnotes-deepgram-api-key": " ", + }); + const repository = new CredentialRepository(api); + + expect( + repository.exportValues( + settings({ + openAISecretId: "podnotes-openai-api-key", + deepgramSecretId: "podnotes-deepgram-api-key", + }), + ), + ).toEqual({ openAI: "sk" }); + }); + + it("distinguishes unconfigured and synced-but-missing references", () => { + const { api } = storage(); + const repository = new CredentialRepository(api); + + expect(repository.status(settings(), "openai")).toBe("unconfigured"); + expect( + repository.status(settings({ openAISecretId: "podnotes-openai-api-key" }), "openai"), + ).toBe("missing"); + }); + + it("refuses an explicit plaintext export when a configured value is missing locally", () => { + const { api } = storage(); + const repository = new CredentialRepository(api); + + expect(() => + repository.exportValues(settings({ openAISecretId: "podnotes-openai-api-key" }), { + requireConfigured: true, + }), + ).toThrow("not available in SecretStorage on this device"); + }); +}); diff --git a/src/services/CredentialRepository.ts b/src/services/CredentialRepository.ts new file mode 100644 index 00000000..95f86c05 --- /dev/null +++ b/src/services/CredentialRepository.ts @@ -0,0 +1,143 @@ +import type { SecretStorage } from "obsidian"; +import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; +import { + isPodNotesSecretId, + isValidSecretId, + type CredentialKind, + type CredentialReferences, + type CredentialValues, +} from "src/types/Credentials"; + +export type { CredentialKind, CredentialReferences, CredentialValues } from "src/types/Credentials"; + +export type CredentialStatus = "unconfigured" | "available" | "missing"; + +const CREDENTIALS: Record< + CredentialKind, + { + baseId: string; + settingsKey: keyof Pick; + } +> = { + openai: { + baseId: "podnotes-openai-api-key", + settingsKey: "openAISecretId", + }, + deepgram: { + baseId: "podnotes-deepgram-api-key", + settingsKey: "deepgramSecretId", + }, +}; + +/** + * Narrow boundary around Obsidian's vault-local SecretStorage. + * + * Runtime reads fail closed and never cache secret values. Migration/import + * writes are strict: an ID is returned only after the exact value reads back. + */ +export class CredentialRepository { + constructor(private readonly storage: SecretStorage) {} + + get(settings: IPodNotesSettings, kind: CredentialKind): string | null { + const id = settings[CREDENTIALS[kind].settingsKey].trim(); + if (!id || !isPodNotesSecretId(kind, id)) return null; + + try { + const value = this.storage.getSecret(id); + return value?.trim() ? value.trim() : null; + } catch (error) { + console.error(`PodNotes: failed to read the ${kind} credential`, error); + return null; + } + } + + has(settings: IPodNotesSettings, kind: CredentialKind): boolean { + return this.get(settings, kind) !== null; + } + + status(settings: IPodNotesSettings, kind: CredentialKind): CredentialStatus { + if (!settings[CREDENTIALS[kind].settingsKey].trim()) return "unconfigured"; + return this.has(settings, kind) ? "available" : "missing"; + } + + exportValues( + settings: IPodNotesSettings, + options: { requireConfigured?: boolean } = {}, + ): CredentialValues { + const openAI = this.get(settings, "openai"); + const deepgram = this.get(settings, "deepgram"); + if (options.requireConfigured) { + if (settings.openAISecretId.trim() && !openAI) { + throw new Error( + "The selected OpenAI API key is not available in SecretStorage on this device.", + ); + } + if (settings.deepgramSecretId.trim() && !deepgram) { + throw new Error( + "The selected Deepgram API key is not available in SecretStorage on this device.", + ); + } + } + + return { + ...(openAI ? { openAI } : {}), + ...(deepgram ? { deepgram } : {}), + }; + } + + /** + * Store values under PodNotes-owned IDs. Existing matching values are reused, + * making retries idempotent after a later data.json save failure. A conflicting + * value is never overwritten: the first free numeric suffix is used instead. + */ + storeValues(values: CredentialValues): CredentialReferences { + const references: CredentialReferences = {}; + const openAI = values.openAI?.trim(); + const deepgram = values.deepgram?.trim(); + + if (openAI) references.openAISecretId = this.store("openai", openAI); + if (deepgram) references.deepgramSecretId = this.store("deepgram", deepgram); + + return references; + } + + /** + * Turn an explicit SecretComponent selection into a provider-scoped PodNotes + * reference. Foreign and cross-provider IDs are read only for this user action; + * their value is copied into a collision-safe ID owned by the selected provider. + */ + adoptReference(kind: CredentialKind, selectedId: string): string { + const id = selectedId.trim(); + if (!id) return ""; + if (!isValidSecretId(id)) { + throw new Error("Obsidian returned an invalid SecretStorage ID."); + } + + const value = this.storage.getSecret(id)?.trim(); + if (!value) { + throw new Error("The selected secret is not available on this device."); + } + + return isPodNotesSecretId(kind, id) ? id : this.store(kind, value); + } + + private store(kind: CredentialKind, secret: string): string { + const { baseId } = CREDENTIALS[kind]; + + for (let suffix = 1; suffix <= 10_000; suffix++) { + const id = suffix === 1 ? baseId : `${baseId}-${suffix}`; + const existing = this.storage.getSecret(id); + + if (existing === secret) return id; + if (existing !== null) continue; + + this.storage.setSecret(id, secret); + if (this.storage.getSecret(id) !== secret) { + throw new Error(`SecretStorage did not retain the ${kind} credential.`); + } + return id; + } + + throw new Error(`Could not allocate a SecretStorage ID for the ${kind} credential.`); + } +} diff --git a/src/services/TranscriptionService.test.ts b/src/services/TranscriptionService.test.ts index dd6cddc7..729b157f 100644 --- a/src/services/TranscriptionService.test.ts +++ b/src/services/TranscriptionService.test.ts @@ -9,6 +9,32 @@ import type PodNotes from "src/main"; const getEpisodeAudioBufferMock = vi.fn(); const transcriptionsCreateMock = vi.fn(); +const diarizeWithDeepgramMock = vi.hoisted(() => vi.fn()); + +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + +async function settlesAfterMicrotasks(promise: Promise): Promise { + let settled = false; + void promise.then( + () => { + settled = true; + }, + () => { + settled = true; + }, + ); + + for (let turn = 0; turn < 10 && !settled; turn++) await Promise.resolve(); + return settled; +} vi.mock("../downloadEpisode", () => ({ getEpisodeAudioBuffer: (...args: unknown[]) => getEpisodeAudioBufferMock(...args), @@ -20,6 +46,14 @@ vi.mock("openai", () => ({ }, })); +vi.mock("./diarization", async () => { + const actual = await vi.importActual("./diarization"); + return { + ...actual, + diarizeWithDeepgram: diarizeWithDeepgramMock, + }; +}); + const mockEpisode: Episode = { title: "Test Episode", streamUrl: "https://example.com/episode.mp3", @@ -34,20 +68,21 @@ const mockEpisode: Episode = { function createMockPlugin( overrides: { - openAIApiKey?: string; + openAIKey?: string; podcast?: Episode | null; existingTranscriptPath?: string | null; } = {}, ): PodNotes { const { - openAIApiKey = "test-api-key", + openAIKey = "test-api-key", podcast = mockEpisode, existingTranscriptPath = null, } = overrides; return { settings: { - openAIApiKey, + openAISecretId: openAIKey ? "openai-secret" : "", + deepgramSecretId: "", transcript: { path: "Transcripts/{{podcast}}/{{title}}.md", template: "# {{title}}\n\n{{transcript}}", @@ -56,6 +91,17 @@ function createMockPlugin( path: "Downloads", }, }, + credentials: { + get: vi.fn((_settings, kind: "openai" | "deepgram") => + kind === "openai" ? openAIKey || null : null, + ), + has: vi.fn((_settings, kind: "openai" | "deepgram") => + kind === "openai" ? Boolean(openAIKey) : false, + ), + status: vi.fn((_settings, kind: "openai" | "deepgram") => + kind === "openai" && openAIKey ? "available" : "unconfigured", + ), + }, api: { podcast, }, @@ -83,6 +129,7 @@ function createMockPlugin( describe("TranscriptionService", () => { beforeEach(() => { vi.clearAllMocks(); + diarizeWithDeepgramMock.mockReset(); getEpisodeAudioBufferMock.mockResolvedValue({ buffer: new ArrayBuffer(1024), extension: "mp3", @@ -133,7 +180,7 @@ describe("TranscriptionService", () => { describe("transcribeCurrentEpisode validation", () => { test("shows notice when no API key is configured", async () => { - const mockPlugin = createMockPlugin({ openAIApiKey: "" }); + const mockPlugin = createMockPlugin({ openAIKey: "" }); const service = new TranscriptionService(mockPlugin); await service.transcribeCurrentEpisode(); @@ -147,6 +194,211 @@ describe("TranscriptionService", () => { }); }); + describe("dispose", () => { + test("prevents queued work and credential reads after unload", async () => { + const plugin = createMockPlugin(); + const service = new TranscriptionService(plugin); + (service as unknown as { pendingEpisodes: Episode[] }).pendingEpisodes = [mockEpisode]; + service.dispose(); + + (service as unknown as { drainQueue: () => void }).drainQueue(); + await expect( + (service as unknown as { getClient: () => Promise }).getClient(), + ).rejects.toThrow("unloaded"); + + expect(getEpisodeAudioBufferMock).not.toHaveBeenCalled(); + expect(plugin.credentials.get).not.toHaveBeenCalled(); + }); + + test("cannot recreate a client when unload happens during the dynamic import", async () => { + const plugin = createMockPlugin(); + let resolveModule!: (module: Pick) => void; + const loader = vi.fn( + () => + new Promise>((resolve) => { + resolveModule = resolve; + }), + ); + const constructor = vi.fn(() => ({ audio: { transcriptions: { create: vi.fn() } } })); + const service = new TranscriptionService(plugin, loader); + const pending = ( + service as unknown as { getClient: () => Promise } + ).getClient(); + await vi.waitFor(() => expect(loader).toHaveBeenCalledOnce()); + + service.dispose(); + resolveModule({ OpenAI: constructor as unknown as typeof import("openai").OpenAI }); + + await expect(pending).rejects.toThrow("unloaded"); + expect(constructor).not.toHaveBeenCalled(); + expect( + (service as unknown as { client: unknown; cachedApiKey: unknown }).client, + ).toBeNull(); + expect( + (service as unknown as { client: unknown; cachedApiKey: unknown }).cachedApiKey, + ).toBeNull(); + }); + + test("settles promptly when unloaded during audio acquisition", async () => { + const audioRequest = deferred<{ + buffer: ArrayBuffer; + extension: string; + basename: string; + }>(); + getEpisodeAudioBufferMock.mockReturnValue(audioRequest.promise); + const plugin = createMockPlugin(); + const service = new TranscriptionService(plugin); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const pending = ( + service as unknown as { + transcribeEpisode: (episode: Episode) => Promise; + } + ).transcribeEpisode(mockEpisode); + await vi.waitFor(() => expect(getEpisodeAudioBufferMock).toHaveBeenCalledOnce()); + + service.dispose(); + const settledPromptly = await settlesAfterMicrotasks(pending); + audioRequest.reject(new Error("late audio failure")); + await pending; + + expect(settledPromptly).toBe(true); + expect(plugin.app.vault.create).not.toHaveBeenCalled(); + expect(consoleError).not.toHaveBeenCalled(); + consoleError.mockRestore(); + }); + + test("aborts active OpenAI work without writing a note or updating notices", async () => { + const plugin = createMockPlugin(); + const service = new TranscriptionService(plugin); + let finishRequest!: (result: { text: string }) => void; + let requestSignal: AbortSignal | undefined; + transcriptionsCreateMock.mockImplementation( + ( + _request: unknown, + options?: { signal?: AbortSignal }, + ): Promise<{ text: string }> => + new Promise((resolve, reject) => { + finishRequest = resolve; + requestSignal = options?.signal; + requestSignal?.addEventListener( + "abort", + () => reject(requestSignal?.reason ?? new Error("aborted")), + { once: true }, + ); + }), + ); + const setMessage = vi.spyOn(Notice.prototype, "setMessage"); + try { + const pending = ( + service as unknown as { + transcribeEpisode: (episode: Episode) => Promise; + } + ).transcribeEpisode(mockEpisode); + await vi.waitFor(() => expect(transcriptionsCreateMock).toHaveBeenCalledOnce()); + + const messagesBeforeDispose = setMessage.mock.calls.length; + service.dispose(); + finishRequest({ text: "This must not be saved." }); + await pending; + + expect(requestSignal?.aborted).toBe(true); + expect(plugin.app.vault.create).not.toHaveBeenCalled(); + expect(setMessage).toHaveBeenCalledTimes(messagesBeforeDispose); + } finally { + setMessage.mockRestore(); + } + }); + + test("does not save Deepgram results that finish after unload", async () => { + const plugin = createMockPlugin(); + plugin.settings.transcript.diarization = { + enabled: true, + provider: "deepgram", + speakerTemplate: "**{{speaker}}:** {{text}}", + }; + vi.mocked(plugin.credentials.get).mockImplementation((_settings, kind) => + kind === "deepgram" ? "deepgram-key" : "test-api-key", + ); + let finishRequest!: (segments: Array<{ speaker: string; text: string }>) => void; + diarizeWithDeepgramMock.mockImplementation( + () => + new Promise((resolve) => { + finishRequest = resolve; + }), + ); + const service = new TranscriptionService(plugin); + const pending = ( + service as unknown as { + transcribeEpisode: (episode: Episode) => Promise; + } + ).transcribeEpisode(mockEpisode); + await vi.waitFor(() => expect(diarizeWithDeepgramMock).toHaveBeenCalledOnce()); + + service.dispose(); + finishRequest([{ speaker: "A", text: "This must not be saved." }]); + await pending; + + expect(plugin.app.vault.create).not.toHaveBeenCalled(); + }); + + test("settles promptly when unloaded during a non-cancelable Deepgram request", async () => { + const plugin = createMockPlugin(); + plugin.settings.transcript.diarization = { + enabled: true, + provider: "deepgram", + speakerTemplate: "**{{speaker}}:** {{text}}", + }; + vi.mocked(plugin.credentials.get).mockImplementation((_settings, kind) => + kind === "deepgram" ? "deepgram-key" : "test-api-key", + ); + const request = deferred>(); + diarizeWithDeepgramMock.mockReturnValue(request.promise); + const service = new TranscriptionService(plugin); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const pending = ( + service as unknown as { + transcribeEpisode: (episode: Episode) => Promise; + } + ).transcribeEpisode(mockEpisode); + await vi.waitFor(() => expect(diarizeWithDeepgramMock).toHaveBeenCalledOnce()); + + service.dispose(); + const settledPromptly = await settlesAfterMicrotasks(pending); + request.reject(new Error("late Deepgram failure")); + await pending; + + expect(settledPromptly).toBe(true); + expect(plugin.app.vault.create).not.toHaveBeenCalled(); + expect(consoleError).not.toHaveBeenCalled(); + consoleError.mockRestore(); + }); + + test("cancels a completed notice hide timer when disposed", async () => { + vi.useFakeTimers(); + const hide = vi.spyOn(Notice.prototype, "hide"); + try { + transcriptionsCreateMock.mockResolvedValue({ text: "Completed transcript." }); + const plugin = createMockPlugin(); + const service = new TranscriptionService(plugin); + await ( + service as unknown as { + transcribeEpisode: (episode: Episode) => Promise; + } + ).transcribeEpisode(mockEpisode); + + expect(hide).not.toHaveBeenCalled(); + service.dispose(); + expect(hide).toHaveBeenCalledOnce(); + + await vi.advanceTimersByTimeAsync(5000); + expect(hide).toHaveBeenCalledOnce(); + } finally { + hide.mockRestore(); + vi.useRealTimers(); + } + }); + }); + describe("empty Whisper transcript (TR-01)", () => { // transcribeEpisode is private; drive it directly so the empty-body throw is // observed as "no file written" (the throw is caught and surfaced as a diff --git a/src/services/TranscriptionService.ts b/src/services/TranscriptionService.ts index b6efd2cd..f2022a7a 100644 --- a/src/services/TranscriptionService.ts +++ b/src/services/TranscriptionService.ts @@ -21,6 +21,9 @@ function TimerNotice(heading: string, initialMessage: string) { let currentMessage = initialMessage; const startTime = Date.now(); let stopTime: number; + let interval: number | null = null; + let hideTimeout: number | null = null; + let disposed = false; const notice = new Notice(initialMessage, 0); function formatMsg(message: string): string { @@ -28,11 +31,12 @@ function TimerNotice(heading: string, initialMessage: string) { } function update(message: string) { + if (disposed) return; currentMessage = message; notice.setMessage(formatMsg(currentMessage)); } - const interval = window.setInterval(() => { + interval = window.setInterval(() => { notice.setMessage(formatMsg(currentMessage)); }, 1000); @@ -40,13 +44,41 @@ function TimerNotice(heading: string, initialMessage: string) { return formatTime(stopTime ? stopTime - startTime : Date.now() - startTime); } + function stop() { + if (interval === null) return; + stopTime = Date.now(); + window.clearInterval(interval); + interval = null; + } + + function scheduleHide(delayMs: number, onHide: () => void) { + if (disposed) return; + if (hideTimeout !== null) window.clearTimeout(hideTimeout); + hideTimeout = window.setTimeout(() => { + hideTimeout = null; + if (disposed) return; + disposed = true; + notice.hide(); + onHide(); + }, delayMs); + } + + function dispose() { + if (disposed) return; + disposed = true; + stop(); + if (hideTimeout !== null) { + window.clearTimeout(hideTimeout); + hideTimeout = null; + } + notice.hide(); + } + return { update, - hide: () => notice.hide(), - stop: () => { - stopTime = Date.now(); - window.clearInterval(interval); - }, + stop, + scheduleHide, + dispose, }; } @@ -69,24 +101,41 @@ export class TranscriptionService { private plugin: PodNotes; private client: OpenAI | null = null; private cachedApiKey: string | null = null; + private disposed = false; + private readonly lifetimeAbortController = new AbortController(); + private readonly activeNotices = new Set>(); private MAX_RETRIES = 3; private readonly MAX_CONCURRENT_TRANSCRIPTIONS = 2; private readonly MAX_CONCURRENT_CHUNK_TRANSCRIPTIONS = 3; private pendingEpisodes: Episode[] = []; private activeTranscriptions = new Set(); - constructor(plugin: PodNotes) { + constructor( + plugin: PodNotes, + private readonly loadOpenAI: () => Promise> = () => + import("openai"), + ) { this.plugin = plugin; } async transcribeCurrentEpisode(): Promise { - if (!requiredTranscriptionKeyPresent(this.plugin.settings)) { + if (this.disposed) return; + if ( + !requiredTranscriptionKeyPresent(this.plugin.settings, (kind) => + this.plugin.credentials.has(this.plugin.settings, kind), + ) + ) { const diarization = this.plugin.settings.transcript.diarization; const needsDeepgram = diarization?.enabled && diarization.provider === "deepgram"; + const kind = needsDeepgram ? "deepgram" : "openai"; + const unavailableOnDevice = + this.plugin.credentials.status(this.plugin.settings, kind) === "missing"; new Notice( - needsDeepgram - ? "Please add your Deepgram API key in the transcript settings to use Deepgram diarization." - : "Please add your OpenAI API key in the transcript settings first.", + unavailableOnDevice + ? `The selected ${needsDeepgram ? "Deepgram" : "OpenAI"} API key is not available on this device. Select or create it in the transcript settings.` + : needsDeepgram + ? "Select or create a Deepgram API key in the transcript settings on this device." + : "Select or create an OpenAI API key in the transcript settings on this device.", ); return; } @@ -122,6 +171,10 @@ export class TranscriptionService { } private drainQueue(): void { + if (this.disposed) { + this.pendingEpisodes = []; + return; + } while ( this.activeTranscriptions.size < this.MAX_CONCURRENT_TRANSCRIPTIONS && this.pendingEpisodes.length > 0 @@ -136,7 +189,7 @@ export class TranscriptionService { void this.transcribeEpisode(nextEpisode).finally(() => { this.activeTranscriptions.delete(episodeKey); - this.drainQueue(); + if (!this.disposed) this.drainQueue(); }); } } @@ -146,23 +199,29 @@ export class TranscriptionService { } private async transcribeEpisode(episode: Episode): Promise { + this.assertActive(); const notice = TimerNotice(`Transcription: ${episode.title}`, "Preparing to transcribe..."); + this.activeNotices.add(notice); + const updateNotice = (message: string) => { + if (!this.disposed) notice.update(message); + }; try { const transcriptPath = this.getTranscriptPath(episode); const existingFile = this.plugin.app.vault.getAbstractFileByPath(transcriptPath); if (existingFile instanceof TFile) { notice.stop(); - notice.update(`Transcript already exists - skipped (${transcriptPath}).`); + updateNotice(`Transcript already exists - skipped (${transcriptPath}).`); return; } - notice.update("Fetching episode audio..."); + updateNotice("Fetching episode audio..."); const { buffer: fileBuffer, extension: fileExtension, basename, - } = await getEpisodeAudioBuffer(episode); + } = await this.waitForLifecycle(getEpisodeAudioBuffer(episode)); + this.assertActive(); const mimeType = getMimeType(fileExtension); const { body: transcriptBody, warning } = await this.buildTranscriptBody( @@ -172,22 +231,30 @@ export class TranscriptionService { extension: fileExtension, basename, }, - notice.update, + updateNotice, ); + this.assertActive(); - notice.update("Saving transcription..."); + updateNotice("Saving transcription..."); await this.saveTranscription(episode, transcriptBody); + this.assertActive(); notice.stop(); - notice.update(warning ?? "Transcription completed and saved."); + updateNotice(warning ?? "Transcription completed and saved."); } catch (error) { + if (this.disposed || this.lifetimeAbortController.signal.aborted) return; console.error("Transcription error:", error); const message = error instanceof Error ? error.message : String(error); notice.stop(); - notice.update(`Transcription failed: ${message}`); + updateNotice(`Transcription failed: ${message}`); } finally { notice.stop(); - window.setTimeout(() => notice.hide(), 5000); + if (this.disposed) { + notice.dispose(); + this.activeNotices.delete(notice); + } else { + notice.scheduleHide(5000, () => this.activeNotices.delete(notice)); + } } } @@ -216,6 +283,7 @@ export class TranscriptionService { if (diarization?.enabled) { const segments = await this.diarize(audio, diarization.provider, updateNotice); + this.assertActive(); if (segments.length === 0) { throw new Error("Diarization returned no speech segments."); } @@ -226,8 +294,10 @@ export class TranscriptionService { updateNotice("Creating audio chunks..."); const files = await createChunkFiles(audio); + this.assertActive(); updateNotice("Starting transcription..."); const { text, failedChunks } = await this.transcribeChunks(files, updateNotice); + this.assertActive(); // Strip the error placeholders (and trim) to see whether ANY real speech was // transcribed. Nothing real means every chunk failed or the only successes @@ -259,19 +329,25 @@ export class TranscriptionService { provider: DiarizationProviderId, updateNotice: (message: string) => void, ): Promise { + this.assertActive(); if (provider === "deepgram") { - const apiKey = this.plugin.settings.diarizationApiKey?.trim(); + const apiKey = this.plugin.credentials.get(this.plugin.settings, "deepgram"); if (!apiKey) { - throw new Error("Missing Deepgram API key for diarization."); + throw new Error("Missing Deepgram API key on this device."); } // Deepgram ingests the whole file in one request, so it needs no // chunking — which is exactly why its speaker labels stay consistent // across the entire episode. - return diarizeWithDeepgram({ - audio, - apiKey, - onProgress: updateNotice, - }); + const segments = await this.waitForLifecycle( + diarizeWithDeepgram({ + audio, + apiKey, + onProgress: updateNotice, + signal: this.lifetimeAbortController.signal, + }), + ); + this.assertActive(); + return segments; } // OpenAI diarization shares Whisper's ~20 MB chunk limit (a conservative @@ -279,12 +355,16 @@ export class TranscriptionService { // Speaker labels can differ across chunks on a long episode. updateNotice("Creating audio chunks..."); const chunkFiles = await createChunkFiles(audio); - return diarizeWithOpenAI({ + this.assertActive(); + const segments = await diarizeWithOpenAI({ getClient: () => this.getClient(), chunkFiles, maxRetries: this.MAX_RETRIES, onProgress: updateNotice, + signal: this.lifetimeAbortController.signal, }); + this.assertActive(); + return segments; } private async transcribeChunks( @@ -292,6 +372,7 @@ export class TranscriptionService { updateNotice: (message: string) => void, ): Promise<{ text: string; failedChunks: number }> { const client = await this.getClient(); + this.assertActive(); const transcriptions: string[] = Array.from({ length: files.length }); let completedChunks = 0; let failedChunks = 0; @@ -308,22 +389,29 @@ export class TranscriptionService { const worker = async () => { while (true) { + this.assertActive(); const index = nextIndex++; if (index >= files.length) return; const file = files[index]; let retries = 0; while (retries < this.MAX_RETRIES) { + this.assertActive(); try { - const result = await client.audio.transcriptions.create({ - model: "whisper-1", - file, - }); + const result = await client.audio.transcriptions.create( + { + model: "whisper-1", + file, + }, + { signal: this.lifetimeAbortController.signal }, + ); + this.assertActive(); transcriptions[index] = result.text; completedChunks++; updateProgress(); break; } catch (error) { + this.assertActive(); retries++; if (retries >= this.MAX_RETRIES) { console.error( @@ -335,9 +423,7 @@ export class TranscriptionService { completedChunks++; updateProgress(); } else { - await new Promise((resolve) => - window.setTimeout(resolve, 1000 * retries), - ); + await this.waitForRetry(1000 * retries); } } } @@ -348,10 +434,72 @@ export class TranscriptionService { const workers = Array.from({ length: workerCount }, () => worker()); await Promise.all(workers); + this.assertActive(); return { text: transcriptions.join(" "), failedChunks }; } + private waitForRetry(delayMs: number): Promise { + this.assertActive(); + const signal = this.lifetimeAbortController.signal; + + return new Promise((resolve, reject) => { + const timeout = window.setTimeout(() => { + signal.removeEventListener("abort", onAbort); + resolve(); + }, delayMs); + const onAbort = () => { + window.clearTimeout(timeout); + signal.removeEventListener("abort", onAbort); + reject(this.getAbortReason()); + }; + + signal.addEventListener("abort", onAbort, { once: true }); + if (signal.aborted) onAbort(); + }); + } + + private waitForLifecycle(operation: Promise): Promise { + this.assertActive(); + const signal = this.lifetimeAbortController.signal; + + return new Promise((resolve, reject) => { + let settled = false; + const cleanup = () => signal.removeEventListener("abort", onAbort); + const onAbort = () => { + if (settled) return; + settled = true; + cleanup(); + reject(this.getAbortReason()); + }; + + signal.addEventListener("abort", onAbort, { once: true }); + void operation.then( + (value) => { + if (settled) return; + if (signal.aborted) { + onAbort(); + return; + } + settled = true; + cleanup(); + resolve(value); + }, + (error) => { + if (settled) return; + if (signal.aborted) { + onAbort(); + return; + } + settled = true; + cleanup(); + reject(error); + }, + ); + if (signal.aborted) onAbort(); + }); + } + /** * The on-disk path of an episode's transcript note, capped so a long title * can't trip ENAMETOOLONG (#22). Used for the existence checks and the write @@ -364,6 +512,7 @@ export class TranscriptionService { } private async saveTranscription(episode: Episode, transcriptBody: string): Promise { + this.assertActive(); const transcriptPath = this.getTranscriptPath(episode); // transcriptBody is already formatted by buildTranscriptBody (sentence // reflow for Whisper, speaker turns for diarization), so it is templated @@ -381,15 +530,19 @@ export class TranscriptionService { // existing folder, which the old hand-rolled loop surfaced as a spurious // failure (the #87 class of bug). const directory = transcriptPath.substring(0, transcriptPath.lastIndexOf("/")); - await ensureFolderExists(directory, vault); + await ensureFolderExists(directory, vault, () => this.assertActive()); + this.assertActive(); const file = vault.getAbstractFileByPath(transcriptPath); if (!file) { + this.assertActive(); const newFile = await vault.create(transcriptPath, transcriptContent); + this.assertActive(); await this.plugin.app.workspace.getLeaf().openFile(newFile); } else if (file instanceof TFile) { // File already exists - open it without overwriting + this.assertActive(); await this.plugin.app.workspace.getLeaf().openFile(file); } else { throw new Error("Expected a file but found a folder at transcript path."); @@ -397,16 +550,18 @@ export class TranscriptionService { } private async getClient(): Promise { - const apiKey = this.plugin.settings.openAIApiKey?.trim(); + this.assertActive(); + const apiKey = this.plugin.credentials.get(this.plugin.settings, "openai"); if (!apiKey) { - throw new Error("Missing OpenAI API key"); + throw new Error("Missing OpenAI API key on this device"); } if (this.client && this.cachedApiKey === apiKey) { return this.client; } - const { OpenAI } = await import("openai"); + const { OpenAI } = await this.loadOpenAI(); + this.assertActive(); this.client = new OpenAI({ apiKey, dangerouslyAllowBrowser: true, @@ -415,4 +570,37 @@ export class TranscriptionService { return this.client; } + + /** Drop the client and its credential material when the plugin unloads. */ + dispose(): void { + if (this.disposed) return; + this.disposed = true; + this.lifetimeAbortController.abort( + new DOMException("PodNotes was unloaded during transcription.", "AbortError"), + ); + this.pendingEpisodes = []; + for (const notice of this.activeNotices) { + notice.dispose(); + } + this.activeNotices.clear(); + this.clearCredentialCache(); + } + + clearCredentialCache(): void { + this.client = null; + this.cachedApiKey = null; + } + + private assertActive(): void { + if (this.disposed || this.lifetimeAbortController.signal.aborted) { + throw this.getAbortReason(); + } + } + + private getAbortReason(): unknown { + return ( + this.lifetimeAbortController.signal.reason ?? + new DOMException("PodNotes was unloaded during transcription.", "AbortError") + ); + } } diff --git a/src/services/diarization/deepgramProvider.test.ts b/src/services/diarization/deepgramProvider.test.ts index 6e3ccd03..06bcc843 100644 --- a/src/services/diarization/deepgramProvider.test.ts +++ b/src/services/diarization/deepgramProvider.test.ts @@ -20,6 +20,29 @@ function stubResponse(overrides: Partial<{ status: number; json: unknown; text: } as unknown as Awaited>; } +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +async function settlesAfterMicrotasks(promise: Promise): Promise { + let settled = false; + void promise.then( + () => { + settled = true; + }, + () => { + settled = true; + }, + ); + + for (let turn = 0; turn < 10 && !settled; turn++) await Promise.resolve(); + return settled; +} + describe("diarizeWithDeepgram (#168)", () => { it("posts the whole audio buffer with token auth and parses the response", async () => { const request = vi.fn().mockResolvedValue( @@ -72,4 +95,37 @@ describe("diarizeWithDeepgram (#168)", () => { }), ).rejects.toThrow(/HTTP 401.*Invalid credentials/); }); + + it("aborts promptly and ignores a late HTTP failure from requestUrl", async () => { + const controller = new AbortController(); + const abortError = new DOMException("plugin unloaded", "AbortError"); + const response = deferred>>(); + const request = vi.fn().mockReturnValue(response.promise); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const pending = diarizeWithDeepgram({ + audio, + apiKey: "bad", + onProgress: () => {}, + request, + signal: controller.signal, + }); + const outcome = pending.then( + () => undefined, + (error: unknown) => error, + ); + await vi.waitFor(() => expect(request).toHaveBeenCalledOnce()); + + try { + controller.abort(abortError); + const settledPromptly = await settlesAfterMicrotasks(outcome); + response.resolve(stubResponse({ status: 401, json: { err_msg: "late failure" } })); + const result = await outcome; + + expect(settledPromptly).toBe(true); + expect(result).toBe(abortError); + expect(consoleError).not.toHaveBeenCalled(); + } finally { + consoleError.mockRestore(); + } + }); }); diff --git a/src/services/diarization/deepgramProvider.ts b/src/services/diarization/deepgramProvider.ts index 4234cd16..e7cf5dee 100644 --- a/src/services/diarization/deepgramProvider.ts +++ b/src/services/diarization/deepgramProvider.ts @@ -36,13 +36,15 @@ export async function diarizeWithDeepgram(opts: { apiKey: string; onProgress: (message: string) => void; request?: RequestUrlFn; + signal?: AbortSignal; }): Promise { - const { audio, apiKey, onProgress } = opts; + const { audio, apiKey, onProgress, signal } = opts; const request = opts.request ?? (requestUrl as unknown as RequestUrlFn); + if (signal) throwIfAborted(signal); onProgress("Diarizing with Deepgram..."); - const response = await request({ + const requestPromise = request({ url: `${DEEPGRAM_LISTEN_URL}?${DEEPGRAM_QUERY}`, method: "POST", headers: { Authorization: `Token ${apiKey}` }, @@ -50,6 +52,7 @@ export async function diarizeWithDeepgram(opts: { body: audio.buffer, throw: false, }); + const response = signal ? await waitForAbort(requestPromise, signal) : await requestPromise; if (response.status < 200 || response.status >= 300) { const detail = extractDeepgramError(response); @@ -62,6 +65,54 @@ export async function diarizeWithDeepgram(opts: { return parseDeepgramSegments(response.json); } +function throwIfAborted(signal: AbortSignal): void { + if (!signal.aborted) return; + throw signal.reason ?? new DOMException("Deepgram diarization was aborted.", "AbortError"); +} + +function waitForAbort(operation: Promise, signal: AbortSignal): Promise { + throwIfAborted(signal); + + return new Promise((resolve, reject) => { + let settled = false; + const cleanup = () => signal.removeEventListener("abort", onAbort); + const onAbort = () => { + if (settled) return; + settled = true; + cleanup(); + reject( + signal.reason ?? + new DOMException("Deepgram diarization was aborted.", "AbortError"), + ); + }; + + signal.addEventListener("abort", onAbort, { once: true }); + void operation.then( + (value) => { + if (settled) return; + if (signal.aborted) { + onAbort(); + return; + } + settled = true; + cleanup(); + resolve(value); + }, + (error) => { + if (settled) return; + if (signal.aborted) { + onAbort(); + return; + } + settled = true; + cleanup(); + reject(error); + }, + ); + if (signal.aborted) onAbort(); + }); +} + /** Pull a human-readable message out of Deepgram's error body, if any. */ function extractDeepgramError(response: RequestUrlResponse): string { try { diff --git a/src/services/diarization/index.ts b/src/services/diarization/index.ts index 80f62afb..a09453d0 100644 --- a/src/services/diarization/index.ts +++ b/src/services/diarization/index.ts @@ -1,4 +1,5 @@ import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; +import type { CredentialKind } from "src/types/Credentials"; export * from "./types"; export * from "./segments"; @@ -8,16 +9,19 @@ export { diarizeWithDeepgram, type RequestUrlFn } from "./deepgramProvider"; /** * Whether the credentials the active transcription mode needs are present. * - * Diarization via Deepgram needs the dedicated `diarizationApiKey`; every other - * mode (plain Whisper, or OpenAI diarization) reuses `openAIApiKey`. Used to gate + * Diarization via Deepgram needs the dedicated Deepgram credential; every other + * mode (plain Whisper, or OpenAI diarization) reuses the OpenAI credential. Used to gate * the transcribe command and guard the service so a user can't kick off a run * that is certain to fail for a missing key. Pure so it is unit-testable and * usable from both the command callback and the service. */ -export function requiredTranscriptionKeyPresent(settings: IPodNotesSettings): boolean { +export function requiredTranscriptionKeyPresent( + settings: IPodNotesSettings, + hasCredential: (kind: CredentialKind) => boolean, +): boolean { const diarization = settings.transcript?.diarization; if (diarization?.enabled && diarization.provider === "deepgram") { - return Boolean(settings.diarizationApiKey?.trim()); + return hasCredential("deepgram"); } - return Boolean(settings.openAIApiKey?.trim()); + return hasCredential("openai"); } diff --git a/src/services/diarization/openaiProvider.test.ts b/src/services/diarization/openaiProvider.test.ts index f6f36a5f..25586e5f 100644 --- a/src/services/diarization/openaiProvider.test.ts +++ b/src/services/diarization/openaiProvider.test.ts @@ -26,6 +26,7 @@ describe("diarizeWithOpenAI (#168)", () => { chunkFiles: [chunk("a.mp3"), chunk("b.mp3")], maxRetries: 2, onProgress: () => {}, + signal: new AbortController().signal, }); expect(segments).toEqual([ @@ -45,6 +46,7 @@ describe("diarizeWithOpenAI (#168)", () => { chunkFiles: [chunk("a.mp3"), chunk("b.mp3")], maxRetries: 1, onProgress: () => {}, + signal: new AbortController().signal, }); expect(segments).toEqual([ @@ -62,7 +64,44 @@ describe("diarizeWithOpenAI (#168)", () => { chunkFiles: [chunk("a.mp3"), chunk("b.mp3")], maxRetries: 1, onProgress: () => {}, + signal: new AbortController().signal, }), ).rejects.toThrow(/every chunk: invalid api key/); }); + + it("aborts an in-flight request without retrying or logging a failure", async () => { + const controller = new AbortController(); + const abortError = new DOMException("plugin unloaded", "AbortError"); + const create = vi.fn( + (_request: unknown, options?: { signal?: AbortSignal }): Promise => + new Promise((_resolve, reject) => { + options?.signal?.addEventListener( + "abort", + () => reject(options.signal?.reason ?? new Error("aborted")), + { once: true }, + ); + }), + ); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + + try { + const pending = diarizeWithOpenAI({ + getClient: fakeClient(create), + chunkFiles: [chunk("a.mp3")], + maxRetries: 3, + onProgress: () => {}, + signal: controller.signal, + }); + await vi.waitFor(() => expect(create).toHaveBeenCalledOnce()); + + controller.abort(abortError); + + await expect(pending).rejects.toBe(abortError); + expect(create).toHaveBeenCalledTimes(1); + expect(create.mock.calls[0]?.[1]).toEqual({ signal: controller.signal }); + expect(consoleError).not.toHaveBeenCalled(); + } finally { + consoleError.mockRestore(); + } + }); }); diff --git a/src/services/diarization/openaiProvider.ts b/src/services/diarization/openaiProvider.ts index d96df3a6..f6c7ed1f 100644 --- a/src/services/diarization/openaiProvider.ts +++ b/src/services/diarization/openaiProvider.ts @@ -25,29 +25,38 @@ export async function diarizeWithOpenAI(opts: { chunkFiles: File[]; maxRetries: number; onProgress: (message: string) => void; + signal: AbortSignal; }): Promise { - const { getClient, chunkFiles, maxRetries, onProgress } = opts; + const { getClient, chunkFiles, maxRetries, onProgress, signal } = opts; + throwIfAborted(signal); const client = await getClient(); + throwIfAborted(signal); const segments: DiarizedSegment[] = []; let failedChunks = 0; let lastError: unknown; for (let index = 0; index < chunkFiles.length; index++) { + throwIfAborted(signal); onProgress(`Diarizing with OpenAI... chunk ${index + 1}/${chunkFiles.length}`); const file = chunkFiles[index]; let attempt = 0; while (true) { + throwIfAborted(signal); try { - const result = await client.audio.transcriptions.create({ - model: OPENAI_DIARIZE_MODEL, - file, - // The SDK types `response_format`/`chunking_strategy` for this model, - // but the create() overload returns a union; parse from the raw - // payload so we never depend on which arm TS narrows to. - response_format: "diarized_json", - chunking_strategy: "auto", - }); + const result = await client.audio.transcriptions.create( + { + model: OPENAI_DIARIZE_MODEL, + file, + // The SDK types `response_format`/`chunking_strategy` for this model, + // but the create() overload returns a union; parse from the raw + // payload so we never depend on which arm TS narrows to. + response_format: "diarized_json", + chunking_strategy: "auto", + }, + { signal }, + ); + throwIfAborted(signal); // Each chunk's start/end are relative to that chunk, so the // concatenated segments' timestamps are not episode-absolute. The // rendered transcript does not surface timestamps today; if it ever @@ -55,6 +64,7 @@ export async function diarizeWithOpenAI(opts: { segments.push(...parseOpenAIDiarizedSegments(result)); break; } catch (error) { + throwIfAborted(signal); attempt++; if (attempt >= maxRetries) { console.error( @@ -69,7 +79,7 @@ export async function diarizeWithOpenAI(opts: { }); break; } - await new Promise((resolve) => window.setTimeout(resolve, 1000 * attempt)); + await waitForRetry(1000 * attempt, signal); } } } @@ -81,3 +91,29 @@ export async function diarizeWithOpenAI(opts: { return segments; } + +function throwIfAborted(signal: AbortSignal): void { + if (!signal.aborted) return; + throw signal.reason ?? new DOMException("OpenAI diarization was aborted.", "AbortError"); +} + +function waitForRetry(delayMs: number, signal: AbortSignal): Promise { + throwIfAborted(signal); + + return new Promise((resolve, reject) => { + const timeout = window.setTimeout(() => { + signal.removeEventListener("abort", onAbort); + resolve(); + }, delayMs); + const onAbort = () => { + window.clearTimeout(timeout); + signal.removeEventListener("abort", onAbort); + reject( + signal.reason ?? new DOMException("OpenAI diarization was aborted.", "AbortError"), + ); + }; + + signal.addEventListener("abort", onAbort, { once: true }); + if (signal.aborted) onAbort(); + }); +} diff --git a/src/services/diarization/requiredTranscriptionKeyPresent.test.ts b/src/services/diarization/requiredTranscriptionKeyPresent.test.ts index 40eea65e..b1dd5f7e 100644 --- a/src/services/diarization/requiredTranscriptionKeyPresent.test.ts +++ b/src/services/diarization/requiredTranscriptionKeyPresent.test.ts @@ -7,71 +7,54 @@ function settings(overrides: Partial = {}): IPodNotesSettings return structuredClone({ ...DEFAULT_SETTINGS, ...overrides }); } -function withDiarization( - provider: "openai" | "deepgram", - enabled: boolean, - keys: { openAIApiKey?: string; diarizationApiKey?: string } = {}, -): IPodNotesSettings { - const s = settings(keys); +function withDiarization(provider: "openai" | "deepgram", enabled: boolean): IPodNotesSettings { + const s = settings(); s.transcript.diarization.enabled = enabled; s.transcript.diarization.provider = provider; return s; } +const has = + (...kinds: Array<"openai" | "deepgram">) => + (kind: "openai" | "deepgram") => + kinds.includes(kind); + describe("requiredTranscriptionKeyPresent (#168)", () => { it("requires the OpenAI key when diarization is off", () => { - expect(requiredTranscriptionKeyPresent(settings({ openAIApiKey: "sk" }))).toBe(true); - expect(requiredTranscriptionKeyPresent(settings({ openAIApiKey: "" }))).toBe(false); + expect(requiredTranscriptionKeyPresent(settings(), has("openai"))).toBe(true); + expect(requiredTranscriptionKeyPresent(settings(), has())).toBe(false); }); it("requires the OpenAI key for the OpenAI diarization provider", () => { expect( - requiredTranscriptionKeyPresent( - withDiarization("openai", true, { openAIApiKey: "sk" }), - ), + requiredTranscriptionKeyPresent(withDiarization("openai", true), has("openai")), ).toBe(true); expect( - requiredTranscriptionKeyPresent( - withDiarization("openai", true, { - openAIApiKey: "", - diarizationApiKey: "dg", - }), - ), + requiredTranscriptionKeyPresent(withDiarization("openai", true), has("deepgram")), ).toBe(false); }); it("requires the Deepgram key for the Deepgram diarization provider", () => { expect( - requiredTranscriptionKeyPresent( - withDiarization("deepgram", true, { diarizationApiKey: "dg" }), - ), + requiredTranscriptionKeyPresent(withDiarization("deepgram", true), has("deepgram")), ).toBe(true); // An OpenAI key does not satisfy the Deepgram provider. expect( - requiredTranscriptionKeyPresent( - withDiarization("deepgram", true, { - openAIApiKey: "sk", - diarizationApiKey: "", - }), - ), + requiredTranscriptionKeyPresent(withDiarization("deepgram", true), has("openai")), ).toBe(false); }); it("falls back to the OpenAI key when Deepgram is selected but diarization is off", () => { // provider=deepgram only matters when diarization is enabled. expect( - requiredTranscriptionKeyPresent( - withDiarization("deepgram", false, { openAIApiKey: "sk" }), - ), + requiredTranscriptionKeyPresent(withDiarization("deepgram", false), has("openai")), ).toBe(true); }); - it("treats whitespace-only keys as absent", () => { - expect(requiredTranscriptionKeyPresent(settings({ openAIApiKey: " " }))).toBe(false); - expect( - requiredTranscriptionKeyPresent( - withDiarization("deepgram", true, { diarizationApiKey: " " }), - ), - ).toBe(false); + it("treats dangling SecretStorage references as absent", () => { + expect(requiredTranscriptionKeyPresent(settings(), has())).toBe(false); + expect(requiredTranscriptionKeyPresent(withDiarization("deepgram", true), has())).toBe( + false, + ); }); }); diff --git a/src/settingsTransfer.test.ts b/src/settingsTransfer.test.ts index 92b37c79..7458fe34 100644 --- a/src/settingsTransfer.test.ts +++ b/src/settingsTransfer.test.ts @@ -19,7 +19,7 @@ const NOW = "2026-06-14T00:00:00.000Z"; describe("serializeSettings", () => { it("wraps settings in a versioned envelope", () => { - const envelope = serializeSettings(makeSettings(), { includeSecret: false }, "2.16.0", NOW); + const envelope = serializeSettings(makeSettings(), {}, "2.16.0", NOW); expect(envelope.type).toBe(SETTINGS_EXPORT_TYPE); expect(envelope.version).toBe(SETTINGS_EXPORT_VERSION); @@ -37,52 +37,55 @@ describe("serializeSettings", () => { currentEpisode: { title: "ep" } as never, }); - const { settings: exported } = serializeSettings( - settings, - { includeSecret: false }, - "2.16.0", - NOW, - ); + const { settings: exported } = serializeSettings(settings, {}, "2.16.0", NOW); for (const key of EXCLUDED_KEYS) { expect(exported).not.toHaveProperty(key as string); } }); - it("omits the API key unless includeSecret is set", () => { - const settings = makeSettings({ openAIApiKey: "sk-secret" }); + it("keeps SecretStorage references out and puts opted-in values in a separate payload", () => { + const settings = makeSettings({ openAISecretId: "podnotes-openai-api-key" }); - const without = serializeSettings(settings, { includeSecret: false }, "2.16.0", NOW); - expect(without.settings).not.toHaveProperty("openAIApiKey"); + const ordinary = serializeSettings(settings, {}, "2.16.0", NOW); + expect(ordinary.settings).not.toHaveProperty("openAISecretId"); + expect(ordinary).not.toHaveProperty("secrets"); - const withKey = serializeSettings(settings, { includeSecret: true }, "2.16.0", NOW); - expect(withKey.settings.openAIApiKey).toBe("sk-secret"); + const withKey = serializeSettings( + settings, + { secrets: { openAI: "sk-secret" } }, + "2.16.0", + NOW, + ); + expect(withKey.settings).not.toHaveProperty("openAISecretId"); + expect(withKey.secrets).toEqual({ openAI: "sk-secret" }); }); - it("redacts the diarization (Deepgram) key alongside the OpenAI key (#168)", () => { + it("keeps both provider values in the explicit top-level payload (#168)", () => { const settings = makeSettings({ - openAIApiKey: "sk-secret", - diarizationApiKey: "dg-secret", + openAISecretId: "podnotes-openai-api-key", + deepgramSecretId: "podnotes-deepgram-api-key", }); - const without = serializeSettings(settings, { includeSecret: false }, "2.16.0", NOW); - expect(without.settings).not.toHaveProperty("openAIApiKey"); - expect(without.settings).not.toHaveProperty("diarizationApiKey"); + const without = serializeSettings(settings, {}, "2.16.0", NOW); + expect(without.settings).not.toHaveProperty("openAISecretId"); + expect(without.settings).not.toHaveProperty("deepgramSecretId"); - const withKey = serializeSettings(settings, { includeSecret: true }, "2.16.0", NOW); - expect(withKey.settings.diarizationApiKey).toBe("dg-secret"); + const withKeys = serializeSettings( + settings, + { secrets: { openAI: "sk-secret", deepgram: "dg-secret" } }, + "2.16.0", + NOW, + ); + expect(withKeys.secrets).toEqual({ openAI: "sk-secret", deepgram: "dg-secret" }); + expect(JSON.stringify(withKeys.settings)).not.toContain("secret"); }); it("never lets the Deepgram key ride along inside the transcript object (#168)", () => { // The key lives top-level precisely so it can be redacted; the nested // transcript object (copied wholesale) must never carry a secret. - const settings = makeSettings({ diarizationApiKey: "dg-secret" }); - const { settings: exported } = serializeSettings( - settings, - { includeSecret: false }, - "2.16.0", - NOW, - ); + const settings = makeSettings({ deepgramSecretId: "podnotes-deepgram-api-key" }); + const { settings: exported } = serializeSettings(settings, {}, "2.16.0", NOW); expect(JSON.stringify(exported.transcript ?? {})).not.toContain("dg-secret"); }); @@ -94,12 +97,7 @@ describe("serializeSettings", () => { }, }); - const { settings: exported } = serializeSettings( - settings, - { includeSecret: false }, - "2.16.0", - NOW, - ); + const { settings: exported } = serializeSettings(settings, {}, "2.16.0", NOW); expect(exported.note).toEqual(settings.note); expect(exported.savedFeeds).toEqual(settings.savedFeeds); @@ -125,7 +123,7 @@ describe("serializeSettings", () => { }, }); - const exported = serializeSettings(settings, { includeSecret: false }, "2.17.3", NOW); + const exported = serializeSettings(settings, {}, "2.17.3", NOW); expect(exported.settings.queue?.episodes[0].episodeDate).toBe(date.toISOString()); }); }); @@ -172,7 +170,7 @@ describe("parseImport", () => { defaultPlaybackRate: 1.5, note: { path: "p", template: "t" }, }); - const envelope = serializeSettings(settings, { includeSecret: false }, "2.16.0", NOW); + const envelope = serializeSettings(settings, {}, "2.16.0", NOW); const result = parseImport(JSON.stringify(envelope)); expect(result.ok).toBe(true); @@ -215,9 +213,9 @@ describe("parseImport", () => { }); it("rejects a raw data file from a newer persistence schema", () => { - const result = parseImport(JSON.stringify({ schemaVersion: 2, defaultVolume: 0.5 })); + const result = parseImport(JSON.stringify({ schemaVersion: 3, defaultVolume: 0.5 })); expect(result).toEqual( - expect.objectContaining({ ok: false, error: expect.stringContaining("schema v2") }), + expect.objectContaining({ ok: false, error: expect.stringContaining("schema v3") }), ); }); @@ -298,11 +296,63 @@ describe("parseImport", () => { const result = parseImport(JSON.stringify({ openAIApiKey: "sk-real" })); expect(result.ok).toBe(true); if (result.ok) { - expect(result.settings.openAIApiKey).toBe("sk-real"); + expect(result.settings).not.toHaveProperty("openAIApiKey"); + expect(result.secrets.openAI).toBe("sk-real"); expect(result.meta.includesSecret).toBe(true); } }); + it("parses the v2 secrets payload separately from settings", () => { + const result = parseImport( + JSON.stringify({ + type: SETTINGS_EXPORT_TYPE, + version: 2, + settings: { defaultVolume: 0.5, openAISecretId: "not-transferable" }, + secrets: { openAI: " sk ", deepgram: "dg" }, + }), + ); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.settings).not.toHaveProperty("openAISecretId"); + expect(result.secrets).toEqual({ openAI: "sk", deepgram: "dg" }); + } + }); + + it("rejects a malformed v2 secrets payload", () => { + const result = parseImport( + JSON.stringify({ + type: SETTINGS_EXPORT_TYPE, + version: 2, + settings: { defaultVolume: 0.5 }, + secrets: { openAI: 42 }, + }), + ); + expect(result).toEqual(expect.objectContaining({ ok: false })); + }); + + it("extracts legacy values from v1 envelopes but not from v2 data", () => { + const legacy = parseImport( + JSON.stringify({ + type: SETTINGS_EXPORT_TYPE, + version: 1, + settings: { openAIApiKey: "sk-legacy" }, + }), + ); + expect(legacy).toEqual( + expect.objectContaining({ ok: true, secrets: { openAI: "sk-legacy" } }), + ); + + const retiredInV2 = parseImport( + JSON.stringify({ + schemaVersion: 2, + defaultVolume: 0.5, + openAIApiKey: "must-not-return", + }), + ); + expect(retiredInV2).toEqual(expect.objectContaining({ ok: true, secrets: {} })); + }); + it("drops a built-in playlist whose episodes is not an array (PL-10)", () => { const result = parseImport( JSON.stringify({ @@ -429,16 +479,16 @@ describe("mergeImportedSettings", () => { it("preserves a saved secret when the import omits it (IE-05)", () => { const current = makeSettings({ - openAIApiKey: "sk-saved", - diarizationApiKey: "dg-saved", + openAISecretId: "podnotes-openai-api-key-2", + deepgramSecretId: "podnotes-deepgram-api-key-2", }); // parseImport strips empty secrets, so a raw data.json with blank keys // arrives here without them; the merge must keep the configured keys. const merged = mergeImportedSettings(current, { defaultVolume: 0.5 }); - expect(merged.openAIApiKey).toBe("sk-saved"); - expect(merged.diarizationApiKey).toBe("dg-saved"); + expect(merged.openAISecretId).toBe("podnotes-openai-api-key-2"); + expect(merged.deepgramSecretId).toBe("podnotes-deepgram-api-key-2"); }); it("keeps the current nested value when the import omits the field", () => { @@ -487,17 +537,16 @@ describe("mergeImportedSettings", () => { describe("describeSecrets (#168)", () => { it("names only the secrets that actually hold a value", () => { - expect(describeSecrets(makeSettings())).toEqual([]); - expect(describeSecrets(makeSettings({ openAIApiKey: "sk" }))).toEqual(["OpenAI API key"]); - expect(describeSecrets(makeSettings({ diarizationApiKey: "dg" }))).toEqual([ + expect(describeSecrets({})).toEqual([]); + expect(describeSecrets({ openAI: "sk" })).toEqual(["OpenAI API key"]); + expect(describeSecrets({ deepgram: "dg" })).toEqual(["Deepgram API key"]); + expect(describeSecrets({ openAI: "sk", deepgram: "dg" })).toEqual([ + "OpenAI API key", "Deepgram API key", ]); - expect( - describeSecrets(makeSettings({ openAIApiKey: "sk", diarizationApiKey: "dg" })), - ).toEqual(["OpenAI API key", "Deepgram API key"]); }); it("ignores whitespace-only secrets", () => { - expect(describeSecrets(makeSettings({ openAIApiKey: " " }))).toEqual([]); + expect(describeSecrets({ openAI: " " })).toEqual([]); }); }); diff --git a/src/settingsTransfer.ts b/src/settingsTransfer.ts index b29df412..f9675527 100644 --- a/src/settingsTransfer.ts +++ b/src/settingsTransfer.ts @@ -2,9 +2,11 @@ import { DEFAULT_SETTINGS } from "./constants"; import { decodePodNotesData, encodePodNotesData, + PODNOTES_DATA_SCHEMA_VERSION, type PersistedPodNotesSettings, PodNotesDataError, } from "./persistence/podNotesData"; +import type { CredentialValues } from "./types/Credentials"; import type { IPodNotesSettings } from "./types/IPodNotesSettings"; /** @@ -18,7 +20,7 @@ import type { IPodNotesSettings } from "./types/IPodNotesSettings"; */ export const SETTINGS_EXPORT_TYPE = "podnotes-settings"; -export const SETTINGS_EXPORT_VERSION = 1; +export const SETTINGS_EXPORT_VERSION = 2; /** * Runtime / vault-specific state that must never travel between vaults: playback @@ -33,22 +35,22 @@ export const EXCLUDED_KEYS: readonly (keyof IPodNotesSettings)[] = [ "currentEpisode", ]; -/** - * API keys are only exported when the user explicitly opts in. Both the OpenAI - * key and the dedicated diarization (Deepgram) key are top-level so they can be - * redacted by name here; the diarization key is deliberately NOT nested inside - * `transcript` (a wholesale-copied nested key) so it can never leak (#168). - */ -export const SECRET_KEYS: ReadonlySet = new Set([ - "openAIApiKey", - "diarizationApiKey", +/** SecretStorage references are device-local implementation details, not settings transfer data. */ +export const SECRET_REFERENCE_KEYS: ReadonlySet = new Set([ + "openAISecretId", + "deepgramSecretId", ]); +const LEGACY_SECRET_KEYS = { + openAIApiKey: "openAI", + diarizationApiKey: "deepgram", +} as const; + /** Human-facing names for each secret, so export/import copy can name exactly * which keys leave or enter the vault instead of hard-coding "OpenAI". */ -const SECRET_KEY_LABELS: Record = { - openAIApiKey: "OpenAI API key", - diarizationApiKey: "Deepgram API key", +const SECRET_KEY_LABELS: Record = { + openAI: "OpenAI API key", + deepgram: "Deepgram API key", }; /** @@ -57,10 +59,10 @@ const SECRET_KEY_LABELS: Record = { * import confirmation honest about which keys are involved — so a Deepgram-only * user is never told only "OpenAI API key" (and vice versa). See issue #168. */ -export function describeSecrets(settings: Partial): string[] { - return [...SECRET_KEYS] - .filter((key) => Boolean((settings[key] as string | undefined)?.trim())) - .map((key) => SECRET_KEY_LABELS[key as string]); +export function describeSecrets(secrets: CredentialValues): string[] { + return (Object.keys(SECRET_KEY_LABELS) as (keyof CredentialValues)[]) + .filter((key) => Boolean(secrets[key]?.trim())) + .map((key) => SECRET_KEY_LABELS[key]); } /** Keys that, if copied into the settings object, could pollute Object.prototype. */ @@ -69,7 +71,9 @@ const DANGEROUS_KEYS = new Set(["__proto__", "constructor", "prototype"]); /** Known top-level setting keys that are safe to import (excludes runtime state). */ const IMPORTABLE_KEYS = new Set( Object.keys(DEFAULT_SETTINGS).filter( - (key) => !EXCLUDED_KEYS.includes(key as keyof IPodNotesSettings), + (key) => + !EXCLUDED_KEYS.includes(key as keyof IPodNotesSettings) && + !SECRET_REFERENCE_KEYS.has(key as keyof IPodNotesSettings), ), ); @@ -89,17 +93,20 @@ export interface SettingsEnvelope { pluginVersion: string; exportedAt: string; settings: Partial; + /** Present only after the user explicitly opts into a plaintext credential backup. */ + secrets?: CredentialValues; } export interface ExportOptions { - /** Include the (plaintext) OpenAI API key in the export. */ - includeSecret: boolean; + /** Explicit values resolved by the UI after the user opts in. Never inferred here. */ + secrets?: CredentialValues; } export type ParseResult = | { ok: true; settings: Partial; + secrets: CredentialValues; meta: { fromEnvelope: boolean; version: number | null; @@ -111,8 +118,9 @@ export type ParseResult = /** * Build a versioned export envelope from the live settings, copying only - * allow-listed keys by name (never spreading the whole object). Runtime state is - * always excluded; the API key is excluded unless `opts.includeSecret` is set. + * allow-listed keys by name (never spreading the whole object). Runtime state and + * SecretStorage references are always excluded. Plaintext values appear only in + * the separate payload explicitly passed by the caller. */ export function serializeSettings( settings: IPodNotesSettings, @@ -126,9 +134,9 @@ export function serializeSettings( for (const key of Object.keys(settings)) { if (DANGEROUS_KEYS.has(key)) continue; if (!IMPORTABLE_KEYS.has(key)) continue; - if (SECRET_KEYS.has(key as keyof IPodNotesSettings) && !opts.includeSecret) continue; out[key] = persisted[key]; } + const secrets = normalizeSecrets(opts.secrets ?? {}); return { type: SETTINGS_EXPORT_TYPE, @@ -136,6 +144,7 @@ export function serializeSettings( pluginVersion, exportedAt: nowISO, settings: out as Partial, + ...(Object.keys(secrets).length > 0 ? { secrets } : {}), }; } @@ -160,6 +169,7 @@ export function parseImport(jsonText: string): ParseResult { let fromEnvelope = false; let version: number | null = null; let pluginVersion: string | null = null; + let secrets: CredentialValues = {}; if (raw.type === SETTINGS_EXPORT_TYPE) { fromEnvelope = true; @@ -187,22 +197,34 @@ export function parseImport(jsonText: string): ParseResult { } source = raw.settings; + if (version >= 2) { + const parsedSecrets = parseSecretsPayload(raw.secrets); + if ("error" in parsedSecrets) return { ok: false, error: parsedSecrets.error }; + secrets = parsedSecrets.values; + } else { + secrets = extractLegacySecrets(source); + } } else if (Object.prototype.hasOwnProperty.call(raw, "schemaVersion")) { try { // Validate raw data.json imports against the same schema gate as plugin // startup. The field sanitizer below still keeps import partial. - decodePodNotesData(raw); + const decoded = decodePodNotesData(raw); + if (decoded.sourceVersion < PODNOTES_DATA_SCHEMA_VERSION) { + secrets = extractLegacySecrets(source); + } } catch (error) { if (error instanceof PodNotesDataError) { return { ok: false, error: error.message }; } throw error; } + } else { + secrets = extractLegacySecrets(source); } const settings = sanitizeImportedSettings(source); - if (Object.keys(settings).length === 0) { + if (Object.keys(settings).length === 0 && Object.keys(secrets).length === 0) { return { ok: false, error: "No recognizable PodNotes settings were found in the file.", @@ -212,11 +234,12 @@ export function parseImport(jsonText: string): ParseResult { return { ok: true, settings, + secrets, meta: { fromEnvelope, version, pluginVersion, - includesSecret: [...SECRET_KEYS].some((key) => key in settings), + includesSecret: Object.keys(secrets).length > 0, }, }; } @@ -242,9 +265,9 @@ export function mergeImportedSettings( } // Converge import and startup on the same deep validators and date revival. - // Marking the in-memory merge as v1 avoids treating this internal call as a - // legacy migration while preserving the import envelope's independent version. - return decodePodNotesData({ ...merged, schemaVersion: 1 }).settings; + // Mark the in-memory merge as current so it cannot be mistaken for a legacy + // data migration while preserving the import envelope's independent version. + return decodePodNotesData({ ...merged, schemaVersion: PODNOTES_DATA_SCHEMA_VERSION }).settings; } function sanitizeImportedSettings(source: Record): Partial { @@ -261,14 +284,6 @@ function sanitizeImportedSettings(source: Record): Partial, @@ -294,6 +309,43 @@ function sanitizeImportedSettings(source: Record): Partial; } +function extractLegacySecrets(source: Record): CredentialValues { + const values: CredentialValues = {}; + + for (const [legacyKey, credentialKey] of Object.entries(LEGACY_SECRET_KEYS) as Array< + [keyof typeof LEGACY_SECRET_KEYS, keyof CredentialValues] + >) { + const value = source[legacyKey]; + if (typeof value === "string" && value.trim()) values[credentialKey] = value.trim(); + } + + return values; +} + +function parseSecretsPayload(value: unknown): { values: CredentialValues } | { error: string } { + if (value === undefined) return { values: {} }; + if (!isPlainObject(value)) { + return { error: "Export file has an invalid secrets payload." }; + } + + for (const key of ["openAI", "deepgram"] as const) { + if (value[key] !== undefined && typeof value[key] !== "string") { + return { error: `Export file has an invalid ${SECRET_KEY_LABELS[key]}.` }; + } + } + + return { values: normalizeSecrets(value as CredentialValues) }; +} + +function normalizeSecrets(values: CredentialValues): CredentialValues { + const openAI = values.openAI?.trim(); + const deepgram = values.deepgram?.trim(); + return { + ...(openAI ? { openAI } : {}), + ...(deepgram ? { deepgram } : {}), + }; +} + /** Drop any playlist entry that isn't a plain object with an `episodes` array. */ function sanitizePlaylistMap(value: Record): Record { const out: Record = {}; diff --git a/src/store/persistence.ts b/src/store/persistence.ts index befa4214..b9dbd094 100644 --- a/src/store/persistence.ts +++ b/src/store/persistence.ts @@ -5,6 +5,7 @@ import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import { currentEpisode, downloadedEpisodes, + episodeListLimit, favorites, hidePlayedEpisodes, localFiles, @@ -12,7 +13,9 @@ import { playlists, queue, savedFeeds, + volume, } from "src/store"; +import { sanitizeEpisodeListLimit } from "src/utility/episodeListLimit"; /** * A type-erased rule for mirroring one Svelte store into plugin settings. The @@ -90,6 +93,65 @@ const BINDINGS: PersistenceBinding[] = [ ), ]; +const IMPORT_ROLLBACK_BINDINGS: PersistenceBinding[] = [ + ...BINDINGS, + // Volume changes are persisted by main.ts rather than bindStoresToSettings. + // Capture them here so an equal-to-candidate event cannot be lost on rollback. + bind(volume, (settings, value) => { + if (Number.isFinite(value)) settings.defaultVolume = Math.min(1, Math.max(0, value)); + }), + // The limit control owns both this store and the persisted setting. It lives + // outside BINDINGS because changing it also rebuilds the Latest Episodes list. + bind(episodeListLimit, (settings, value) => { + settings.episodeListLimit = sanitizeEpisodeListLimit(value); + }), +]; + +export interface PersistedStoreChangeReplay { + replayInto: (settings: IPodNotesSettings) => void; + dispose: () => void; +} + +/** + * Record authoritative store emissions while an import candidate is being + * written. Replaying only stores that emitted avoids stale snapshots and solves + * the ABA case where the new store value already equals the import candidate, + * so the normal persistence binding intentionally performs no settings write. + * + * playbackRate is deliberately absent. That store is the current episode's + * transient speed, while defaultPlaybackRate is a preference changed directly + * by the settings tab. The tab locks its controls while an import is pending. + */ +export function observePersistedStoreChanges(): PersistedStoreChangeReplay { + const latestValues = new Map(); + const unsubscribers = IMPORT_ROLLBACK_BINDINGS.map((binding) => { + let receivedInitialValue = false; + return binding.subscribe((value) => { + if (!receivedInitialValue) { + receivedInitialValue = true; + return; + } + + latestValues.set(binding, structuredClone(value)); + }); + }); + let disposed = false; + + return { + replayInto(settings) { + for (const [binding, value] of latestValues) { + binding.apply(settings, structuredClone(value)); + } + }, + dispose() { + if (disposed) return; + disposed = true; + for (const unsubscribe of unsubscribers) unsubscribe(); + latestValues.clear(); + }, + }; +} + /** * Subscribes every persisted store to plugin settings and returns a single * unsubscriber that tears all of them down. Svelte fires each subscription diff --git a/src/types/Credentials.ts b/src/types/Credentials.ts new file mode 100644 index 00000000..f14030c1 --- /dev/null +++ b/src/types/Credentials.ts @@ -0,0 +1,29 @@ +export type CredentialKind = "openai" | "deepgram"; + +export interface CredentialValues { + openAI?: string; + deepgram?: string; +} + +export interface CredentialReferences { + openAISecretId?: string; + deepgramSecretId?: string; +} + +/** Obsidian SecretStorage IDs are lowercase alphanumeric strings with dashes. */ +export function isValidSecretId(value: string): boolean { + return /^(?=.*[a-z0-9])[a-z0-9-]+$/.test(value); +} + +const PODNOTES_SECRET_ID_PATTERNS: Record = { + openai: /^podnotes-openai-api-key(?:-[1-9]\d*)?$/, + deepgram: /^podnotes-deepgram-api-key(?:-[1-9]\d*)?$/, +}; + +/** + * Persisted references are capabilities, not arbitrary SecretStorage lookups. + * Keep each provider confined to the IDs PodNotes owns for that provider. + */ +export function isPodNotesSecretId(kind: CredentialKind, value: string): boolean { + return PODNOTES_SECRET_ID_PATTERNS[kind].test(value); +} diff --git a/src/types/IPodNotesSettings.ts b/src/types/IPodNotesSettings.ts index 535514e8..884d507d 100644 --- a/src/types/IPodNotesSettings.ts +++ b/src/types/IPodNotesSettings.ts @@ -53,13 +53,10 @@ export interface IPodNotesSettings { path: string; }; downloadedEpisodes: { [podcastName: string]: DownloadedEpisode[] }; - openAIApiKey: string; - /** - * API key for the dedicated diarization provider (Deepgram). Kept separate - * from `openAIApiKey` and top-level so the settings export can redact it as a - * secret; OpenAI diarization reuses `openAIApiKey` instead. See issue #168. - */ - diarizationApiKey: string; + /** SecretStorage ID for the OpenAI API key. Never contains the key itself. */ + openAISecretId: string; + /** SecretStorage ID for the Deepgram API key. Never contains the key itself. */ + deepgramSecretId: string; transcript: { path: string; template: string; diff --git a/src/ui/settings/PodNotesSettingsTab.import.test.ts b/src/ui/settings/PodNotesSettingsTab.import.test.ts index 464e1261..bfc3d65e 100644 --- a/src/ui/settings/PodNotesSettingsTab.import.test.ts +++ b/src/ui/settings/PodNotesSettingsTab.import.test.ts @@ -4,15 +4,27 @@ import type { App } from "obsidian"; import * as obsidian from "obsidian"; import { DEFAULT_SETTINGS } from "src/constants"; -import type PodNotes from "src/main"; -import { playbackRate } from "src/store"; +import PodNotes from "src/main"; +import { episodeListLimit, hidePlayedEpisodes, playbackRate, volume } from "src/store"; +import { bindStoresToSettings } from "src/store/persistence"; import { PodNotesSettingsTab } from "./PodNotesSettingsTab"; +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + describe("PodNotesSettingsTab settings import", () => { it("rehydrates the live playback-rate store when importing a new default", async () => { playbackRate.set(1); const plugin = { settings: structuredClone(DEFAULT_SETTINGS), + credentials: { storeValues: vi.fn(() => ({})) }, saveSettings: vi.fn().mockResolvedValue(undefined), saveSettingsStrict: vi.fn().mockResolvedValue(undefined), } as unknown as PodNotes; @@ -38,6 +50,7 @@ describe("PodNotesSettingsTab settings import", () => { const failure = new Error("disk full"); const plugin = { settings: previous, + credentials: { storeValues: vi.fn(() => ({})) }, saveSettings: vi.fn().mockResolvedValue(undefined), saveSettingsStrict: vi .fn() @@ -61,9 +74,521 @@ describe("PodNotesSettingsTab settings import", () => { expect(get(playbackRate)).toBe(1); expect(display).not.toHaveBeenCalled(); expect(notice).toHaveBeenCalledWith( - "Could not import PodNotes settings. Previous settings were kept.", + "Could not import PodNotes settings. The failed import was rolled back without overwriting newer changes.", 10000, ); expect(plugin.saveSettingsStrict).toHaveBeenCalledTimes(2); }); + + it("locks ordinary settings controls until an import and its lane drain finish", async () => { + const firstSave = deferred(); + const plugin = { + settings: structuredClone(DEFAULT_SETTINGS), + credentials: { storeValues: vi.fn(() => ({})) }, + saveSettings: vi.fn().mockResolvedValue(undefined), + saveSettingsStrict: vi + .fn() + .mockImplementationOnce(() => firstSave.promise) + .mockResolvedValue(undefined), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, plugin); + const enabledInput = tab.containerEl.createEl("input"); + const disabledButton = tab.containerEl.createEl("button"); + disabledButton.disabled = true; + vi.spyOn(tab, "display").mockImplementation(() => {}); + + const importing = ( + tab as unknown as { + applyImportedSettings: ( + imported: Partial, + ) => Promise; + } + ).applyImportedSettings({ defaultPlaybackRate: 2 }); + expect(enabledInput.disabled).toBe(true); + expect(disabledButton.disabled).toBe(true); + + firstSave.resolve(); + await importing; + + expect(enabledInput.disabled).toBe(false); + expect(disabledButton.disabled).toBe(true); + playbackRate.set(DEFAULT_SETTINGS.defaultPlaybackRate); + }); + + it("stores imported secret values first and persists only their references", async () => { + const plugin = { + settings: structuredClone(DEFAULT_SETTINGS), + credentials: { + storeValues: vi.fn(() => ({ + openAISecretId: "podnotes-openai-api-key", + deepgramSecretId: "podnotes-deepgram-api-key", + })), + }, + invalidateTranscriptionCredentialCache: vi.fn(), + saveSettings: vi.fn().mockResolvedValue(undefined), + saveSettingsStrict: vi.fn().mockResolvedValue(undefined), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, plugin); + vi.spyOn(tab, "display").mockImplementation(() => {}); + + await ( + tab as unknown as { + applyImportedSettings: ( + imported: Partial, + secrets: { openAI?: string; deepgram?: string }, + ) => Promise; + } + ).applyImportedSettings({ defaultVolume: 0.4 }, { openAI: "sk", deepgram: "dg" }); + + expect(plugin.credentials.storeValues).toHaveBeenCalledWith({ + openAI: "sk", + deepgram: "dg", + }); + expect(plugin.settings.openAISecretId).toBe("podnotes-openai-api-key"); + expect(plugin.settings.deepgramSecretId).toBe("podnotes-deepgram-api-key"); + expect(plugin.invalidateTranscriptionCredentialCache).toHaveBeenCalledOnce(); + expect(plugin.settings).not.toHaveProperty("openAIApiKey"); + expect(plugin.settings).not.toHaveProperty("diarizationApiKey"); + }); + + it("does not mutate settings when SecretStorage import fails partway", async () => { + const previous = structuredClone(DEFAULT_SETTINGS); + const plugin = { + settings: previous, + credentials: { + storeValues: vi.fn(() => { + throw new Error("second credential failed"); + }), + }, + saveSettings: vi.fn().mockResolvedValue(undefined), + saveSettingsStrict: vi.fn().mockResolvedValue(undefined), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, plugin); + const notice = vi.spyOn(obsidian, "Notice"); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await ( + tab as unknown as { + applyImportedSettings: ( + imported: Partial, + secrets: { openAI?: string; deepgram?: string }, + ) => Promise; + } + ).applyImportedSettings({ defaultVolume: 0.4 }, { openAI: "sk", deepgram: "dg" }); + + expect(plugin.settings).toBe(previous); + expect(plugin.saveSettingsStrict).not.toHaveBeenCalled(); + expect(notice).toHaveBeenCalledWith( + expect.stringContaining("retrying will safely reuse"), + 10000, + ); + }); + + it("restores previous secret references when the settings save fails", async () => { + const previous = { + ...structuredClone(DEFAULT_SETTINGS), + openAISecretId: "podnotes-openai-api-key", + }; + const plugin = { + settings: previous, + credentials: { + storeValues: vi.fn(() => ({ openAISecretId: "podnotes-openai-api-key-2" })), + }, + invalidateTranscriptionCredentialCache: vi.fn(), + saveSettings: vi.fn().mockResolvedValue(undefined), + saveSettingsStrict: vi + .fn() + .mockRejectedValueOnce(new Error("disk full")) + .mockResolvedValueOnce(undefined), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, plugin); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await ( + tab as unknown as { + applyImportedSettings: ( + imported: Partial, + secrets: { openAI?: string }, + ) => Promise; + } + ).applyImportedSettings({ defaultVolume: 0.4 }, { openAI: "sk-new" }); + + expect(plugin.settings).toBe(previous); + expect(plugin.settings.openAISecretId).toBe("podnotes-openai-api-key"); + expect(plugin.invalidateTranscriptionCredentialCache).toHaveBeenCalledTimes(2); + expect(plugin.saveSettingsStrict).toHaveBeenCalledTimes(2); + }); + + it("rolls back a SecretComponent reference when strict persistence fails", async () => { + const plugin = { + settings: { + ...structuredClone(DEFAULT_SETTINGS), + openAISecretId: "podnotes-openai-api-key", + }, + credentials: { + adoptReference: vi.fn(() => "podnotes-openai-api-key-2"), + }, + saveSettingsStrict: vi + .fn() + .mockRejectedValueOnce(new Error("disk full")) + .mockResolvedValueOnce(undefined), + invalidateTranscriptionCredentialCache: vi.fn(), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, plugin); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + const saved = await ( + tab as unknown as { + saveSecretReference: ( + key: "openAISecretId", + value: string, + ) => Promise<{ persistedId: string; saved: boolean; isLatest: boolean }>; + } + ).saveSecretReference("openAISecretId", "shared-new-secret"); + + expect(saved).toEqual({ + persistedId: "podnotes-openai-api-key", + saved: false, + isLatest: true, + }); + expect(plugin.settings.openAISecretId).toBe("podnotes-openai-api-key"); + expect(plugin.invalidateTranscriptionCredentialCache).toHaveBeenCalledTimes(2); + expect(plugin.saveSettingsStrict).toHaveBeenCalledTimes(2); + }); + + it("keeps a newer selection authoritative when an older save fails", async () => { + const firstSave = deferred(); + let disk = "podnotes-openai-api-key"; + let saveCall = 0; + const plugin = { + settings: { + ...structuredClone(DEFAULT_SETTINGS), + openAISecretId: disk, + }, + credentials: { + adoptReference: vi.fn((_kind: string, id: string) => + id === "shared-a" ? "podnotes-openai-api-key-2" : "podnotes-openai-api-key-3", + ), + }, + invalidateTranscriptionCredentialCache: vi.fn(), + saveSettingsStrict: vi.fn(() => { + const snapshot = plugin.settings.openAISecretId; + saveCall++; + if (saveCall === 1) { + return firstSave.promise.then(() => { + disk = snapshot; + }); + } + disk = snapshot; + return Promise.resolve(); + }), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, plugin); + vi.spyOn(console, "error").mockImplementation(() => undefined); + let ui = "shared-a"; + const secret = { + setValue: vi.fn((value: string) => { + ui = value; + return secret; + }), + }; + const handle = ( + tab as unknown as { + handleSecretSelection: ( + key: "openAISecretId", + value: string, + component: typeof secret, + onSettled: () => void, + ) => Promise; + } + ).handleSecretSelection.bind(tab); + + const older = handle("openAISecretId", "shared-a", secret, vi.fn()); + await Promise.resolve(); + ui = "shared-b"; + const newer = handle("openAISecretId", "shared-b", secret, vi.fn()); + firstSave.reject(new Error("first save failed")); + await Promise.all([older, newer]); + + expect(plugin.settings.openAISecretId).toBe("podnotes-openai-api-key-3"); + expect(disk).toBe("podnotes-openai-api-key-3"); + expect(ui).toBe("podnotes-openai-api-key-3"); + expect(secret.setValue).toHaveBeenCalledTimes(1); + expect(plugin.saveSettingsStrict).toHaveBeenCalledTimes(3); + }); + + it("restores the last durable selection when the newer overlapping save fails", async () => { + const firstSave = deferred(); + const secondSave = deferred(); + let disk = "podnotes-openai-api-key"; + let saveCall = 0; + const plugin = { + settings: { + ...structuredClone(DEFAULT_SETTINGS), + openAISecretId: disk, + }, + credentials: { + adoptReference: vi.fn((_kind: string, id: string) => + id === "shared-a" ? "podnotes-openai-api-key-2" : "podnotes-openai-api-key-3", + ), + }, + invalidateTranscriptionCredentialCache: vi.fn(), + saveSettingsStrict: vi.fn(() => { + const snapshot = plugin.settings.openAISecretId; + saveCall++; + if (saveCall === 1) { + return firstSave.promise.then(() => { + disk = snapshot; + }); + } + if (saveCall === 2) { + return secondSave.promise.then(() => { + disk = snapshot; + }); + } + disk = snapshot; + return Promise.resolve(); + }), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, plugin); + vi.spyOn(console, "error").mockImplementation(() => undefined); + let ui = "shared-b"; + const secret = { + setValue: vi.fn((value: string) => { + ui = value; + return secret; + }), + }; + const handle = ( + tab as unknown as { + handleSecretSelection: ( + key: "openAISecretId", + value: string, + component: typeof secret, + onSettled: () => void, + ) => Promise; + } + ).handleSecretSelection.bind(tab); + + const older = handle("openAISecretId", "shared-a", secret, vi.fn()); + await Promise.resolve(); + const newer = handle("openAISecretId", "shared-b", secret, vi.fn()); + firstSave.resolve(); + await older; + expect(plugin.saveSettingsStrict).toHaveBeenCalledTimes(2); + secondSave.reject(new Error("second save failed")); + await newer; + + expect(plugin.settings.openAISecretId).toBe("podnotes-openai-api-key-2"); + expect(disk).toBe("podnotes-openai-api-key-2"); + expect(ui).toBe("podnotes-openai-api-key-2"); + expect(secret.setValue).toHaveBeenCalledTimes(1); + expect(plugin.saveSettingsStrict).toHaveBeenCalledTimes(3); + }); + + it("keeps a newer secret selection and store update after a failing import", async () => { + const firstWrite = deferred(); + const previousSecretId = "podnotes-openai-api-key"; + const importedSecretId = "podnotes-openai-api-key-2"; + const newerSecretId = "podnotes-openai-api-key-3"; + const newerProgress = { + "Podcast::Episode": { + title: "Episode", + podcastName: "Podcast", + time: 12, + duration: 30, + finished: false, + }, + }; + let disk = { + ...structuredClone(DEFAULT_SETTINGS), + openAISecretId: previousSecretId, + }; + let saveCall = 0; + const saveData = vi.fn((snapshot: unknown) => { + saveCall++; + if (saveCall === 1) { + return firstWrite.promise.then(() => { + disk = structuredClone(snapshot) as typeof disk; + }); + } + disk = structuredClone(snapshot) as typeof disk; + return Promise.resolve(); + }); + const plugin = Object.create(PodNotes.prototype) as PodNotes; + Object.assign(plugin, { + settings: structuredClone(disk), + credentials: { + storeValues: vi.fn(() => ({ openAISecretId: importedSecretId })), + adoptReference: vi.fn(() => newerSecretId), + }, + invalidateTranscriptionCredentialCache: vi.fn(), + isReady: true, + pendingSave: null, + pendingSaveWaiters: [], + saveScheduled: false, + saveChain: Promise.resolve(), + persistenceUnknownFields: {}, + saveData, + }); + const tab = new PodNotesSettingsTab({} as App, plugin); + vi.spyOn(tab, "display").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => undefined); + let ui = "shared-newer"; + const secret = { + setValue: vi.fn((value: string) => { + ui = value; + return secret; + }), + }; + const applyImport = ( + tab as unknown as { + applyImportedSettings: ( + imported: Partial, + secrets: { openAI?: string }, + ) => Promise; + } + ).applyImportedSettings.bind(tab); + const selectSecret = ( + tab as unknown as { + handleSecretSelection: ( + key: "openAISecretId", + value: string, + component: typeof secret, + onSettled: () => void, + ) => Promise; + } + ).handleSecretSelection.bind(tab); + + const importing = applyImport({ defaultVolume: 0.4 }, { openAI: "imported" }); + await vi.waitFor(() => expect(saveData).toHaveBeenCalledTimes(1)); + + plugin.settings.playedEpisodes = newerProgress; + const savingProgress = plugin.saveSettings(); + const selecting = selectSecret("openAISecretId", "shared-newer", secret, vi.fn()); + await Promise.resolve(); + + firstWrite.reject(new Error("first import write failed")); + await Promise.all([importing, savingProgress, selecting]); + + expect(plugin.settings.openAISecretId).toBe(newerSecretId); + expect(plugin.settings.defaultVolume).toBe(DEFAULT_SETTINGS.defaultVolume); + expect(plugin.settings.playedEpisodes).toEqual(newerProgress); + expect(disk.openAISecretId).toBe(newerSecretId); + expect(disk.defaultVolume).toBe(DEFAULT_SETTINGS.defaultVolume); + expect(disk.playedEpisodes).toEqual(newerProgress); + expect(ui).toBe(newerSecretId); + }); + + it("keeps an ABA hide-played store update when the matching import fails", async () => { + const firstWrite = deferred(); + let disk = structuredClone(DEFAULT_SETTINGS); + let saveCall = 0; + const saveData = vi.fn((snapshot: unknown) => { + saveCall++; + if (saveCall === 1) { + return firstWrite.promise.then(() => { + disk = structuredClone(snapshot) as typeof disk; + }); + } + disk = structuredClone(snapshot) as typeof disk; + return Promise.resolve(); + }); + const plugin = Object.create(PodNotes.prototype) as PodNotes; + Object.assign(plugin, { + settings: structuredClone(DEFAULT_SETTINGS), + credentials: { storeValues: vi.fn(() => ({})) }, + invalidateTranscriptionCredentialCache: vi.fn(), + isReady: false, + pendingSave: null, + pendingSaveWaiters: [], + saveScheduled: false, + saveChain: Promise.resolve(), + persistenceUnknownFields: {}, + saveData, + }); + hidePlayedEpisodes.set(false); + volume.set(DEFAULT_SETTINGS.defaultVolume); + episodeListLimit.set(DEFAULT_SETTINGS.episodeListLimit); + playbackRate.set(DEFAULT_SETTINGS.defaultPlaybackRate); + const unsubscribe = bindStoresToSettings(plugin); + Object.assign(plugin, { isReady: true }); + const tab = new PodNotesSettingsTab({} as App, plugin); + vi.spyOn(tab, "display").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + try { + const importing = ( + tab as unknown as { + applyImportedSettings: ( + imported: Partial, + ) => Promise; + } + ).applyImportedSettings({ + hidePlayedEpisodes: true, + defaultVolume: 0.4, + episodeListLimit: 20, + defaultPlaybackRate: 1.8, + }); + await vi.waitFor(() => expect(saveData).toHaveBeenCalledTimes(1)); + + hidePlayedEpisodes.set(true); + volume.set(0.4); + episodeListLimit.set(20); + // Current playback speed is runtime state, not the persisted default. + playbackRate.set(2.5); + firstWrite.reject(new Error("first import write failed")); + await importing; + + expect(get(hidePlayedEpisodes)).toBe(true); + expect(plugin.settings.hidePlayedEpisodes).toBe(true); + expect(disk.hidePlayedEpisodes).toBe(true); + expect(plugin.settings.defaultVolume).toBe(0.4); + expect(disk.defaultVolume).toBe(0.4); + expect(plugin.settings.episodeListLimit).toBe(20); + expect(disk.episodeListLimit).toBe(20); + expect(get(playbackRate)).toBe(2.5); + expect(plugin.settings.defaultPlaybackRate).toBe(DEFAULT_SETTINGS.defaultPlaybackRate); + expect(disk.defaultPlaybackRate).toBe(DEFAULT_SETTINGS.defaultPlaybackRate); + } finally { + unsubscribe(); + hidePlayedEpisodes.set(false); + volume.set(DEFAULT_SETTINGS.defaultVolume); + episodeListLimit.set(DEFAULT_SETTINGS.episodeListLimit); + playbackRate.set(DEFAULT_SETTINGS.defaultPlaybackRate); + } + }); + + it("ordinary settings export never resolves or serializes SecretStorage values", async () => { + const create = vi.fn().mockResolvedValue(undefined); + const exportValues = vi.fn(() => ({ openAI: "must-not-be-read" })); + const app = { + vault: { + getAbstractFileByPath: vi.fn(() => null), + create, + }, + } as unknown as App; + const plugin = { + settings: { + ...structuredClone(DEFAULT_SETTINGS), + openAISecretId: "podnotes-openai-api-key", + }, + credentials: { exportValues }, + manifest: { version: "2.17.3" }, + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab(app, plugin); + + await ( + tab as unknown as { + handleSettingsExport: (fileName: string, includeSecret: boolean) => Promise; + } + ).handleSettingsExport("settings.json", false); + + expect(exportValues).not.toHaveBeenCalled(); + expect(create).toHaveBeenCalledTimes(1); + const serialized = create.mock.calls[0][1] as string; + expect(serialized).not.toContain("must-not-be-read"); + expect(serialized).not.toContain("openAISecretId"); + expect(JSON.parse(serialized)).not.toHaveProperty("secrets"); + }); }); diff --git a/src/ui/settings/PodNotesSettingsTab.ts b/src/ui/settings/PodNotesSettingsTab.ts index bd7af5e0..98a0e271 100644 --- a/src/ui/settings/PodNotesSettingsTab.ts +++ b/src/ui/settings/PodNotesSettingsTab.ts @@ -5,6 +5,7 @@ import { Modal, Notice, PluginSettingTab, + SecretComponent, Setting, } from "obsidian"; import type PodNotes from "../../main"; @@ -46,6 +47,122 @@ import { } from "src/settingsTransfer"; import { normalizePlaybackRate } from "src/utility/playbackRate"; import { DEFAULT_SPEAKER_TEMPLATE, type DiarizationProviderId } from "src/services/diarization"; +import type { CredentialKind, CredentialValues } from "src/types/Credentials"; +import { observePersistedStoreChanges } from "src/store/persistence"; + +type SecretReferenceKey = "openAISecretId" | "deepgramSecretId"; +type SettingsControl = + | HTMLButtonElement + | HTMLInputElement + | HTMLSelectElement + | HTMLTextAreaElement; + +interface SecretReferenceSaveResult { + persistedId: string; + saved: boolean; + isLatest: boolean; +} + +type ImportMutationResult = "failed" | "applied" | "complete"; + +function isPlainRecord(value: unknown): value is Record { + if (typeof value !== "object" || value === null || Array.isArray(value)) return false; + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +function settingsValuesEqual(left: unknown, right: unknown): boolean { + if (Object.is(left, right)) return true; + if (left instanceof Date || right instanceof Date) { + return left instanceof Date && right instanceof Date && left.getTime() === right.getTime(); + } + if (Array.isArray(left) || Array.isArray(right)) { + return ( + Array.isArray(left) && + Array.isArray(right) && + left.length === right.length && + left.every((value, index) => settingsValuesEqual(value, right[index])) + ); + } + if (!isPlainRecord(left) || !isPlainRecord(right)) return false; + + const leftKeys = Object.keys(left); + const rightKeys = Object.keys(right); + return ( + leftKeys.length === rightKeys.length && + leftKeys.every( + (key) => + Object.prototype.hasOwnProperty.call(right, key) && + settingsValuesEqual(left[key], right[key]), + ) + ); +} + +/** + * Three-way rollback for a failed import. Only values changed by the import and + * still equal to its failed candidate are restored. Store subscriptions and + * other runtime writers that changed a value while the write was pending win. + */ +function restoreFailedImport( + previous: IPodNotesSettings, + candidate: IPodNotesSettings, + current: IPodNotesSettings, +): IPodNotesSettings { + const restored = structuredClone(current); + restoreFailedImportChanges( + previous as unknown as Record, + candidate as unknown as Record, + current as unknown as Record, + restored as unknown as Record, + ); + return restored; +} + +function restoreFailedImportChanges( + previous: Record, + candidate: Record, + current: Record, + restored: Record, +): void { + for (const key of new Set([...Object.keys(previous), ...Object.keys(candidate)])) { + const previousHasKey = Object.prototype.hasOwnProperty.call(previous, key); + const candidateHasKey = Object.prototype.hasOwnProperty.call(candidate, key); + const currentHasKey = Object.prototype.hasOwnProperty.call(current, key); + + if ( + previousHasKey === candidateHasKey && + (!previousHasKey || settingsValuesEqual(previous[key], candidate[key])) + ) { + continue; + } + + if ( + currentHasKey === candidateHasKey && + (!currentHasKey || settingsValuesEqual(current[key], candidate[key])) + ) { + if (previousHasKey) restored[key] = structuredClone(previous[key]); + else delete restored[key]; + continue; + } + + if ( + previousHasKey && + candidateHasKey && + currentHasKey && + isPlainRecord(previous[key]) && + isPlainRecord(candidate[key]) && + isPlainRecord(current[key]) && + isPlainRecord(restored[key]) + ) { + restoreFailedImportChanges(previous[key], candidate[key], current[key], restored[key]); + } + } +} + +const CREDENTIAL_KIND_BY_REFERENCE: Record = { + openAISecretId: "openai", + deepgramSecretId: "deepgram", +}; /** * Stack a Setting's control beneath its name, full width — the layout the @@ -77,6 +194,13 @@ export class PodNotesSettingsTab extends PluginSettingTab { private podcastQueryGrid: Record | null = null; private playlistManager: Record | null = null; + private settingsMutationTail: Promise = Promise.resolve(); + private credentialSaveGenerations: Record = { + openAISecretId: 0, + deepgramSecretId: 0, + }; + private settingsInteractionLockCount = 0; + private settingsControlDisabledStates = new Map(); private settingsTab: PodNotesSettingsTab; @@ -590,7 +714,7 @@ export class PodNotesSettingsTab extends PluginSettingTab { new Setting(containerEl) .setName("Include API keys") .setDesc( - "When enabled, the export file contains your OpenAI and Deepgram API keys in plaintext (whichever you have set). The file is stored in your vault, so it may sync to other devices and be read by other plugins.", + "When enabled, a separate plaintext secrets payload is added for the OpenAI and Deepgram keys available on this device. The file is stored in your vault, so it may sync to other devices and be read by other plugins.", ) .addToggle((toggle) => toggle.setValue(includeSecret).onChange((value) => { @@ -673,9 +797,14 @@ export class PodNotesSettingsTab extends PluginSettingTab { private async handleSettingsExport(fileName: string, includeSecret: boolean): Promise { try { + const secrets = includeSecret + ? this.plugin.credentials.exportValues(this.plugin.settings, { + requireConfigured: true, + }) + : {}; const envelope = serializeSettings( this.plugin.settings, - { includeSecret }, + { secrets: includeSecret ? secrets : undefined }, this.plugin.manifest.version, new Date().toISOString(), ); @@ -690,7 +819,7 @@ export class PodNotesSettingsTab extends PluginSettingTab { } await this.app.vault.create(fileName, contents); - const exportedSecrets = includeSecret ? describeSecrets(this.plugin.settings) : []; + const exportedSecrets = describeSecrets(secrets); new Notice( exportedSecrets.length ? `Exported PodNotes settings to "${fileName}" (includes your ${exportedSecrets.join(" and ")}).` @@ -734,42 +863,98 @@ export class PodNotesSettingsTab extends PluginSettingTab { if (willReplace("playlists", this.plugin.settings.playlists)) { sections.push("playlists"); } - sections.push(...describeSecrets(result.settings)); + sections.push(...describeSecrets(result.secrets)); const detail = sections.length ? ` This also replaces your ${sections.join(", ")}.` : ""; + const secretDetail = + Object.keys(result.secrets).length > 0 + ? " Existing Obsidian secrets are never overwritten; conflicts are saved under a new PodNotes name." + : ""; new ConfirmModal( this.app, "Import PodNotes settings?", - `This overwrites your current PodNotes preferences and templates with the imported values.${detail} Your playback progress and downloads are kept.`, + `This overwrites your current PodNotes preferences and templates with the imported values.${detail}${secretDetail} Your playback progress and downloads are kept.`, "Import", () => { - void this.applyImportedSettings(result.settings); + void this.applyImportedSettings(result.settings, result.secrets); }, ).open(); } - private async applyImportedSettings(imported: Partial): Promise { - const previous = this.plugin.settings; - const merged = mergeImportedSettings(previous, imported); + private async applyImportedSettings( + imported: Partial, + secrets: CredentialValues = {}, + ): Promise { + const unlockSettingsInteractions = this.lockSettingsInteractions(); + try { + const result = await this.enqueueSettingsMutation(() => + this.persistImportedSettings(imported, secrets), + ); + if (result === "failed") return; + + await this.waitForSettingsMutationLaneToDrain(); + this.display(); + if (result === "complete") new Notice("Imported PodNotes settings."); + } finally { + await this.waitForSettingsMutationLaneToDrain(); + unlockSettingsInteractions(); + } + } + + private async persistImportedSettings( + imported: Partial, + secrets: CredentialValues, + ): Promise { + const previousSettings = this.plugin.settings; + const previous = structuredClone(previousSettings); + let secretReferences: Partial = {}; + try { + secretReferences = this.plugin.credentials.storeValues(secrets); + } catch (error) { + new Notice( + "Could not finish importing API keys into Obsidian SecretStorage. Existing settings were kept, and retrying will safely reuse any PodNotes secrets already created.", + 10000, + ); + console.error("PodNotes: failed to import credentials into SecretStorage", error); + return "failed"; + } + + const merged = mergeImportedSettings(previous, { ...imported, ...secretReferences }); + const failedCandidate = structuredClone(merged); + const openAIReferenceChanged = merged.openAISecretId !== previous.openAISecretId; + if (openAIReferenceChanged) this.plugin.invalidateTranscriptionCredentialCache(); + const concurrentStoreChanges = observePersistedStoreChanges(); this.plugin.settings = merged; try { // Persist before mutating live stores so a disk failure can restore the // previous in-memory settings without leaving the UI half-imported. await this.plugin.saveSettingsStrict(); } catch (error) { - this.plugin.settings = previous; + let restored: IPodNotesSettings; + try { + restored = restoreFailedImport(previous, failedCandidate, this.plugin.settings); + concurrentStoreChanges.replayInto(restored); + } finally { + concurrentStoreChanges.dispose(); + } + this.plugin.settings = + settingsValuesEqual(restored, previous) && + settingsValuesEqual(previousSettings, previous) + ? previousSettings + : restored; + if (openAIReferenceChanged) this.plugin.invalidateTranscriptionCredentialCache(); try { // A store event may have queued a newer merged snapshot while the first // write was pending. Queue the rollback after it and wait for durability // before claiming that the previous settings were kept. await this.plugin.saveSettingsStrict(); new Notice( - "Could not import PodNotes settings. Previous settings were kept.", + "Could not import PodNotes settings. The failed import was rolled back without overwriting newer changes.", 10000, ); } catch (rollbackError) { new Notice( - "Could not import PodNotes settings or persist the rollback. Previous settings remain active for this session.", + "Could not import PodNotes settings or persist its rollback. The safest recovered settings remain active for this session.", 10000, ); console.error( @@ -778,8 +963,9 @@ export class PodNotesSettingsTab extends PluginSettingTab { ); } console.error("PodNotes: failed to persist imported settings", error); - return; + return "failed"; } + concurrentStoreChanges.dispose(); // Re-hydrate the live stores so the running UI and the persistence // bindings reflect the import. Keys without a store (templates, paths, @@ -808,7 +994,7 @@ export class PodNotesSettingsTab extends PluginSettingTab { 10000, ); console.error("PodNotes: failed to persist normalized imported settings", error); - return; + return "applied"; } // Re-emit the plugin store so an open player/grid recomputes Queue tile/list // visibility (and any other $plugin-derived UI) after an import, mirroring the @@ -816,8 +1002,132 @@ export class PodNotesSettingsTab extends PluginSettingTab { // this keeps the import path correct independent of that incidental emission // (issue #108). plugin.set(this.plugin); - this.display(); - new Notice("Imported PodNotes settings."); + return "complete"; + } + + private enqueueSettingsMutation(mutation: () => Promise): Promise { + const operation = this.settingsMutationTail.then(mutation); + this.settingsMutationTail = operation.then( + () => undefined, + () => undefined, + ); + return operation; + } + + private lockSettingsInteractions(): () => void { + this.settingsInteractionLockCount++; + if (this.settingsInteractionLockCount === 1) { + for (const control of this.containerEl.querySelectorAll( + "button, input, select, textarea", + )) { + this.settingsControlDisabledStates.set(control, control.disabled); + control.disabled = true; + } + } + + let released = false; + return () => { + if (released) return; + released = true; + this.settingsInteractionLockCount--; + if (this.settingsInteractionLockCount > 0) return; + + for (const [control, wasDisabled] of this.settingsControlDisabledStates) { + control.disabled = wasDisabled; + } + this.settingsControlDisabledStates.clear(); + }; + } + + private async waitForSettingsMutationLaneToDrain(): Promise { + while (true) { + const tail = this.settingsMutationTail; + await tail; + if (tail === this.settingsMutationTail) return; + } + } + + private async saveSecretReference( + key: SecretReferenceKey, + selectedId: string, + ): Promise { + const generation = ++this.credentialSaveGenerations[key]; + const operation = this.enqueueSettingsMutation(() => + this.persistSecretReference(key, selectedId), + ); + const result = await operation; + return { + ...result, + isLatest: generation === this.credentialSaveGenerations[key], + }; + } + + private async persistSecretReference( + key: SecretReferenceKey, + selectedId: string, + ): Promise> { + const previous = this.plugin.settings[key]; + let persistedId: string; + try { + persistedId = selectedId + ? this.plugin.credentials.adoptReference( + CREDENTIAL_KIND_BY_REFERENCE[key], + selectedId, + ) + : ""; + } catch (error) { + new Notice( + "Could not use the selected API key. It may no longer be available in Obsidian SecretStorage.", + 0, + ); + console.error("PodNotes: failed to adopt credential reference", error); + return { persistedId: previous, saved: false }; + } + + if (persistedId === previous) return { persistedId, saved: true }; + if (key === "openAISecretId") this.plugin.invalidateTranscriptionCredentialCache(); + this.plugin.settings[key] = persistedId; + + try { + await this.plugin.saveSettingsStrict(); + return { persistedId, saved: true }; + } catch (error) { + this.plugin.settings[key] = previous; + if (key === "openAISecretId") this.plugin.invalidateTranscriptionCredentialCache(); + try { + await this.plugin.saveSettingsStrict(); + new Notice( + "Could not save the API key selection. The previous selection was kept.", + 0, + ); + } catch (rollbackError) { + new Notice( + "Could not save the API key selection or its rollback. The previous selection remains active for this session.", + 0, + ); + console.error( + "PodNotes: failed to persist credential-reference rollback", + rollbackError, + ); + } + console.error("PodNotes: failed to persist credential reference", error); + return { persistedId: previous, saved: false }; + } + } + + private async handleSecretSelection( + key: SecretReferenceKey, + selectedId: string, + secret: Pick, + onLatestSettled: () => void, + ): Promise { + const result = await this.saveSecretReference(key, selectedId); + if (!result.isLatest) return; + + // Always show the canonical persisted ID. Foreign/shared selections are + // copied into provider-scoped PodNotes IDs before they reach data.json. + secret.setValue(result.persistedId); + onLatestSettled(); } private addTranscriptSettings(container: HTMLDivElement) { @@ -825,18 +1135,29 @@ export class PodNotesSettingsTab extends PluginSettingTab { const randomEpisode = getRandomEpisode(); - new Setting(container) - .setName("OpenAI API Key") - .setDesc("Enter your OpenAI API key for transcription functionality.") - .addText((text) => { - text.setPlaceholder("Enter your OpenAI API key") - .setValue(this.plugin.settings.openAIApiKey) - .onChange(async (value) => { - this.plugin.settings.openAIApiKey = value; - await this.plugin.saveSettings(); - }); - text.inputEl.type = "password"; + const openAISetting = new Setting(container).setName("OpenAI API key"); + const updateOpenAIDescription = () => { + openAISetting.setDesc( + this.plugin.credentials.status(this.plugin.settings, "openai") === "missing" + ? "The selected secret is not available on this device. Select an existing secret or create one." + : "Select an existing Obsidian secret or create one for transcription.", + ); + }; + updateOpenAIDescription(); + openAISetting.addComponent((element) => { + const secret = new SecretComponent(this.app, element).setValue( + this.plugin.settings.openAISecretId, + ); + secret.onChange(async (value) => { + await this.handleSecretSelection( + "openAISecretId", + value, + secret, + updateOpenAIDescription, + ); }); + return secret; + }); new Setting(container) .setName("Transcript file path") @@ -882,84 +1203,90 @@ export class PodNotesSettingsTab extends PluginSettingTab { this.addDiarizationSettings(container); } - /** - * Opt-in speaker diarization controls (issue #168). Rendered into its own - * container so the provider-dependent fields (Deepgram key) can be shown or - * hidden in place when the toggle/provider changes, without re-rendering the - * whole settings tab. - */ + /** Opt-in speaker diarization controls (issue #168). */ private addDiarizationSettings(container: HTMLElement): void { const diarizationContainer = container.createDiv(); + const diarization = this.plugin.settings.transcript.diarization; + let updateVisibility = () => {}; - const renderDiarizationSettings = () => { - diarizationContainer.empty(); - const diarization = this.plugin.settings.transcript.diarization; + new Setting(diarizationContainer) + .setName("Speaker diarization") + .setDesc( + "Label transcript segments by speaker. Routes the episode audio to a diarization-capable provider instead of plain Whisper.", + ) + .addToggle((toggle) => + toggle.setValue(diarization.enabled).onChange(async (value) => { + this.plugin.settings.transcript.diarization.enabled = value; + await this.plugin.saveSettings(); + updateVisibility(); + }), + ); - new Setting(diarizationContainer) - .setName("Speaker diarization") - .setDesc( - "Label transcript segments by speaker. Routes the episode audio to a diarization-capable provider instead of plain Whisper.", - ) - .addToggle((toggle) => - toggle.setValue(diarization.enabled).onChange(async (value) => { - this.plugin.settings.transcript.diarization.enabled = value; + const providerSetting = new Setting(diarizationContainer) + .setName("Diarization provider") + .setDesc( + "OpenAI reuses your OpenAI API key above (long episodes are chunked, so speaker labels can reset across chunks). Deepgram needs its own key and keeps speaker labels consistent across the whole episode.", + ) + .addDropdown((dropdown) => + dropdown + .addOption("openai", "OpenAI (gpt-4o-transcribe-diarize)") + .addOption("deepgram", "Deepgram") + .setValue(diarization.provider) + .onChange(async (value) => { + this.plugin.settings.transcript.diarization.provider = + value as DiarizationProviderId; await this.plugin.saveSettings(); - renderDiarizationSettings(); + updateVisibility(); }), - ); - - if (!diarization.enabled) return; + ); - new Setting(diarizationContainer) - .setName("Diarization provider") - .setDesc( - "OpenAI reuses your OpenAI API key above (long episodes are chunked, so speaker labels can reset across chunks). Deepgram needs its own key and keeps speaker labels consistent across the whole episode.", - ) - .addDropdown((dropdown) => - dropdown - .addOption("openai", "OpenAI (gpt-4o-transcribe-diarize)") - .addOption("deepgram", "Deepgram") - .setValue(diarization.provider) - .onChange(async (value) => { - this.plugin.settings.transcript.diarization.provider = - value as DiarizationProviderId; - await this.plugin.saveSettings(); - renderDiarizationSettings(); - }), + const deepgramSetting = new Setting(diarizationContainer).setName("Deepgram API key"); + const updateDeepgramDescription = () => { + deepgramSetting.setDesc( + this.plugin.credentials.status(this.plugin.settings, "deepgram") === "missing" + ? "The selected secret is not available on this device. Select an existing secret or create one." + : "Select an Obsidian secret for Deepgram diarization, or create one at deepgram.com.", + ); + }; + updateDeepgramDescription(); + deepgramSetting.addComponent((element) => { + const secret = new SecretComponent(this.app, element).setValue( + this.plugin.settings.deepgramSecretId, + ); + secret.onChange(async (value) => { + await this.handleSecretSelection( + "deepgramSecretId", + value, + secret, + updateDeepgramDescription, ); + }); + return secret; + }); - if (diarization.provider === "deepgram") { - new Setting(diarizationContainer) - .setName("Deepgram API key") - .setDesc("Used only for Deepgram diarization. Create one at deepgram.com.") - .addText((text) => { - text.setPlaceholder("Enter your Deepgram API key") - .setValue(this.plugin.settings.diarizationApiKey) - .onChange(async (value) => { - this.plugin.settings.diarizationApiKey = value; - await this.plugin.saveSettings(); - }); - text.inputEl.type = "password"; - }); - } + const speakerSetting = new Setting(diarizationContainer) + .setName("Speaker label format") + .setDesc( + "Prefix added before each speaker's turn. Use {{speaker}} for the speaker label (OpenAI labels speakers A, B, …; Deepgram labels them 1, 2, …).", + ) + .addText((text) => + text + .setPlaceholder(DEFAULT_SPEAKER_TEMPLATE) + .setValue(diarization.speakerTemplate) + .onChange(async (value) => { + this.plugin.settings.transcript.diarization.speakerTemplate = value; + await this.plugin.saveSettings(); + }), + ); - new Setting(diarizationContainer) - .setName("Speaker label format") - .setDesc( - "Prefix added before each speaker's turn. Use {{speaker}} for the speaker label (OpenAI labels speakers A, B, …; Deepgram labels them 1, 2, …).", - ) - .addText((text) => - text - .setPlaceholder(DEFAULT_SPEAKER_TEMPLATE) - .setValue(diarization.speakerTemplate) - .onChange(async (value) => { - this.plugin.settings.transcript.diarization.speakerTemplate = value; - await this.plugin.saveSettings(); - }), - ); + updateVisibility = () => { + providerSetting.settingEl.toggle(diarization.enabled); + deepgramSetting.settingEl.toggle( + diarization.enabled && diarization.provider === "deepgram", + ); + speakerSetting.settingEl.toggle(diarization.enabled); }; - - renderDiarizationSettings(); + updateVisibility(); } } diff --git a/src/utility/ensureFolderExists.test.ts b/src/utility/ensureFolderExists.test.ts index 8b8714dc..e65b0dbc 100644 --- a/src/utility/ensureFolderExists.test.ts +++ b/src/utility/ensureFolderExists.test.ts @@ -2,6 +2,14 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { ensureFolderExists } from "./ensureFolderExists"; import { plugin } from "../store"; +function deferred() { + let resolve!: () => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + function setupApp(existing: string[] = []) { const present = new Set(existing); const created: string[] = []; @@ -120,4 +128,29 @@ describe("ensureFolderExists", () => { await expect(ensureFolderExists("Podcasts/My Show")).rejects.toThrow("EACCES"); }); + + it("stops between folder segments when the lifecycle guard fails", async () => { + const firstCreate = deferred(); + const created: string[] = []; + const createFolder = vi.fn(async (path: string) => { + created.push(path); + if (created.length === 1) await firstCreate.promise; + }); + const vault = { + getAbstractFileByPath: () => null, + createFolder, + } as unknown as Parameters[1]; + const lifecycleError = new DOMException("plugin unloaded", "AbortError"); + let active = true; + const guard = () => { + if (!active) throw lifecycleError; + }; + const pending = ensureFolderExists("Podcasts/My Show/Season 1", vault, guard); + await vi.waitFor(() => expect(createFolder).toHaveBeenCalledOnce()); + active = false; + firstCreate.resolve(); + + await expect(pending).rejects.toBe(lifecycleError); + expect(created).toEqual(["Podcasts"]); + }); }); diff --git a/src/utility/ensureFolderExists.ts b/src/utility/ensureFolderExists.ts index 33ebe990..afef7829 100644 --- a/src/utility/ensureFolderExists.ts +++ b/src/utility/ensureFolderExists.ts @@ -20,15 +20,20 @@ import { plugin } from "../store"; * `vault` defaults to the plugin's app vault; callers holding an injected vault * (e.g. a service given `plugin.app`) pass it so folder creation and the * subsequent file write target the same vault instance. + * + * `assertActive`, when supplied, runs around each asynchronous folder creation + * so a lifecycle-bound caller can stop before creating another path segment. */ export async function ensureFolderExists( folderPath: string, vault: Vault = get(plugin).app.vault, + assertActive?: () => void, ): Promise { const segments = folderPath.split("/").filter(Boolean); let current = ""; for (const segment of segments) { + assertActive?.(); current = current ? `${current}/${segment}` : segment; if (vault.getAbstractFileByPath(current)) { continue; @@ -37,6 +42,7 @@ export async function ensureFolderExists( try { await vault.createFolder(current); } catch (error) { + assertActive?.(); const alreadyExists = error instanceof Error && /already exists/i.test(error.message); // Re-check after a failed create: a case-insensitive filesystem or a // concurrent create can leave the folder present even though the @@ -46,5 +52,6 @@ export async function ensureFolderExists( throw error; } } + assertActive?.(); } } diff --git a/tests/e2e/podnotes-runtime.test.ts b/tests/e2e/podnotes-runtime.test.ts index 4110c4dc..58c2afa2 100644 --- a/tests/e2e/podnotes-runtime.test.ts +++ b/tests/e2e/podnotes-runtime.test.ts @@ -18,6 +18,8 @@ import { type PodNotesData = Partial & { schemaVersion?: number; + openAIApiKey?: string; + diarizationApiKey?: string; legacyExtension?: { enabled: boolean }; }; @@ -420,7 +422,7 @@ describe("PodNotes runtime", () => { expect(runtimeVolume).toBe(1); }); - test("migrates a legacy JSON date through note creation and the next durable save", async () => { + test("migrates legacy dates and credentials before runtime hydration", async () => { const { obsidian, plugin, sandbox } = getContext(); const audioPath = await seedAudio(sandbox, "legacy-date-episode.mp3"); const isoDate = "2024-03-01T10:05:03.000Z"; @@ -428,10 +430,19 @@ describe("PodNotes runtime", () => { ...createLocalEpisode("E2E Legacy Date Episode", audioPath), episodeDate: isoDate as unknown as Date, }; + await evalJsonAsync( + obsidian, + `(() => { + app.secretStorage.setSecret("podnotes-openai-api-key", "unrelated-openai-sentinel"); + app.secretStorage.setSecret("podnotes-deepgram-api-key", "unrelated-deepgram-sentinel"); + return true; + })()`, + ); await seedRuntimeData(plugin, sandbox, episode, { currentEpisode: episode, legacyData: true, + legacySecrets: { openAI: "e2e-openai-secret", deepgram: "e2e-deepgram-secret" }, note: { path: sandbox.path("legacy-date-note.md"), template: "date: {{date:YYYY-MM-DD}}\n", @@ -440,8 +451,37 @@ describe("PodNotes runtime", () => { await waitForPodNotesReady(obsidian); const beforeSave = await plugin.data().read(); - expect(beforeSave.schemaVersion).toBeUndefined(); + expect(beforeSave.schemaVersion).toBe(2); + expect(beforeSave.openAIApiKey).toBeUndefined(); + expect(beforeSave.diarizationApiKey).toBeUndefined(); + expect(beforeSave.openAISecretId).toMatch(/^podnotes-openai-api-key(?:-\d+)?$/); + expect(beforeSave.deepgramSecretId).toMatch(/^podnotes-deepgram-api-key(?:-\d+)?$/); + expect(beforeSave.openAISecretId).not.toBe("podnotes-openai-api-key"); + expect(beforeSave.deepgramSecretId).not.toBe("podnotes-deepgram-api-key"); expect(beforeSave.legacyExtension).toEqual({ enabled: true }); + const storedSecrets = await evalJsonAsync<{ + openAI: boolean; + deepgram: boolean; + openAISentinel: string | null; + deepgramSentinel: string | null; + }>( + obsidian, + `(() => { + const settings = app.plugins.plugins.${PLUGIN_ID}.settings; + return { + openAI: app.secretStorage.getSecret(settings.openAISecretId) === "e2e-openai-secret", + deepgram: app.secretStorage.getSecret(settings.deepgramSecretId) === "e2e-deepgram-secret", + openAISentinel: app.secretStorage.getSecret("podnotes-openai-api-key"), + deepgramSentinel: app.secretStorage.getSecret("podnotes-deepgram-api-key"), + }; + })()`, + ); + expect(storedSecrets).toEqual({ + openAI: true, + deepgram: true, + openAISentinel: "unrelated-openai-sentinel", + deepgramSentinel: "unrelated-deepgram-sentinel", + }); const runtimeDate = await evalJsonAsync<{ isDate: boolean; iso: string }>( obsidian, `(() => { @@ -461,11 +501,25 @@ describe("PodNotes runtime", () => { await setVolume(obsidian, 0.42); const persisted = await plugin.waitForData( - (data) => data.schemaVersion === 1 && data.defaultVolume === 0.42, + (data) => data.schemaVersion === 2 && data.defaultVolume === 0.42, WAIT_OPTS, ); expect(persisted.legacyExtension).toEqual({ enabled: true }); expect(persisted.currentEpisode?.episodeDate).toBe(isoDate); + const idsBeforeReload = await obsidian.dev.evalJson( + 'app.secretStorage.listSecrets().filter((id) => id.startsWith("podnotes-openai-api-key") || id.startsWith("podnotes-deepgram-api-key")).sort()', + ); + await plugin.reload(RELOAD_OPTIONS); + await waitForPodNotesReady(obsidian); + const afterReload = await plugin.data().read(); + const idsAfterReload = await obsidian.dev.evalJson( + 'app.secretStorage.listSecrets().filter((id) => id.startsWith("podnotes-openai-api-key") || id.startsWith("podnotes-deepgram-api-key")).sort()', + ); + expect(idsAfterReload).toEqual(idsBeforeReload); + expect(afterReload.openAISecretId).toBe(beforeSave.openAISecretId); + expect(afterReload.deepgramSecretId).toBe(beforeSave.deepgramSecretId); + expect(afterReload.openAIApiKey).toBeUndefined(); + expect(afterReload.diarizationApiKey).toBeUndefined(); expect(await obsidian.dev.runtimeErrors()).toEqual([]); }); @@ -585,7 +639,7 @@ describe("PodNotes runtime", () => { const original = await plugin.data().read(); const future = { ...original, - schemaVersion: 2, + schemaVersion: 3, legacyExtension: { enabled: true }, }; @@ -607,14 +661,18 @@ describe("PodNotes runtime", () => { } finally { await plugin.data().write(original); // A failed onload can leave Obsidian's enabled flag set without a live - // plugin instance. Await the plugin manager directly so cleanup cannot - // return before the restored onload finishes. + // plugin instance. Start the restored enable operation without awaiting + // its manager promise: after a rejected onload Obsidian can leave that + // promise pending even though the plugin becomes live. The readiness poll + // below is the authoritative completion signal. await evalJsonAsync( obsidian, `(async () => { await app.plugins.disablePlugin(${JSON.stringify(PLUGIN_ID)}); - await app.plugins.enablePlugin(${JSON.stringify(PLUGIN_ID)}); - return Boolean(app.plugins.plugins.${PLUGIN_ID}); + void app.plugins.enablePlugin(${JSON.stringify(PLUGIN_ID)}).catch((error) => { + console.error("PodNotes E2E: restored enable failed", error); + }); + return true; })()`, ); await waitForPodNotesReady(obsidian); @@ -710,6 +768,7 @@ async function seedRuntimeData( played?: { duration: number; time: number }; timestampTemplate?: string; legacyData?: boolean; + legacySecrets?: { openAI?: string; deepgram?: string }; } = {}, ): Promise { const placeholderEpisode = createLocalEpisode( @@ -726,6 +785,8 @@ async function seedRuntimeData( if (options.legacyData) { delete data.schemaVersion; data.legacyExtension = { enabled: true }; + data.openAIApiKey = options.legacySecrets?.openAI; + data.diarizationApiKey = options.legacySecrets?.deepgram; } data.currentEpisode = options.currentEpisode ?? placeholderEpisode; data.defaultPlaybackRate = options.defaultPlaybackRate ?? 1; diff --git a/tests/mocks/obsidian.ts b/tests/mocks/obsidian.ts index 71608622..1e92c233 100644 --- a/tests/mocks/obsidian.ts +++ b/tests/mocks/obsidian.ts @@ -1,4 +1,22 @@ -export class App {} +export class SecretStorage { + private values = new Map(); + + setSecret(id: string, secret: string): void { + this.values.set(id, secret); + } + + getSecret(id: string): string | null { + return this.values.get(id) ?? null; + } + + listSecrets(): string[] { + return [...this.values.keys()]; + } +} + +export class App { + secretStorage = new SecretStorage(); +} export class Plugin {} export class Component { app?: App; @@ -229,6 +247,15 @@ export class TextComponent extends BaseInteractiveElement { } } +export class SecretComponent extends TextComponent { + constructor( + public app: App, + container: HTMLElement, + ) { + super(container); + } +} + export class Setting { settingEl: HTMLElement; @@ -268,6 +295,11 @@ export class Setting { callback(new ButtonComponent(this.settingEl)); return this; } + + addComponent(callback: (container: HTMLElement) => T) { + callback(this.settingEl); + return this; + } } export class PluginSettingTab {