Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions apps/desktop/src/main/ai/providers/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,59 @@ function isOAuthToken(token: string | undefined): boolean {
return token.startsWith('sk-ant-oa') || token.startsWith('sk-ant-ort');
}

// =============================================================================
// Anthropic OAuth System Prompt Injection
// =============================================================================

/**
* The Claude Code identity string that Anthropic's API requires as the first
* system block for OAuth tokens to access Opus/Sonnet models.
* Must be a SEPARATE system block (not combined with other text).
*/
const CLAUDE_CODE_IDENTITY = "You are Claude Code, Anthropic's official CLI for Claude.";

/**
* Creates a fetch interceptor that injects the Claude Code identity as a
* separate first system block in Anthropic API requests.
*
* Anthropic's API validates that OAuth tokens include this identity as an
* exact-match first system block to access Opus/Sonnet models.
* The identity must be a separate `{type: "text", text: "..."}` entry —
* combining it with other text in a single block is rejected.
*/
function createOAuthSystemPromptFetch(): typeof globalThis.fetch {
return async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
if (init?.method?.toUpperCase() === 'POST' && init.body) {
try {
const body = JSON.parse(typeof init.body === 'string' ? init.body : new TextDecoder().decode(init.body as ArrayBuffer));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Body type handling may throw on non-string/ArrayBuffer types.

init.body can be Blob, FormData, URLSearchParams, ReadableStream, or other types. Casting directly to ArrayBuffer will fail for these. While the Vercel AI SDK likely only sends strings, a defensive check avoids surprises.

🛡️ Proposed fix for safer body parsing
-        const body = JSON.parse(typeof init.body === 'string' ? init.body : new TextDecoder().decode(init.body as ArrayBuffer));
+        let bodyStr: string;
+        if (typeof init.body === 'string') {
+          bodyStr = init.body;
+        } else if (init.body instanceof ArrayBuffer || ArrayBuffer.isView(init.body)) {
+          bodyStr = new TextDecoder().decode(init.body);
+        } else {
+          // Unsupported body type — pass through unchanged
+          return globalThis.fetch(input, init);
+        }
+        const body = JSON.parse(bodyStr);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/ai/providers/factory.ts` at line 65, The current
one-line parse of init.body can throw for non-string/ArrayBuffer types; update
the body parsing where const body = JSON.parse(...) is assigned to first detect
init.body's runtime type: if it's a string parse directly; if it's an
ArrayBuffer decode with TextDecoder; if it's a Blob call blob.text(); if it's
URLSearchParams call toString(); if it's FormData convert via
Object.fromEntries(formData) then JSON.stringify that result (or extract entries
into an object) and then JSON.parse; if it's a ReadableStream or other
unsupported type, read it to a string (e.g., via Response/stream handling) or
fall back to safe empty/object handling and log a warning. Ensure all branches
produce a string or object suitable for JSON.parse and keep references to
init.body and the const body assignment location so the fix targets that line in
factory.ts.


if (body.system && Array.isArray(body.system)) {
// Check if identity block already present
const hasIdentity = body.system.length > 0
&& body.system[0]?.type === 'text'
&& body.system[0]?.text === CLAUDE_CODE_IDENTITY;

Comment on lines +67 to +72
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The identity injection logic does not handle cases where body.system is a string, causing the injection to be silently skipped.
Severity: MEDIUM

Suggested Fix

Add a new conditional branch or modify the existing one to handle the case where body.system is a string. Convert the string to an array containing that string, and then prepend the identity block as planned.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/desktop/src/main/ai/providers/factory.ts#L67-L72

Potential issue: The fetch interceptor logic for identity injection only handles cases
where `body.system` is an array or is `undefined`. If the request body contains a
`system` property that is a string, neither conditional branch is executed. This results
in the identity block not being prepended to the system prompt, silently bypassing the
intended functionality of the PR. While the Vercel AI SDK may typically send an array,
the code should defensively handle the string case to ensure robustness.

Did we get this right? 👍 / 👎 to inform future reviews.

if (!hasIdentity) {
body.system = [
{ type: 'text', text: CLAUDE_CODE_IDENTITY },
...body.system,
];
init = { ...init, body: JSON.stringify(body) };
}
} else if (body.system === undefined && body.messages) {
// No system prompt at all — add identity block
body.system = [{ type: 'text', text: CLAUDE_CODE_IDENTITY }];
init = { ...init, body: JSON.stringify(body) };
}
Comment on lines +67 to +84
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation doesn't handle the case where body.system is a string, which is a valid type for the Anthropic API. If body.system is a string, the identity prompt will not be injected, and the API call may fail for OAuth users.

The logic can be simplified and made more robust by first normalizing the system prompt (whether it's a string or an array) into an array of blocks, and then performing the identity check and injection. This ensures all valid formats for the system prompt are handled correctly.

        if (body.system) {
          const systemBlocks = Array.isArray(body.system)
            ? body.system
            : [{ type: 'text', text: body.system }];

          const hasIdentity = systemBlocks.length > 0 &&
            systemBlocks[0]?.type === 'text' &&
            systemBlocks[0]?.text === CLAUDE_CODE_IDENTITY;

          if (!hasIdentity) {
            body.system = [
              { type: 'text', text: CLAUDE_CODE_IDENTITY },
              ...systemBlocks,
            ];
            init = { ...init, body: JSON.stringify(body) };
          }
        } else if (body.messages) {
          // No system prompt at all — add identity block
          body.system = [{ type: 'text', text: CLAUDE_CODE_IDENTITY }];
          init = { ...init, body: JSON.stringify(body) };
        }

Comment on lines +67 to +84
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing handling for string system prompts.

The Anthropic API accepts system as either a string or an array of content blocks. This interceptor only handles the array case (line 67) and undefined case (line 80). If body.system is a string (e.g., "You are a helpful assistant"), neither branch triggers and the identity block won't be prepended—OAuth users would silently get 400 errors or be restricted to Haiku.

🐛 Proposed fix to handle string system prompts
         if (body.system && Array.isArray(body.system)) {
           // Check if identity block already present
           const hasIdentity = body.system.length > 0
             && body.system[0]?.type === 'text'
             && body.system[0]?.text === CLAUDE_CODE_IDENTITY;

           if (!hasIdentity) {
             body.system = [
               { type: 'text', text: CLAUDE_CODE_IDENTITY },
               ...body.system,
             ];
             init = { ...init, body: JSON.stringify(body) };
           }
+        } else if (typeof body.system === 'string') {
+          // Convert string system to array with identity prepended
+          body.system = [
+            { type: 'text', text: CLAUDE_CODE_IDENTITY },
+            { type: 'text', text: body.system },
+          ];
+          init = { ...init, body: JSON.stringify(body) };
         } else if (body.system === undefined && body.messages) {
           // No system prompt at all — add identity block
           body.system = [{ type: 'text', text: CLAUDE_CODE_IDENTITY }];
           init = { ...init, body: JSON.stringify(body) };
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (body.system && Array.isArray(body.system)) {
// Check if identity block already present
const hasIdentity = body.system.length > 0
&& body.system[0]?.type === 'text'
&& body.system[0]?.text === CLAUDE_CODE_IDENTITY;
if (!hasIdentity) {
body.system = [
{ type: 'text', text: CLAUDE_CODE_IDENTITY },
...body.system,
];
init = { ...init, body: JSON.stringify(body) };
}
} else if (body.system === undefined && body.messages) {
// No system prompt at all — add identity block
body.system = [{ type: 'text', text: CLAUDE_CODE_IDENTITY }];
init = { ...init, body: JSON.stringify(body) };
}
if (body.system && Array.isArray(body.system)) {
// Check if identity block already present
const hasIdentity = body.system.length > 0
&& body.system[0]?.type === 'text'
&& body.system[0]?.text === CLAUDE_CODE_IDENTITY;
if (!hasIdentity) {
body.system = [
{ type: 'text', text: CLAUDE_CODE_IDENTITY },
...body.system,
];
init = { ...init, body: JSON.stringify(body) };
}
} else if (typeof body.system === 'string') {
// Convert string system to array with identity prepended
body.system = [
{ type: 'text', text: CLAUDE_CODE_IDENTITY },
{ type: 'text', text: body.system },
];
init = { ...init, body: JSON.stringify(body) };
} else if (body.system === undefined && body.messages) {
// No system prompt at all — add identity block
body.system = [{ type: 'text', text: CLAUDE_CODE_IDENTITY }];
init = { ...init, body: JSON.stringify(body) };
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/ai/providers/factory.ts` around lines 67 - 84, The
interceptor currently only handles body.system when it's an array or undefined,
so string system prompts are ignored; update the logic around body.system
handling to detect when body.system is a string and convert it into an array
that prepends the CLAUDE_CODE_IDENTITY block (e.g., replace string with [{ type:
'text', text: CLAUDE_CODE_IDENTITY }, { type: 'text', text: body.system }]) and
then set init = { ...init, body: JSON.stringify(body) } just like the other
branches; ensure you reference and update the same variables (body.system,
CLAUDE_CODE_IDENTITY, init) and preserve existing messages if present.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String system prompt silently skips identity injection

Medium Severity

The Anthropic API accepts system as either a string or an array of content blocks. The conditional only handles body.system when it's an array (Array.isArray) or undefined, but when body.system is a string, it's truthy (so the first branch is skipped) and not === undefined (so the second branch is also skipped). The identity block is silently not injected, meaning OAuth users would still get 400 errors for any request where the SDK sends system as a plain string.

Fix in Cursor Fix in Web

} catch {
// JSON parse failed — pass through unchanged
}
}

return globalThis.fetch(input, init);
};
}

