fix(coding-agent): surface auth storage save failures

closes #6223

packages/coding-agent/CHANGELOG.md

@@ -9,6 +9,7 @@
 
 ### Fixed
 
+- Fixed `/login` to report auth storage persistence failures instead of claiming credentials were saved when `auth.json` is locked ([#6223](https://github.com/earendil-works/pi/issues/6223)).
 - Fixed split-turn compaction to serialize summary requests so single-concurrency local providers do not fail with 429 errors ([#5536](https://github.com/earendil-works/pi/issues/5536)).
 - Fixed custom session entries appended during assistant streaming to render before the live assistant message, matching persisted session order.
 - Fixed oversized bash tool timeouts to fail with a clear validation error instead of being clamped to an immediate timeout ([#6181](https://github.com/earendil-works/pi/issues/6181)).

packages/coding-agent/src/core/auth-storage.ts

@@ -271,12 +271,21 @@ export class AuthStorage {
 		}
 	}
 
-	private persistProviderChange(provider: string, credential: AuthCredential | undefined): void {
+	private persistProviderChange(provider: string, credential: AuthCredential | undefined): AuthStorageData {
 		if (this.loadError) {
-			return;
+			this.reload();
+		}
+
+		if (this.loadError) {
+			const error = new Error(
+				`Cannot update auth storage because it could not be loaded: ${this.loadError.message}`,
+			);
+			this.recordError(error);
+			throw error;
 		}
 
 		try {
+			let persistedData: AuthStorageData = {};
 			this.storage.withLock((current) => {
 				const currentData = this.parseStorageData(current);
 				const merged: AuthStorageData = { ...currentData };
@@ -285,10 +294,14 @@ export class AuthStorage {
 				} else {
 					delete merged[provider];
 				}
+				persistedData = merged;
 				return { result: undefined, next: JSON.stringify(merged, null, 2) };
 			});
+			this.loadError = null;
+			return persistedData;
 		} catch (error) {
 			this.recordError(error);
+			throw error;
 		}
 	}
 
@@ -311,16 +324,14 @@ export class AuthStorage {
 	 * Set credential for a provider.
 	 */
 	set(provider: string, credential: AuthCredential): void {
-		this.data[provider] = credential;
+		this.data = this.persistProviderChange(provider, credential);
-		this.persistProviderChange(provider, credential);
 	}
 
 	/**
 	 * Remove credential for a provider.
 	 */
 	remove(provider: string): void {
-		delete this.data[provider];
+		this.data = this.persistProviderChange(provider, undefined);
-		this.persistProviderChange(provider, undefined);
 	}
 
 	/**

packages/coding-agent/test/auth-storage.test.ts

@@ -581,7 +581,7 @@ describe("AuthStorage", () => {
 			expect(updated.google.key).toBe("google-key");
 		});
 
-		test("does not overwrite malformed auth file after load error", () => {
+		test("throws and does not overwrite malformed auth file after load error", () => {
 			writeAuthJson({
 				anthropic: { type: "api_key", key: "anthropic-key" },
 			});
@@ -590,10 +590,40 @@ describe("AuthStorage", () => {
 			writeFileSync(authJsonPath, "{invalid-json", "utf-8");
 
 			authStorage.reload();
-			authStorage.set("openai", { type: "api_key", key: "openai-key" });
+			expect(() => authStorage.set("openai", { type: "api_key", key: "openai-key" })).toThrow(
+				"Cannot update auth storage because it could not be loaded",
+			);
 
 			const raw = readFileSync(authJsonPath, "utf-8");
 			expect(raw).toBe("{invalid-json");
+			expect(authStorage.has("openai")).toBe(false);
+		});
+
+		test("throws when a stale auth lock prevents persistence", () => {
+			writeAuthJson({});
+			writeFileSync(`${authJsonPath}.lock`, "", "utf-8");
+
+			authStorage = AuthStorage.create(authJsonPath);
+			expect(() => authStorage.set("github-copilot", { type: "api_key", key: "copilot-key" })).toThrow(
+				"Cannot update auth storage because it could not be loaded",
+			);
+
+			expect(readFileSync(authJsonPath, "utf-8")).toBe("{}");
+			expect(authStorage.has("github-copilot")).toBe(false);
+		});
+
+		test("recovers from an earlier load error before persisting", () => {
+			writeAuthJson({});
+			const lockPath = `${authJsonPath}.lock`;
+			writeFileSync(lockPath, "", "utf-8");
+
+			authStorage = AuthStorage.create(authJsonPath);
+			rmSync(lockPath);
+			authStorage.set("github-copilot", { type: "api_key", key: "copilot-key" });
+
+			const updated = JSON.parse(readFileSync(authJsonPath, "utf-8")) as Record<string, { key: string }>;
+			expect(updated["github-copilot"].key).toBe("copilot-key");
+			expect(authStorage.has("github-copilot")).toBe(true);
 		});
 
 		test("reload records parse errors and drainErrors clears buffer", () => {