fix(keychain): empty-keychain banner, reset modal copy, lifecycle reset (followup to #2364) (#2455)

* fix(keychain): empty-keychain banner, reset modal copy, lifecycle reset (followup to #2364)

Three issues flagged during inline review of #2364, applied as a
followup. Each addresses a concrete user-facing risk and is mechanical.

1. **Empty-keychain banner** (AdvancedSettings.tsx)

   The five-state storage status machine didn't distinguish "keychain-
   only mode, keychain available, but this device's keychain is empty".
   The result was a green "Obsidian Keychain" pill and no banner — even
   though the user has no usable keys.

   Hits three real scenarios:
   - Desktop migrates → Sync ships stripped data.json to mobile →
     mobile keychain is empty.
   - User restores a vault backup with `_keychainOnly: true` to a new
     device.
   - User opens a synced vault on a third device for the first time.

   Fix: new `keychainAppearsEmpty` derived state. When true, the API
   Key Storage description renders a warning ("No API keys found in
   this device's Obsidian Keychain — re-enter your API keys… this
   device's keychain is separate from other devices.") instead of the
   default "stored in this device's Obsidian Keychain" message.

2. **Reset Settings modal copy** (ResetSettingsConfirmModal.tsx)

   The modal told users that resetting would "lose any custom settings
   you have made including the API keys" — but in keychain-only mode,
   `resetSettings()` deliberately does not touch the keychain (per the
   DESIGN NOTE in model.ts). After reset, keys re-hydrate from the
   keychain on next load, leading to "ghost key" reports and a privacy
   expectation mismatch.

   Fix: rewrite the copy so the user knows API keys are *not* cleared
   and points them at "Delete All Keys" in Advanced Settings → API Key
   Storage if they want the dedicated path.

3. **Module-level state lifecycle** (settingsPersistence.ts, main.ts)

   `writeQueue`, `diskHasSecrets`, `lastPersistedSettings`,
   `suppressNextPersist`, `transactionEpoch`, `pendingTombstones`,
   `persistHadUndecryptableSecrets` are all module singletons. Plugin
   disable→enable doesn't reset them (Node / Electron require cache
   keeps the module instance alive), so a mid-migration disable
   poisons the next session with stale epoch + tombstone state.

   Switching vaults via "Open another vault" without restarting also
   inherits stale baseline state until the next save refreshes it.

   Fix: new exported `resetPersistenceState()` clears every module-
   level variable. Called from `main.ts onunload()` alongside the
   existing `KeychainService.resetInstance()` static method. Cheap
   insurance, no behavioral change for the happy path.

Verification:
- npm run lint: pre-existing warnings only
- npm test: 116 suites / 2105 tests pass
- npm run build: succeeds
- Manual mobile test confirmed (empty-keychain banner appears on
  freshly-synced device; reset copy reads correctly).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(keychain): move state reset from onunload to onload (addresses Codex P1)

Codex review caught a real race: onunload is fire-and-forget from
Obsidian's perspective (the method is typed void), so the `await
flushPersistence()` continuation can run AFTER the next onload has
already initialized a fresh KeychainService instance. Our late reset
would then null out the *new* instance, breaking saves and the settings
UI until another full reload.

Same intent (next session starts from clean state), no race: move the
reset to the START of onload instead.

- onload: resetPersistenceState() + KeychainService.resetInstance()
  fire before KeychainService.getInstance(this.app), so the next
  session always starts with a clean slate regardless of what the
  previous onunload's tail did.
- onunload: keep the flushPersistence() but drop the reset calls;
  inline comment explains the move and points back to onload.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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"
     );
   }

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<string>();
   async onload(): Promise<void> {
+    // 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

src/services/settingsPersistence.ts

@@ -114,6 +114,27 @@ export function refreshDiskHasSecrets(data: CopilotSettings): void {
   diskHasSecrets = hasPersistedSecrets(data as unknown as Record<string, unknown>);
 }
 
+/**
+ * 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()`.

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 <code>1.11.4+</code>, or re-enter keys on a supported device.
               </>
             ) : storageStatus === "active" ? (
-              <>
-                API keys are stored in this device&apos;s{" "}
-                <strong className="tw-font-semibold tw-text-normal">Obsidian Keychain</strong>.
-              </>
+              keychainAppearsEmpty ? (
+                <span className="tw-text-warning">
+                  No API keys found in this device&apos;s{" "}
+                  <strong className="tw-font-semibold tw-text-normal">Obsidian Keychain</strong>.
+                  Re-enter your API keys in the relevant settings sections — this device&apos;s
+                  keychain is separate from other devices.
+                </span>
+              ) : (
+                <>
+                  API keys are stored in this device&apos;s{" "}
+                  <strong className="tw-font-semibold tw-text-normal">Obsidian Keychain</strong>.
+                </>
+              )
             ) : (
               <>
                 Your API keys are stored in <code>data.json</code>. Move them to the{" "}