Skip to content

fix: track spawned profile ID to correctly attribute rate limit errors #1994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
octo-patch wants to merge 1 commit into
base: develop
Choose a base branch
from
+6 −4
Open

fix: track spawned profile ID to correctly attribute rate limit errors #1994

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions apps/desktop/src/main/agent/agent-process.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class AgentProcessManager {

private setupProcessEnvironment(
extraEnv: Record<string, string>
): NodeJS.ProcessEnv {
): { env: NodeJS.ProcessEnv; profileId: string; profileName: string } {
// Get best available Claude profile environment (automatically handles rate limits)
const profileResult = getBestAvailableProfileEnv();
const profileEnv = profileResult.env;
Expand Down Expand Up @@ -253,7 +253,7 @@ export class AgentProcessManager {
apiKeyPrefix: mergedEnv.ANTHROPIC_API_KEY?.substring(0, 8) || '(not set)',
});

return mergedEnv;
return { env: mergedEnv, profileId: profileResult.profileId, profileName: profileResult.profileName };
}

private handleProcessFailure(
Expand All @@ -263,7 +263,8 @@ export class AgentProcessManager {
): boolean {
console.log('[AgentProcess] Checking for rate limit in output (last 500 chars):', allOutput.slice(-500));

const rateLimitDetection = detectRateLimit(allOutput);
const profileAssignment = this.state.getTaskProfileAssignment(taskId);
const rateLimitDetection = detectRateLimit(allOutput, profileAssignment?.profileId);
Comment on lines +266 to +267
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 4, 2026

Choose a reason for hiding this comment

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

high

The handleProcessFailure method is currently only invoked from the deprecated spawnProcess method. However, spawnWorkerProcess (line 838) is the primary execution path for autonomous AI tasks and it does not appear to utilize this rate limit detection and attribution logic. To fully address the issue described in the PR, the worker-based execution path should also be updated to track and attribute rate limits correctly.

console.log('[AgentProcess] Rate limit detection result:', {
isRateLimited: rateLimitDetection.isRateLimited,
resetTime: rateLimitDetection.resetTime,
Expand Down Expand Up @@ -549,7 +550,8 @@ export class AgentProcessManager {
spawnId
});

const env = this.setupProcessEnvironment(extraEnv);
const { env, profileId: spawnedProfileId, profileName: spawnedProfileName } = this.setupProcessEnvironment(extraEnv);
this.state.assignProfileToTask(taskId, spawnedProfileId, spawnedProfileName, 'proactive');
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 4, 2026

Choose a reason for hiding this comment

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

high

While calling assignProfileToTask here ensures correct attribution during failure, the assignment is never removed from the taskProfileAssignments map in AgentState. This will result in a memory leak as the map grows with every task execution. Please ensure that this.state.clearTaskProfileAssignment(taskId) is called in the process exit and error handlers (e.g., near lines 777 and 812).

Copy link
Copy Markdown

@sentry sentry bot Apr 4, 2026

Choose a reason for hiding this comment

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

Bug: The call to assignProfileToTask() was added to the deprecated spawnProcess method instead of the spawnWorkerProcess method used by production code, so profile assignments are not tracked.
Severity: MEDIUM

Suggested Fix

Move the assignProfileToTask() call from the deprecated spawnProcess method to the spawnWorkerProcess method. This will ensure that all production tasks have their associated profile ID correctly tracked for error handling.

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/agent/agent-process.ts#L554

Potential issue: The fix to track the profile ID for a spawned task was added to the
`spawnProcess` method. However, this method is deprecated and no longer used in
production. All production tasks are created via `spawnWorkerProcess`, which was not
modified to call `assignProfileToTask()`. As a result, when a task fails (e.g., due to a
rate limit), `handleProcessFailure` cannot retrieve the correct profile ID. The system
then falls back to the currently active profile, causing rate limit errors to be
misattributed if the user has switched profiles since the task began.

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


// Get active API profile environment variables
let apiProfileEnv: Record<string, string> = {};
Expand Down