mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
fix(daemon): retry transient OAuth refresh contention (#8160)
When two local processes race to refresh the same stored OAuth credential, the agent aborts its refresh and reports the condition as transient: Failed to refresh OAuth token: another Claude Code process is refreshing it or exited mid-refresh. This is usually transient; retry in a minute... The classifier had no branch for this text, so it fell to the generic process-exit fallback as `exit_nonzero`, which `transientSuppressedReason` refuses for `process_exit` with `non_retryable_category`. The safe-retry path was suppressed and the run died on a dead-end failure card, for a condition that clears as soon as the contending holder releases the lock. Name it `credential_refresh_contention` and wire it the way `session_resume_expired` already is: classified at `session_init` as retryable with `user_action: 'retry'`, and added to the `process_exit` transient allowlist. The branch is claimed ahead of the auth verdicts on purpose. The message ends "...or sign in again", and `auth` is fixed at `retryable: false` / `user_action: 'login'` — reaching it would prescribe re-authenticating a credential that never expired. The detector is anchored on the contention itself rather than on the word "refresh", so a genuinely spent or replayed refresh token keeps its non-retryable auth verdict; a test pins that direction. Retry is safe without further gating: the failure is raised before a request is sent, so nothing streamed, no tool ran and no artifact was written, leaving the side-effect gates in `decideSafeRunRetry` satisfied.
This commit is contained in:
@@ -354,6 +354,20 @@ function isSessionResumeExpiredText(text: string): boolean {
|
||||
/\bsession [\w-]+ not found\b/i.test(text);
|
||||
}
|
||||
|
||||
// A lost race for the local credential-refresh lock, not a bad credential.
|
||||
//
|
||||
// Anchored on the contention itself — another holder, or a holder that died
|
||||
// mid-refresh — because that is what separates this from a genuinely unusable
|
||||
// token. An expired or replayed refresh token reads as "refresh token expired
|
||||
// / already used" and belongs to `authDetail`; those phrases are deliberately
|
||||
// not matched here. The trailing "or sign in again" in the agent's own message
|
||||
// is advice for the persistent case and is likewise not treated as evidence,
|
||||
// so a first occurrence cannot be laundered into a sign-in prompt.
|
||||
function isTransientCredentialRefreshText(text: string): boolean {
|
||||
return /\banother [\w ]*process is refreshing it\b/i.test(text) ||
|
||||
/\bexited mid-refresh\b/i.test(text);
|
||||
}
|
||||
|
||||
function promptTooLargeDetail(text: string): TrackingRunFailureDetail | null {
|
||||
if (
|
||||
/\b(?:Payload Too Large|Request Entity Too Large|request entity too large|request body exceeds configured limit)\b/i.test(text) ||
|
||||
@@ -1212,6 +1226,31 @@ function classifyRunFailureBase(
|
||||
);
|
||||
}
|
||||
|
||||
// Concurrent refreshes of one stored OAuth credential: a second agent process
|
||||
// (or a `claude login` run by hand) held the refresh lock, so this run's
|
||||
// refresh aborted before a request was ever sent.
|
||||
//
|
||||
// Claimed ahead of every auth branch below on purpose. The agent's message
|
||||
// ends "…or sign in again", and `auth` is fixed at `retryable: false` /
|
||||
// `user_action: 'login'` — so reaching that verdict would tell the user to
|
||||
// re-authenticate a credential that is perfectly valid, and strand the run.
|
||||
// Claimed ahead of the `process_exit` fallbacks for the opposite reason:
|
||||
// there it lands on `exit_nonzero`, which `transientSuppressedReason` refuses
|
||||
// as a non-retryable category, which is how this surfaces today — a dead-end
|
||||
// failure card for a condition the agent itself describes as transient.
|
||||
//
|
||||
// Nothing streamed and no tool ran at `session_init`, so the replay is safe
|
||||
// and the side-effect gates in the retry policy stay satisfied.
|
||||
if (isTransientCredentialRefreshText(text)) {
|
||||
return classification(
|
||||
'process_exit',
|
||||
'credential_refresh_contention',
|
||||
'session_init',
|
||||
true,
|
||||
'retry',
|
||||
);
|
||||
}
|
||||
|
||||
if (errorCode === 'AGENT_UNAVAILABLE') {
|
||||
return classification(
|
||||
'process_exit',
|
||||
|
||||
@@ -215,6 +215,12 @@ function transientSuppressedReason(
|
||||
}
|
||||
return detail === 'qoder_stop_sequence' ||
|
||||
detail === 'session_resume_expired' ||
|
||||
// A lost credential-refresh lock resolves as soon as the contending
|
||||
// holder finishes, so the replay is the fix. Like `session_resume_expired`
|
||||
// it is admitted at any stage, `session_init` included: it is raised
|
||||
// before a request is sent, so that stage is the only one it can occur at
|
||||
// and excluding it would make the entry dead on arrival.
|
||||
detail === 'credential_refresh_contention' ||
|
||||
detail === 'stream_error' ||
|
||||
detail === 'fatal_rpc_error'
|
||||
? null
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
// A lost race for the local OAuth refresh lock must classify as a retryable
|
||||
// `credential_refresh_contention`, not as an auth failure and not as an opaque
|
||||
// process exit.
|
||||
//
|
||||
// Upstream text is the Claude Code CLI's own message, raised when a second
|
||||
// process (another agent run, or a `claude login` typed by hand) holds the
|
||||
// refresh lock for the shared credential file:
|
||||
//
|
||||
// "Failed to refresh OAuth token: another Claude Code process is refreshing
|
||||
// it or exited mid-refresh. This is usually transient; retry in a minute,
|
||||
// and if it persists close other Claude Code processes or sign in again"
|
||||
//
|
||||
// Two wrong destinations are possible without this branch, and both are
|
||||
// asserted against below:
|
||||
//
|
||||
// * `auth` — fixed at `retryable: false` / `user_action: 'login'`, which
|
||||
// tells the user to re-authenticate a credential that never expired.
|
||||
// * `exit_nonzero` — refused by `transientSuppressedReason` as a
|
||||
// non-retryable category, which strands a condition the agent itself
|
||||
// calls transient behind a dead-end failure card.
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
// Same stubs as run-failure-classification.test.ts: these modules would claim
|
||||
// the text first, so stubbing them proves the new branch is what recognises it.
|
||||
vi.mock('../src/integrations/vela-errors.js', () => ({
|
||||
classifyAmrAccountFailure: () => null,
|
||||
reportsPlatformProviderCredentialFault: () => false,
|
||||
}));
|
||||
|
||||
vi.mock('../src/runtimes/auth.js', () => ({
|
||||
classifyAgentServiceFailure: () => null,
|
||||
reportsToolPrincipalAuthFailure: () => false,
|
||||
}));
|
||||
|
||||
import {
|
||||
classifyRunFailure,
|
||||
type RunEventForFailureClassification,
|
||||
} from '../src/run-failure-classification.js';
|
||||
|
||||
const CONTENTION_MESSAGE =
|
||||
'Failed to refresh OAuth token: another Claude Code process is refreshing it or exited mid-refresh. This is usually transient; retry in a minute, and if it persists close other Claude Code processes or sign in again';
|
||||
|
||||
function errorEvent(
|
||||
code: string,
|
||||
message: string,
|
||||
): RunEventForFailureClassification {
|
||||
return {
|
||||
event: 'error',
|
||||
data: { message, error: { code, message } },
|
||||
};
|
||||
}
|
||||
|
||||
function classify(code: string, message: string) {
|
||||
return classifyRunFailure({
|
||||
result: 'failed',
|
||||
status: {
|
||||
status: 'failed',
|
||||
error: message,
|
||||
errorCode: code,
|
||||
exitCode: 1,
|
||||
signal: null,
|
||||
},
|
||||
errorCode: code,
|
||||
agentId: 'claude',
|
||||
events: [errorEvent(code, message)],
|
||||
});
|
||||
}
|
||||
|
||||
describe('credential refresh contention', () => {
|
||||
it('names the contention instead of falling through to an opaque exit', () => {
|
||||
const result = classify('AGENT_EXECUTION_FAILED', CONTENTION_MESSAGE);
|
||||
expect(result?.failure_detail).toBe('credential_refresh_contention');
|
||||
expect(result?.failure_category).toBe('process_exit');
|
||||
expect(result?.failure_stage).toBe('session_init');
|
||||
});
|
||||
|
||||
it('stays retryable and asks for a retry, never a sign-in', () => {
|
||||
const result = classify('AGENT_EXECUTION_FAILED', CONTENTION_MESSAGE);
|
||||
expect(result?.retryable).toBe(true);
|
||||
expect(result?.user_action).toBe('retry');
|
||||
// The credential is valid; sending the user to /login is the bug.
|
||||
expect(result?.failure_category).not.toBe('auth');
|
||||
expect(result?.user_action).not.toBe('login');
|
||||
});
|
||||
|
||||
// The message's own tail advises signing in "if it persists". That advice is
|
||||
// for a human reading a repeat, not evidence about this occurrence — it must
|
||||
// not drag the first failure into the auth verdict.
|
||||
it('does not treat the trailing sign-in advice as auth evidence', () => {
|
||||
const result = classify(
|
||||
'AGENT_EXECUTION_FAILED',
|
||||
'Failed to refresh OAuth token: another Claude Code process is refreshing it. Sign in again if it persists.',
|
||||
);
|
||||
expect(result?.failure_detail).toBe('credential_refresh_contention');
|
||||
expect(result?.retryable).toBe(true);
|
||||
});
|
||||
|
||||
// The half of the message that arrives when the lock holder died rather than
|
||||
// when it was merely busy.
|
||||
it('recognises the mid-refresh abort shape on its own', () => {
|
||||
const result = classify(
|
||||
'AGENT_EXECUTION_FAILED',
|
||||
'Failed to refresh OAuth token: the refreshing process exited mid-refresh.',
|
||||
);
|
||||
expect(result?.failure_detail).toBe('credential_refresh_contention');
|
||||
expect(result?.retryable).toBe(true);
|
||||
});
|
||||
|
||||
// Reverse direction: a genuinely unusable token is still an auth failure.
|
||||
// Retrying that one cannot help, so it must keep its non-retryable verdict.
|
||||
it('leaves a spent refresh token in the auth bucket', () => {
|
||||
const result = classify(
|
||||
'AGENT_EXECUTION_FAILED',
|
||||
'Auth error: refresh token already used, please sign in again.',
|
||||
);
|
||||
expect(result?.failure_detail).not.toBe('credential_refresh_contention');
|
||||
expect(result?.failure_category).toBe('auth');
|
||||
expect(result?.retryable).toBe(false);
|
||||
});
|
||||
|
||||
it('does not claim unrelated failures', () => {
|
||||
const result = classify('AGENT_EXECUTION_FAILED', 'Error: spawn ENOENT');
|
||||
expect(result?.failure_detail).not.toBe('credential_refresh_contention');
|
||||
});
|
||||
});
|
||||
@@ -116,6 +116,7 @@ describe('decideSafeRunRetry', () => {
|
||||
'agent_protocol_error',
|
||||
'qoder_stop_sequence',
|
||||
'session_resume_expired',
|
||||
'credential_refresh_contention',
|
||||
'stream_error',
|
||||
'fatal_rpc_error',
|
||||
] as const) {
|
||||
@@ -135,6 +136,40 @@ describe('decideSafeRunRetry', () => {
|
||||
}
|
||||
});
|
||||
|
||||
// A lost credential-refresh lock is raised before any request is sent, so
|
||||
// `session_init` is the only stage it can carry. `agent_protocol_error` is
|
||||
// suppressed at exactly that stage, so pin the distinction: were the entry to
|
||||
// inherit that stage rule it would be dead on arrival and never retry once.
|
||||
it('retries a credential refresh contention at session_init', () => {
|
||||
expect(
|
||||
decide({
|
||||
failure: {
|
||||
failure_category: 'process_exit',
|
||||
failure_detail: 'credential_refresh_contention',
|
||||
failure_stage: 'session_init',
|
||||
retryable: true,
|
||||
},
|
||||
}),
|
||||
).toMatchObject({
|
||||
shouldRetry: true,
|
||||
retryReason: 'transient_failure',
|
||||
});
|
||||
|
||||
expect(
|
||||
decide({
|
||||
failure: {
|
||||
failure_category: 'process_exit',
|
||||
failure_detail: 'agent_protocol_error',
|
||||
failure_stage: 'session_init',
|
||||
retryable: true,
|
||||
},
|
||||
}),
|
||||
).toMatchObject({
|
||||
shouldRetry: false,
|
||||
retrySuppressedReason: 'unsafe_failure_stage',
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps upstream client errors out of the transient retry allowlist', () => {
|
||||
expect(
|
||||
decide({
|
||||
|
||||
@@ -350,6 +350,15 @@ export type TrackingRunFailureDetail =
|
||||
| 'agent_protocol_error'
|
||||
| 'acp_frame_too_large'
|
||||
| 'session_resume_expired'
|
||||
// Two local processes raced to refresh the same stored OAuth credential and
|
||||
// one lost the lock (Claude Code: "another Claude Code process is refreshing
|
||||
// it or exited mid-refresh"). The credential itself is valid — nothing is
|
||||
// expired, revoked or misconfigured — so this must not land in `auth`, whose
|
||||
// verdict prescribes a sign-in the user does not need. The contending holder
|
||||
// releases the lock within seconds, which is why the agent itself calls the
|
||||
// condition transient; named here so it stays retryable rather than falling
|
||||
// through to an opaque exit.
|
||||
| 'credential_refresh_contention'
|
||||
| 'fabricated_role_marker'
|
||||
| 'permission_request_not_found'
|
||||
| 'qoder_stop_sequence'
|
||||
|
||||
Reference in New Issue
Block a user