fix: preserve keychain read failures

docs/implementation/auth.md

@@ -63,7 +63,9 @@ To clear tokens manually, use `githits logout`. This removes stored tokens for t
 
 ## Storage
 
-Credentials are stored in the **system keychain** by default (macOS Keychain, Windows Credential Manager, Linux Secret Service) via `@napi-rs/keyring`. The CLI does not silently downgrade OAuth credentials to plaintext files when the keychain is unavailable.
+Credentials are stored in the **system keychain** by default (macOS Keychain, Windows Credential Manager, Linux Secret Service) via `@napi-rs/keyring`. A missing entry is reported as unauthenticated, while a keychain access failure is surfaced as a storage error. The CLI does not silently downgrade OAuth credentials to plaintext files when the keychain is unavailable.
+
+The currently locked `@napi-rs/keyring@1.3.0` cannot yet preserve that distinction for reads because it converts both missing entries and platform failures to `null`. The upstream fix is tracked in [keyring-node#136](https://github.com/Brooooooklyn/keyring-node/pull/136); the dependency must be updated after a fixed release is published.
 
 Machines without a usable keychain can explicitly opt into plaintext OAuth storage with `auth.storage = "file"` in `config.toml` or `GITHITS_AUTH_STORAGE=file`. `GITHITS_API_TOKEN` remains the preferred automation/CI path because it avoids storing OAuth refresh credentials.
 
@@ -127,7 +129,8 @@ Keychain mode:
 
 1. Check keychain — if found, return it
 2. Do not inspect plaintext file storage or legacy plaintext paths
-3. Keychain empty or unavailable — return null
+3. If the keychain has no matching entry, return null
+4. If the keychain cannot be accessed, fail with storage guidance instead of treating the credential as missing
 
 File mode:
 
@@ -188,7 +191,7 @@ The MCP server starts without a synchronous auth gate. Tool calls resolve tokens
 - **Token refresh fails silently** — The token manager first reloads storage in case another process refreshed credentials. Transient failures retain unchanged refresh credentials and later calls retry. Only classified terminal failures clear the stale token from the active backend and require login again.
 - **Logged out after running in a different storage mode** — Credentials saved under one `auth.storage` mode are invisible to the other (keychain and file modes do not cross-inspect). An inconsistent `GITHITS_AUTH_STORAGE` across contexts (e.g. set for the MCP server but not the interactive shell) looks like a logout. Automatic clears no longer compound this by wiping the other mode's credentials, but the fix for the phantom logout is to make the mode consistent everywhere (prefer `auth.storage` in `config.toml` over the env var) and `githits login --force` once.
 - **Clearing auth** — Run `githits logout` to remove stored tokens and client registration for the current environment.
-- **System keychain unavailable** — In default keychain mode, OAuth login/refresh fails rather than writing plaintext credentials. Use `GITHITS_API_TOKEN`, fix/unlock the keychain, or explicitly configure `auth.storage = "file"` if plaintext local storage is acceptable.
+- **System keychain unavailable** — In default keychain mode, reads and writes are designed to fail with storage guidance rather than treating credentials as missing or writing plaintext credentials. Read failures require the pending upstream `@napi-rs/keyring` fix described above. Use `GITHITS_API_TOKEN`, fix/unlock the keychain, or explicitly configure `auth.storage = "file"` if plaintext local storage is acceptable.
 - **Windows "password encoded as UTF-16 is longer than platform limit"** — The Windows Credential Manager limits credential blobs to 2560 bytes (`CRED_MAX_CREDENTIAL_BLOB_SIZE`). Since passwords are stored as UTF-16 (2 bytes per char), the effective limit is 1280 characters. The `ChunkingKeyringService` decorator handles this automatically by splitting large values across multiple entries. If this error occurs on an older CLI version, upgrade to get chunked storage support.
 
 ## Diagnostics

src/services/migrating-auth-storage.test.ts

@@ -146,7 +146,7 @@ describe("MigratingAuthStorage", () => {
     expect(legacy.clearTokens).not.toHaveBeenCalled();
   });
 
-  it("keychain mode returns null when keychain is unavailable instead of falling back to plaintext", async () => {
+  it("keychain mode exposes unavailable token storage without falling back to plaintext", async () => {
     const token = createValidTokenData();
     const primary = createMockAuthStorage({
       loadTokens: mock(() =>
@@ -156,12 +156,27 @@ describe("MigratingAuthStorage", () => {
     const file = createMockAuthStorage({
       loadTokens: mock(() => Promise.resolve(token)),
     });
-    const legacy = createMockAuthStorage();
-    const storage = new MigratingAuthStorage(primary, file, legacy, "keychain");
+    const legacy = createMockAuthStorage({
+      loadTokens: mock(() => Promise.resolve(token)),
+    });
+    const storage = new MigratingAuthStorage(
+      primary,
+      file,
+      legacy,
+      "keychain",
+      "/home/test/.config/githits/config.toml",
+    );
 
-    await expect(storage.loadTokens(BASE_URL)).resolves.toBeNull();
+    await expect(storage.loadTokens(BASE_URL)).rejects.toBeInstanceOf(
+      AuthStoragePolicyError,
+    );
+    await expect(storage.loadTokens(BASE_URL)).rejects.toThrow(
+      /System keychain is unavailable/,
+    );
     expect(file.loadTokens).not.toHaveBeenCalled();
+    expect(legacy.loadTokens).not.toHaveBeenCalled();
     expect(file.clearTokens).not.toHaveBeenCalled();
+    expect(legacy.clearTokens).not.toHaveBeenCalled();
   });
 
   it("keychain mode save fails instead of writing plaintext when keychain is unavailable", async () => {
@@ -544,6 +559,33 @@ describe("MigratingAuthStorage", () => {
     expect(legacy.clearClient).not.toHaveBeenCalled();
   });
 
+  it("keychain mode exposes unavailable client storage without falling back to plaintext", async () => {
+    const primary = createMockAuthStorage({
+      loadClient: mock(() =>
+        Promise.reject(new KeychainUnavailableError("keychain locked")),
+      ),
+    });
+    const file = createMockAuthStorage({
+      loadClient: mock(() => Promise.resolve(defaultClientRegistration)),
+    });
+    const legacy = createMockAuthStorage({
+      loadClient: mock(() => Promise.resolve(defaultClientRegistration)),
+    });
+    const storage = new MigratingAuthStorage(
+      primary,
+      file,
+      legacy,
+      "keychain",
+      "/home/test/.config/githits/config.toml",
+    );
+
+    await expect(storage.loadClient(BASE_URL)).rejects.toBeInstanceOf(
+      AuthStoragePolicyError,
+    );
+    expect(file.loadClient).not.toHaveBeenCalled();
+    expect(legacy.loadClient).not.toHaveBeenCalled();
+  });
+
   it("migrates clients according to configured mode", async () => {
     const primary = createMockAuthStorage();
     const file = createMockAuthStorage();
@@ -895,7 +937,7 @@ describe("MigratingAuthStorage", () => {
       expect(metadata.clear).not.toHaveBeenCalled();
     });
 
-    it("returns false without throwing when the keychain is unavailable", async () => {
+    it("exposes unavailable keychain storage before clearing tokens", async () => {
       const token = createValidTokenData();
       const primary = createMockAuthStorage({
         loadTokens: mock(() =>
@@ -911,7 +953,7 @@ describe("MigratingAuthStorage", () => {
 
       await expect(
         storage.clearActiveTokensIfUnchanged(BASE_URL, token),
-      ).resolves.toBe(false);
+      ).rejects.toBeInstanceOf(AuthStoragePolicyError);
       expect(primary.clearTokens).not.toHaveBeenCalled();
     });
 

src/services/migrating-auth-storage.ts

@@ -243,7 +243,7 @@ export class MigratingAuthStorage implements AuthStorage {
         return primaryTokens;
       }
     } catch (error) {
-      if (!(error instanceof KeychainUnavailableError)) throw error;
+      throw this.toPolicyError(error);
     }
     return null;
   }
@@ -276,7 +276,7 @@ export class MigratingAuthStorage implements AuthStorage {
       const primaryClient = await this.primary.loadClient(baseUrl);
       if (primaryClient) return primaryClient;
     } catch (error) {
-      if (!(error instanceof KeychainUnavailableError)) throw error;
+      throw this.toPolicyError(error);
     }
     return null;
   }
@@ -428,8 +428,7 @@ export class MigratingAuthStorage implements AuthStorage {
     try {
       return await this.primary.loadTokens(baseUrl);
     } catch (error) {
-      if (error instanceof KeychainUnavailableError) return null;
-      throw error;
+      throw this.toPolicyError(error);
     }
   }