// =============================================================================
// Provider Instance Creators
// =============================================================================
Expand All @@ -53,6 +106,7 @@ function createProviderInstance(config: ProviderConfig) {
case SupportedProvider.Anthropic: {
// OAuth tokens use authToken (Authorization: Bearer) + required beta header
// API keys use apiKey (x-api-key header)
// Custom fetch injects Claude Code identity system block for model access
if (isOAuthToken(apiKey)) {
return createAnthropic({
authToken: apiKey,
Expand All @@ -61,6 +115,7 @@ function createProviderInstance(config: ProviderConfig) {
...headers,
'anthropic-beta': 'claude-code-20250219,oauth-2025-04-20,interleaved-thinking-2025-05-14',
},
fetch: createOAuthSystemPromptFetch(),
});
}
return createAnthropic({
Expand Down
92 changes: 84 additions & 8 deletions apps/desktop/src/main/claude-profile-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ export class ClaudeProfileManager {
// This repairs emails that were truncated due to ANSI escape codes in terminal output
this.migrateCorruptedEmails();

// Remove duplicate profiles sharing the same configDir
// Keeps the first (oldest/default) entry per configDir
this.deduplicateProfiles();

// Populate missing subscription metadata for existing profiles
// This reads subscriptionType and rateLimitTier from Keychain credentials
this.populateSubscriptionMetadata();
Expand Down Expand Up @@ -147,6 +151,47 @@ export class ClaudeProfileManager {
}
}

/**
* Remove duplicate profiles that share the same configDir.
* Keeps the first entry per configDir (which is typically the oldest or the default).
* Remaps activeProfileId if the active profile was a duplicate that got removed.
*/
private deduplicateProfiles(): void {
const seen = new Map<string, number>(); // configDir -> index of first occurrence
const duplicateIndices: number[] = [];
const removedIds: string[] = [];

for (let i = 0; i < this.data.profiles.length; i++) {
const configDir = this.data.profiles[i].configDir;
if (!configDir) continue;

if (seen.has(configDir)) {
duplicateIndices.push(i);
removedIds.push(this.data.profiles[i].id);
} else {
seen.set(configDir, i);
}
Comment on lines +159 to +173
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Canonicalize configDir before using it as the dedupe key.

These comparisons use raw strings. Mixed ~/expanded paths and Windows separator or casing differences can still resolve to the same directory, so the same profile can slip past both startup cleanup and saveProfile() deduplication.

Also applies to: 425-429

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/claude-profile-manager.ts` around lines 159 - 173, The
deduplication uses raw configDir strings so paths like ~/foo vs /home/user/foo,
or differing separators/case on Windows, bypass dedupe; in deduplicateProfiles()
canonicalize each profile.configDir before using it as the Map key (expand
leading ~ to homedir, normalize separators with path.normalize, resolve relative
segments with path.resolve and, if available, follow symlinks with
fs.realpathSync), and on Windows compare case-insensitively (toLowerCase the
canonical path); apply the same canonicalization in saveProfile() (the
referenced block around lines 425-429) so both startup cleanup and saveProfile()
use identical normalized keys for deduplication.

}

if (duplicateIndices.length === 0) return;

// Remap activeProfileId if it points to a duplicate being removed
if (this.data.activeProfileId && removedIds.includes(this.data.activeProfileId)) {
const activeConfigDir = this.data.profiles.find(p => p.id === this.data.activeProfileId)?.configDir;
if (activeConfigDir && seen.has(activeConfigDir)) {
this.data.activeProfileId = this.data.profiles[seen.get(activeConfigDir)!].id;
}
}

// Remove duplicates (iterate in reverse to preserve indices)
for (let i = duplicateIndices.length - 1; i >= 0; i--) {
this.data.profiles.splice(duplicateIndices[i], 1);
}
Comment on lines +186 to +189
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Both duplicate-collapse paths can discard persisted profile state.

Initialization deletes the later row outright, and saveProfile() overwrites the survivor with the incoming object while only preserving id and isDefault. That can drop usage, rateLimitEvents, oauthToken, subscription metadata, or lastUsedAt, which changes auth and availability scoring after restart.

💡 Suggested direction
-    for (let i = duplicateIndices.length - 1; i >= 0; i--) {
-      this.data.profiles.splice(duplicateIndices[i], 1);
-    }
+    for (let i = duplicateIndices.length - 1; i >= 0; i--) {
+      const duplicateIndex = duplicateIndices[i];
+      const duplicate = this.data.profiles[duplicateIndex];
+      const keptIndex = seen.get(duplicate.configDir!)!;
+      this.data.profiles[keptIndex] = this.mergeDuplicateProfiles(
+        this.data.profiles[keptIndex],
+        duplicate
+      );
+      this.data.profiles.splice(duplicateIndex, 1);
+    }
@@
-        profile.id = existingId;
-        profile.isDefault = existingIsDefault || profile.isDefault;
-        this.data.profiles[indexByConfigDir] = profile;
+        this.data.profiles[indexByConfigDir] = this.mergeDuplicateProfiles(
+          this.data.profiles[indexByConfigDir],
+          profile
+        );
+        this.data.profiles[indexByConfigDir].id = existingId;
+        this.data.profiles[indexByConfigDir].isDefault = existingIsDefault || profile.isDefault;
         this.save();
-        return profile;
+        return this.data.profiles[indexByConfigDir];
private mergeDuplicateProfiles(existing: ClaudeProfile, incoming: ClaudeProfile): ClaudeProfile {
  return {
    ...existing,
    ...incoming,
    oauthToken: incoming.oauthToken ?? existing.oauthToken,
    tokenCreatedAt: incoming.tokenCreatedAt ?? existing.tokenCreatedAt,
    usage:
      !existing.usage ||
      (incoming.usage?.lastUpdated && incoming.usage.lastUpdated > existing.usage.lastUpdated)
        ? incoming.usage ?? existing.usage
        : existing.usage,
    rateLimitEvents: [
      ...(existing.rateLimitEvents ?? []),
      ...(incoming.rateLimitEvents ?? []),
    ],
    subscriptionType: incoming.subscriptionType ?? existing.subscriptionType,
    rateLimitTier: incoming.rateLimitTier ?? existing.rateLimitTier,
    lastUsedAt: incoming.lastUsedAt ?? existing.lastUsedAt,
  };
}

Also applies to: 431-444

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/claude-profile-manager.ts` around lines 186 - 189, The
current duplicate-collapse behavior (the reverse-loop that splices
this.data.profiles at duplicateIndices and the saveProfile(...) path) discards
persisted fields like usage, rateLimitEvents, oauthToken, tokenCreatedAt,
subscription metadata, and lastUsedAt; replace the blind deletion/overwrite with
a deterministic merge routine (e.g., add a private
mergeDuplicateProfiles(existing: ClaudeProfile, incoming: ClaudeProfile)) and
call it from both the initialization duplicate-handling loop and saveProfile;
the merge should prefer incoming values when present but fall back to existing
for oauthToken/tokenCreatedAt, choose the most-recent usage by lastUpdated,
concatenate rateLimitEvents arrays, and preserve
subscriptionType/rateLimitTier/lastUsedAt when incoming is undefined so no
persisted state is lost.


console.warn(`[ClaudeProfileManager] Deduplicated profiles: removed ${duplicateIndices.length} duplicates, ${this.data.profiles.length} remaining`);
this.save();
}
Comment on lines +178 to +193
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remap all profile-ID indexes, not just activeProfileId.

After a duplicate is removed, migratedProfileIds and accountPriorityOrder can still point at the deleted ID. apps/desktop/src/main/index.ts:569-593 compares remainingMigratedIds with activeProfile.id, so an active migrated profile will stop triggering re-auth once its duplicate ID is collapsed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/claude-profile-manager.ts` around lines 178 - 193, The
dedupe logic only remaps activeProfileId but must remap all references to
removed profile IDs; create an idRemap (oldId -> retainedId) when you detect
duplicates (using the existing seen map and duplicateIndices), then iterate and
replace entries in this.data.migratedProfileIds and
this.data.accountPriorityOrder (and any other arrays of profile IDs) by mapping
removed IDs to the retained ID; update the arrays in-place (or filter/map to new
arrays) before splicing out duplicates and calling this.save(); reference
ClaudeProfileManager, the seen map, duplicateIndices, removedIds,
migratedProfileIds and accountPriorityOrder when making the changes.


/**
* Populate missing subscription metadata (subscriptionType, rateLimitTier) for existing profiles.
*
Expand Down Expand Up @@ -354,24 +399,55 @@ export class ClaudeProfileManager {
}

/**
* Save or update a profile
* Save or update a profile.
*
* Deduplication: If a profile with the same configDir already exists (but
* a different ID), we update the existing entry instead of creating a
* duplicate. This prevents the profile store from growing unboundedly
* when the UI generates new IDs for the same underlying account.
*/
saveProfile(profile: ClaudeProfile): ClaudeProfile {
// Expand ~ in configDir path
if (profile.configDir) {
profile.configDir = expandHomePath(profile.configDir);
}

const index = this.data.profiles.findIndex(p => p.id === profile.id);
// First, try exact ID match (normal update path)
const indexById = this.data.profiles.findIndex(p => p.id === profile.id);

if (index >= 0) {
// Update existing
this.data.profiles[index] = profile;
} else {
// Add new
this.data.profiles.push(profile);
if (indexById >= 0) {
// Update existing profile by ID
this.data.profiles[indexById] = profile;
this.save();
return profile;
}

// No ID match — check for configDir duplicate before adding
if (profile.configDir) {
const indexByConfigDir = this.data.profiles.findIndex(
p => p.configDir === profile.configDir
);

if (indexByConfigDir >= 0) {
// Same configDir exists with a different ID — update the existing entry
// instead of creating a duplicate. Preserve the existing ID.
const existingId = this.data.profiles[indexByConfigDir].id;
const existingIsDefault = this.data.profiles[indexByConfigDir].isDefault;
debugLog('[ClaudeProfileManager] Dedup: profile with configDir already exists, updating instead of adding', {
existingId,
incomingId: profile.id,
configDir: profile.configDir,
});
profile.id = existingId;
profile.isDefault = existingIsDefault || profile.isDefault;
this.data.profiles[indexByConfigDir] = profile;
this.save();
return profile;
}
}

// Genuinely new profile — add it
this.data.profiles.push(profile);
this.save();
return profile;
}
Expand Down
Loading