From 44827bdb8e42ce8f3e09ce876ea43827734227ec Mon Sep 17 00:00:00 2001 From: Charles Packer Date: Sun, 8 Feb 2026 22:26:47 -0800 Subject: [PATCH] fix: prevent subagent keychain migration churn (#869) Co-authored-by: Letta --- src/settings-manager.ts | 14 +++-- src/tests/settings-manager.test.ts | 36 +++++++++++++ src/utils/secrets.ts | 85 +++++++++++++++++++----------- 3 files changed, 102 insertions(+), 33 deletions(-) diff --git a/src/settings-manager.ts b/src/settings-manager.ts index e24d32a..ccc758a 100644 --- a/src/settings-manager.ts +++ b/src/settings-manager.ts @@ -109,6 +109,10 @@ const DEFAULT_LOCAL_PROJECT_SETTINGS: LocalProjectSettings = { const DEFAULT_LETTA_API_URL = "https://api.letta.com"; +function isSubagentProcess(): boolean { + return process.env.LETTA_CODE_AGENT_ROLE === "subagent"; +} + /** * Normalize a base URL for use as a settings key. * Strips protocol (https://, http://) and returns host:port. @@ -173,8 +177,10 @@ class SettingsManager { // Check secrets availability and warn if not available await this.checkSecretsSupport(); - // Migrate tokens to secrets if they exist in settings - await this.migrateTokensToSecrets(); + // Migrate tokens to secrets if they exist in settings (parent process only) + if (!isSubagentProcess()) { + await this.migrateTokensToSecrets(); + } // Migrate pinnedAgents/pinnedAgentsByServer to agents array this.migrateToAgentsArray(); @@ -185,7 +191,9 @@ class SettingsManager { // Still check secrets support and try to migrate in case of partial failure await this.checkSecretsSupport(); - await this.migrateTokensToSecrets(); + if (!isSubagentProcess()) { + await this.migrateTokensToSecrets(); + } this.migrateToAgentsArray(); } } diff --git a/src/tests/settings-manager.test.ts b/src/tests/settings-manager.test.ts index c51e9df..c9eb9ff 100644 --- a/src/tests/settings-manager.test.ts +++ b/src/tests/settings-manager.test.ts @@ -785,6 +785,42 @@ describe("Settings Manager - Edge Cases", () => { // ============================================================================ describe("Settings Manager - Agents Array Migration", () => { + const originalSubagentRole = process.env.LETTA_CODE_AGENT_ROLE; + + afterEach(() => { + if (originalSubagentRole === undefined) { + delete process.env.LETTA_CODE_AGENT_ROLE; + } else { + process.env.LETTA_CODE_AGENT_ROLE = originalSubagentRole; + } + }); + + test.skipIf(!keychainAvailablePrecompute)( + "Subagent process skips token migration to secrets", + async () => { + const { writeFile, mkdir } = await import("../utils/fs.js"); + const settingsDir = join(testHomeDir, ".letta"); + await mkdir(settingsDir, { recursive: true }); + await writeFile( + join(settingsDir, "settings.json"), + JSON.stringify({ + refreshToken: "rt-subagent-should-stay", + env: { + LETTA_API_KEY: "sk-subagent-should-stay", + }, + }), + ); + + process.env.LETTA_CODE_AGENT_ROLE = "subagent"; + + await settingsManager.initialize(); + const settings = settingsManager.getSettings(); + + expect(settings.refreshToken).toBe("rt-subagent-should-stay"); + expect(settings.env?.LETTA_API_KEY).toBe("sk-subagent-should-stay"); + }, + ); + test("Migrates from pinnedAgents (oldest legacy format)", async () => { // Setup: Write old format to disk const { writeFile, mkdir } = await import("../utils/fs.js"); diff --git a/src/utils/secrets.ts b/src/utils/secrets.ts index 3d50a60..9af23b6 100644 --- a/src/utils/secrets.ts +++ b/src/utils/secrets.ts @@ -18,6 +18,53 @@ let SERVICE_NAME = "letta-code"; const API_KEY_NAME = "letta-api-key"; const REFRESH_TOKEN_NAME = "letta-refresh-token"; +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function isDuplicateKeychainItemError(error: unknown): boolean { + const message = getErrorMessage(error); + return ( + message.includes("already exists in the keychain") || + message.includes("code: -25299") + ); +} + +async function setSecretValue(name: string, value: string): Promise { + if (!secretsAvailable) { + throw new Error("Secrets API unavailable"); + } + + try { + await secrets.set({ + service: SERVICE_NAME, + name, + value, + }); + return; + } catch (error) { + if (!isDuplicateKeychainItemError(error)) { + throw error; + } + } + + // Replace existing keychain item and retry once. + try { + await secrets.delete({ + service: SERVICE_NAME, + name, + }); + } catch { + // Ignore delete errors and retry set below. + } + + await secrets.set({ + service: SERVICE_NAME, + name, + value, + }); +} + /** * Override the keychain service name (useful for tests to avoid touching real credentials) */ @@ -38,23 +85,12 @@ export interface SecureTokens { * Store API key in system secrets */ export async function setApiKey(apiKey: string): Promise { - if (secretsAvailable) { - try { - await secrets.set({ - service: SERVICE_NAME, - name: API_KEY_NAME, - value: apiKey, - }); - return; - } catch (error) { - console.warn( - `Failed to store API key in secrets, using fallback: ${error}`, - ); - } + if (!secretsAvailable) { + // When secrets unavailable, let the settings manager handle fallback + throw new Error("Secrets API unavailable"); } - // When secrets unavailable, let the settings manager handle fallback - throw new Error("Secrets API unavailable"); + await setSecretValue(API_KEY_NAME, apiKey); } /** @@ -80,23 +116,12 @@ export async function getApiKey(): Promise { * Store refresh token in system secrets */ export async function setRefreshToken(refreshToken: string): Promise { - if (secretsAvailable) { - try { - await secrets.set({ - service: SERVICE_NAME, - name: REFRESH_TOKEN_NAME, - value: refreshToken, - }); - return; - } catch (error) { - console.warn( - `Failed to store refresh token in secrets, using fallback: ${error}`, - ); - } + if (!secretsAvailable) { + // When secrets unavailable, let the settings manager handle fallback + throw new Error("Secrets API unavailable"); } - // When secrets unavailable, let the settings manager handle fallback - throw new Error("Secrets API unavailable"); + await setSecretValue(REFRESH_TOKEN_NAME, refreshToken); } /**