diff --git a/src/components/modals/ResetSettingsConfirmModal.tsx b/src/components/modals/ResetSettingsConfirmModal.tsx index 32c065c6..df42300b 100644 --- a/src/components/modals/ResetSettingsConfirmModal.tsx +++ b/src/components/modals/ResetSettingsConfirmModal.tsx @@ -6,7 +6,9 @@ export class ResetSettingsConfirmModal extends ConfirmModal { super( app, onConfirm, - "Resetting settings will clear all settings and restore the default values. You will lose any custom settings you have made including the API keys. Are you sure you want to continue?", + "Resetting settings will clear all settings and restore the default values. " + + 'API keys are not cleared by this action — use "Delete All Keys" in Advanced Settings ' + + "→ API Key Storage if you also want to remove them. Are you sure you want to continue?", "Reset Settings" ); } diff --git a/src/main.ts b/src/main.ts index 90de2034..884e87d7 100644 --- a/src/main.ts +++ b/src/main.ts @@ -28,6 +28,7 @@ import { persistSettings, loadSettingsWithKeychain, flushPersistence, + resetPersistenceState, } from "@/services/settingsPersistence"; import { UserMemoryManager } from "@/memory/UserMemoryManager"; import { clearRecordedPromptPayload } from "@/LLMProviders/chainRunner/utils/promptPayloadRecorder"; @@ -105,6 +106,16 @@ export default class CopilotPlugin extends Plugin { private webSelectionTracker?: WebSelectionTracker; private readonly chatHistoryLastAccessedAtManager = new RecentUsageManager(); async onload(): Promise { + // Reason: clear stale module-level persistence state + KeychainService + // singleton left over from a previous plugin lifecycle in the same + // process (disable→enable, dev hot reload, "Open another vault" without + // restart). Doing this at the START of onload (instead of at the end of + // onunload) avoids a race: onunload is fire-and-forget from Obsidian's + // perspective, so its `await flushPersistence()` continuation can fire + // AFTER the next onload has already initialized — and would then null + // out the new instance, breaking saves until another full reload. + resetPersistenceState(); + KeychainService.resetInstance(); KeychainService.getInstance(this.app); await this.loadSettings(); this.settingsUnsubscriber = subscribeToSettingsChange((prev, next) => { @@ -260,6 +271,9 @@ export default class CopilotPlugin extends Plugin { // Best-effort flush of pending keychain/data.json writes. // Reason: onunload() is void in Obsidian's type system, but awaiting here // is no worse than fire-and-forget, and consistent with the log flush below. + // (Module-level state + KeychainService singleton reset happen at the + // START of the next onload, not here — see comment in onload above for + // the late-write race that motivated the move.) await flushPersistence(); // Clear all persistent selection highlights before unload diff --git a/src/services/settingsPersistence.ts b/src/services/settingsPersistence.ts index 3ff22982..be82afbf 100644 --- a/src/services/settingsPersistence.ts +++ b/src/services/settingsPersistence.ts @@ -114,6 +114,27 @@ export function refreshDiskHasSecrets(data: CopilotSettings): void { diskHasSecrets = hasPersistedSecrets(data as unknown as Record); } +/** + * Reset all module-level persistence state. + * + * Reason: module-level state (lastPersistedSettings, transactionEpoch, + * pendingTombstones, persistHadUndecryptableSecrets, diskHasSecrets, + * suppressNextPersist, writeQueue) survives `onunload` because Node / + * Electron's require cache keeps the module instance alive across plugin + * disable→enable. After a mid-migration disable, stale state would poison + * the next session. Similarly, switching vaults via "Open another vault" + * carries stale state across vaults. Call this from `onunload`. + */ +export function resetPersistenceState(): void { + writeQueue = Promise.resolve(); + diskHasSecrets = false; + lastPersistedSettings = undefined; + suppressNextPersist = false; + transactionEpoch = 0; + pendingTombstones.clear(); + persistHadUndecryptableSecrets = false; +} + /** * Refresh the last known-good settings baseline used by keychain rollback. * Called by dedicated transactions that bypass `doPersist()`. diff --git a/src/settings/v2/components/AdvancedSettings.tsx b/src/settings/v2/components/AdvancedSettings.tsx index 9b347d38..44453538 100644 --- a/src/settings/v2/components/AdvancedSettings.tsx +++ b/src/settings/v2/components/AdvancedSettings.tsx @@ -65,8 +65,7 @@ export const AdvancedSettings: React.FC = () => { // current in-memory `settings` presence so the Migrate / Delete CTAs // appear immediately. Matches the gate used in `canClearDiskSecrets()`. const diskHasSecrets = - hasDiskSecretsToMigrate() || - (!keychainOnly && hasPersistedSecrets(settings)); + hasDiskSecretsToMigrate() || (!keychainOnly && hasPersistedSecrets(settings)); const canMigrate = canClearDiskSecrets(settings); // Reason: a single status string is easier to reason about and debug than @@ -89,6 +88,14 @@ export const AdvancedSettings: React.FC = () => { ? "blocked" : "standard"; + // Reason: keychain-only mode + keychain available but this device has no + // secrets persisted yet. Hits three real scenarios: (1) desktop migrated + // then Sync shipped a stripped data.json to mobile; (2) backup restore of a + // _keychainOnly vault to a new device; (3) synced vault opened on a third + // device for the first time. Without surfacing this, the green "active" + // pill misleads the user into thinking everything is fine. + const keychainAppearsEmpty = storageStatus === "active" && !hasPersistedSecrets(settings); + // Check if the default system prompt exists in the current prompts list const defaultPromptExists = prompts.some( (prompt) => prompt.title === settings.defaultSystemPromptTitle @@ -319,10 +326,19 @@ export const AdvancedSettings: React.FC = () => { Obsidian to 1.11.4+, or re-enter keys on a supported device. ) : storageStatus === "active" ? ( - <> - API keys are stored in this device's{" "} - Obsidian Keychain. - + keychainAppearsEmpty ? ( + + No API keys found in this device's{" "} + Obsidian Keychain. + Re-enter your API keys in the relevant settings sections — this device's + keychain is separate from other devices. + + ) : ( + <> + API keys are stored in this device's{" "} + Obsidian Keychain. + + ) ) : ( <> Your API keys are stored in data.json. Move them to the{" "}