fix(claude-code): a late expiry after a completed login replays without a second login
This commit is contained in:
@@ -87,17 +87,24 @@ public sealed class ClaudeCodeApi(SessionRegistry registry, ClaudeCodeApiOptions
|
||||
try
|
||||
{
|
||||
var loginUi = Options.LoginUi;
|
||||
var coordinator = Options.Coordinator ?? ClaudeLoginCoordinator.Shared;
|
||||
var generation = coordinator.Generation;
|
||||
var expired = await RunAttemptAsync(model, context, streamOptions, writer, deferLoginFailure: loginUi is not null, ct).ConfigureAwait(false);
|
||||
if (expired is null || loginUi is null) return;
|
||||
|
||||
// The session expired before anything was published: pause, let the owner log in, then replay exactly once.
|
||||
// A login that succeeded after this attempt started makes the expiry stale: replay without another flow.
|
||||
if (coordinator.Generation != generation)
|
||||
{
|
||||
await RunAttemptAsync(model, context, streamOptions, writer, deferLoginFailure: false, ct).ConfigureAwait(false);
|
||||
return;
|
||||
}
|
||||
var builder = new AssistantMessageBuilder(model);
|
||||
Notify(streamOptions.SessionId, "Claude Code session expired; waiting for the bot owner to sign in…", NotifyKind.Warning);
|
||||
LoginOutcome outcome;
|
||||
try
|
||||
{
|
||||
outcome = await (Options.Coordinator ?? ClaudeLoginCoordinator.Shared)
|
||||
.EnsureLoggedInAsync(flowCt => RunLoginFlowAsync(loginUi, flowCt), ct).ConfigureAwait(false);
|
||||
outcome = await coordinator.EnsureLoggedInAsync(flowCt => RunLoginFlowAsync(loginUi, flowCt), ct).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
|
||||
@@ -3,7 +3,9 @@ namespace Volcanic.AgenticCore.Extensions.ClaudeCode;
|
||||
/// <summary>
|
||||
/// Keeps Claude Code logins single-flight: every session shares one <c>claude</c> configuration, so concurrent expired
|
||||
/// requests wait on the same flow. The flow runs detached from any caller's token; a caller's cancellation only stops
|
||||
/// its own wait. The slot clears when the flow completes, so a later expiry starts a fresh one.
|
||||
/// its own wait. The slot clears when the flow completes, so a later expiry starts a fresh one. Each successful flow
|
||||
/// advances <see cref="Generation"/>, so a request whose attempt started before a login finished can tell that its
|
||||
/// late expiry is stale and replay without starting another flow.
|
||||
/// </summary>
|
||||
public sealed class ClaudeLoginCoordinator
|
||||
{
|
||||
@@ -12,6 +14,10 @@ public sealed class ClaudeLoginCoordinator
|
||||
|
||||
private readonly Lock _lock = new();
|
||||
private Task<LoginOutcome>? _current;
|
||||
private long _generation;
|
||||
|
||||
/// <summary>How many flows have succeeded so far; read it before an attempt, compare after a login-required failure.</summary>
|
||||
public long Generation => Interlocked.Read(ref _generation);
|
||||
|
||||
/// <summary>Joins the in-flight login, or starts one. <paramref name="runFlow"/> only ever runs for the caller that starts it.</summary>
|
||||
public Task<LoginOutcome> EnsureLoggedInAsync(Func<CancellationToken, Task<LoginOutcome>> runFlow, CancellationToken cancellationToken)
|
||||
@@ -33,17 +39,21 @@ public sealed class ClaudeLoginCoordinator
|
||||
// overwrite it with the now-finished task. The slot would then stay stuck on a stale result forever, since
|
||||
// nothing else ever clears it. Task.Yield() guarantees the finally always runs on a later, separate turn.
|
||||
await Task.Yield();
|
||||
LoginOutcome outcome;
|
||||
try
|
||||
{
|
||||
return await runFlow(CancellationToken.None).ConfigureAwait(false);
|
||||
outcome = await runFlow(CancellationToken.None).ConfigureAwait(false);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
return LoginOutcome.Failed(ex.Message);
|
||||
outcome = LoginOutcome.Failed(ex.Message);
|
||||
}
|
||||
finally
|
||||
lock (_lock)
|
||||
{
|
||||
lock (_lock) _current = null;
|
||||
// Advanced before the slot clears: a caller that finds the slot empty already sees the new generation.
|
||||
if (outcome.Success) Interlocked.Increment(ref _generation);
|
||||
_current = null;
|
||||
}
|
||||
return outcome;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -536,4 +536,38 @@ public class ClaudeCodeApiTests : IDisposable
|
||||
Assert.False(outcome.Success);
|
||||
Assert.StartsWith("the owner could not be reached", outcome.Error);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Login_required_after_a_login_completed_mid_attempt_replays_without_a_second_login()
|
||||
{
|
||||
if (!Fixtures.IsPosix) return;
|
||||
var state = Path.Combine(_root, "state");
|
||||
Directory.CreateDirectory(state);
|
||||
// The first run reports the expiry only once released (after the test's own login); later runs succeed.
|
||||
var script = FakeClaude(
|
||||
$"STATE=\"{state}\"\n" +
|
||||
"echo run >> \"$STATE/runs\"\n" +
|
||||
"if [ \"$(wc -l < \"$STATE/runs\")\" -eq 1 ]; then\n" +
|
||||
" : > \"$STATE/started\"\n" +
|
||||
" while [ ! -f \"$STATE/release\" ]; do sleep 0.05; done\n" +
|
||||
$" cat \"{Fixtures.FixturePath("claude-2.1.281-stream-auth-expired.jsonl")}\"\n" +
|
||||
$"else cat \"{Fixtures.FixturePath("claude-2.1.281-stream-tool-ok.jsonl")}\"; fi",
|
||||
tail: "exit 0");
|
||||
var coordinator = new ClaudeLoginCoordinator();
|
||||
var api = Api(script, loginUi: CountingLoginUi(), coordinator: coordinator);
|
||||
|
||||
var collecting = CollectWithResponses(o => api.StreamAsync(Fixtures.Sonnet, ToolContext(), o, CancellationToken.None));
|
||||
var deadline = DateTime.UtcNow.AddSeconds(10);
|
||||
while (!File.Exists(StatePath("started")) && DateTime.UtcNow < deadline) await Task.Delay(20);
|
||||
Assert.True(File.Exists(StatePath("started")));
|
||||
// A login elsewhere completes while this attempt is still running.
|
||||
Assert.True((await coordinator.EnsureLoggedInAsync(_ => Task.FromResult(LoginOutcome.Ok), CancellationToken.None)).Success);
|
||||
File.WriteAllText(StatePath("release"), "");
|
||||
var (events, responses) = await collecting.WaitAsync(TimeSpan.FromSeconds(20));
|
||||
|
||||
Assert.Equal(["Start", "TextStart", "TextDelta", "TextEnd", "ToolCallStart", "ToolCallDelta", "ToolCallEnd", "Done"], events.Select(e => e.GetType().Name).ToArray());
|
||||
Assert.Equal(2, Runs());
|
||||
Assert.Equal([0], responses);
|
||||
Assert.Equal(0, _loginRequests);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -86,4 +86,24 @@ public class ClaudeLoginCoordinatorTests
|
||||
|
||||
Assert.Equal(50, runs);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Generation_advances_only_on_a_successful_flow()
|
||||
{
|
||||
var coordinator = new ClaudeLoginCoordinator();
|
||||
Assert.Equal(0, coordinator.Generation);
|
||||
|
||||
await coordinator.EnsureLoggedInAsync(_ => Task.FromResult(LoginOutcome.Failed("no code was entered")), CancellationToken.None);
|
||||
Assert.Equal(0, coordinator.Generation);
|
||||
|
||||
var gate = new TaskCompletionSource<LoginOutcome>(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
var running = coordinator.EnsureLoggedInAsync(_ => gate.Task, CancellationToken.None);
|
||||
Assert.Equal(0, coordinator.Generation);
|
||||
gate.SetResult(LoginOutcome.Ok);
|
||||
await running;
|
||||
Assert.Equal(1, coordinator.Generation);
|
||||
|
||||
await coordinator.EnsureLoggedInAsync(_ => throw new InvalidOperationException("boom"), CancellationToken.None);
|
||||
Assert.Equal(1, coordinator.Generation);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user