diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fd23c6e89..f43cdce46 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/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)). diff --git a/packages/coding-agent/src/core/auth-storage.ts b/packages/coding-agent/src/core/auth-storage.ts index 31a0ec9c3..725b6f3d7 100644 --- a/packages/coding-agent/src/core/auth-storage.ts +++ b/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.persistProviderChange(provider, credential); + this.data = this.persistProviderChange(provider, credential); } /** * Remove credential for a provider. */ remove(provider: string): void { - delete this.data[provider]; - this.persistProviderChange(provider, undefined); + this.data = this.persistProviderChange(provider, undefined); } /** diff --git a/packages/coding-agent/test/auth-storage.test.ts b/packages/coding-agent/test/auth-storage.test.ts index 651f9a177..62ecd81a5 100644 --- a/packages/coding-agent/test/auth-storage.test.ts +++ b/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; + expect(updated["github-copilot"].key).toBe("copilot-key"); + expect(authStorage.has("github-copilot")).toBe(true); }); test("reload records parse errors and drainErrors clears buffer", () => {