fix: prevent subagent keychain migration churn (#869)
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
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<string | null> {
|
||||
* Store refresh token in system secrets
|
||||
*/
|
||||
export async function setRefreshToken(refreshToken: string): Promise<void> {
|
||||
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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user