Skip to content
Merged
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
35 changes: 32 additions & 3 deletions src/Capacitor.App/Services/ServerWorkContextSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,50 @@ namespace Capacitor.App.Services;
public sealed class ServerWorkContextSource : IWorkContextSource, IAsyncDisposable {
readonly AuthenticatedServerReads<WorkContextClient> _reads;
readonly LastReads<WorkContextRead> _last = new();
// _last.Generation moves on every credential change the app sees, so a viewer settled under an
// older one, or by a read that began before the change, is never served. The lifetime covers
// the change it cannot see: the recovery handler can adopt another account's stored token.
public static readonly TimeSpan ViewerLifetime = TimeSpan.FromMinutes(5);
readonly Lock _viewerLock = new();
readonly TimeProvider _time;
SettledViewer? _viewer;

sealed record SettledViewer(long Generation, DateTimeOffset At, ViewerIdentityDto? Identity);

public ServerWorkContextSource(ConfigRoot config, ProfileContext? profiles, ProfileOverrides env,
MachineAuth machine, AuthenticatedServerReads<WorkContextClient>.ClientFactory? factory = null,
TimeProvider? time = null,
LocalRoot? localRoot = null) {
_time = time ?? TimeProvider.System;
_reads = new(config, profiles, env, machine, (http, url) => new WorkContextClient(http, url), factory, localRoot: localRoot);
}
public async Task<WorkContextRead> ReadAsync(string sessionId, CancellationToken ct) {
var read = await _last.RecordAsync(sessionId, () => _reads.ReadAsync(
(channel, token) => WorkContextReader.ReadAsync(channel, sessionId, token), read => read.Kind == WorkContextReadKind.SignedOut,
WorkContextRead.Of(WorkContextReadKind.SignedOut), WorkContextRead.Of(WorkContextReadKind.Unreachable, "disposed"), ct),
var read = await _last.RecordAsync(sessionId, () => ReadWithViewerAsync(sessionId, ct),
read => read.Kind switch { WorkContextReadKind.Ready => true, WorkContextReadKind.Unreachable => null, _ => false }).ConfigureAwait(false);
// A lapsed credential ends what every session's cache may still show.
if (read.Kind == WorkContextReadKind.SignedOut) _last.Clear();
return read;
}

async Task<WorkContextRead> ReadWithViewerAsync(string sessionId, CancellationToken ct) {
long generation;
SettledViewer? known;
lock (_viewerLock) {
generation = _last.Generation;
known = _viewer is { } v && v.Generation == generation && _time.GetUtcNow() - v.At < ViewerLifetime ? v : null;
}
var read = await _reads.ReadAsync(
(channel, token) => WorkContextReader.ReadAsync(channel, sessionId, readViewer: known is null, token), read => read.Kind == WorkContextReadKind.SignedOut,
WorkContextRead.Of(WorkContextReadKind.SignedOut), WorkContextRead.Of(WorkContextReadKind.Unreachable, "disposed"), ct).ConfigureAwait(false);
if (read.Kind != WorkContextReadKind.Ready) return read;

lock (_viewerLock) {
if (generation != _last.Generation) return read with { Viewer = null };
if (known is null && !read.ViewerFailed) known = _viewer = new(generation, _time.GetUtcNow(), read.Viewer);
return read with { Viewer = known?.Identity };
}
}

public WorkContextRead? Cached(string sessionId) => _last.Get(sessionId);
public ValueTask DisposeAsync() => _reads.DisposeAsync();
public void InvalidateAuthentication() {
Expand Down
20 changes: 18 additions & 2 deletions src/Capacitor.App/ViewModels/WorkContextViewModel.Projections.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ Phase is WorkContextPhase.Ready or WorkContextPhase.NoWorkItem
public string RequesterRole { get => _requesterRole; private set => this.RaiseAndSetIfChanged(ref _requesterRole, value); }
string _requesterInitial = "Y";
public string RequesterInitial { get => _requesterInitial; private set => this.RaiseAndSetIfChanged(ref _requesterInitial, value); }
string? _requesterId;
ViewerIdentityDto? _viewer;
string? _requesterAvatarUrl;
public string? RequesterAvatarUrl { get => _requesterAvatarUrl; private set => this.RaiseAndSetIfChanged(ref _requesterAvatarUrl, value); }

Expand Down Expand Up @@ -216,10 +218,22 @@ void UpdateRequester(AgentStatusDto dto, string vendorLabel) {
Requester = FirstNonBlank(dto.RequesterDisplay, dto.Requester) ?? "You";
RequesterRole = $"This session · {vendorLabel}";
RequesterInitial = WorkContextPersonViewModel.InitialOf(Requester);
RequesterAvatarUrl = GitHubAvatarUrl(dto.Requester);
_requesterId = dto.Requester;
ResolveRequesterAvatar();
}

/// A launch with no requester is the daemon owner's, and the viewer only sees their own daemons.
/// Anyone else gets a picture only from a `github:` id.
void ResolveRequesterAvatar() =>
RequesterAvatarUrl = _viewer is { AvatarUrl: { } own } && (_requesterId is null || _viewer.Owns(_requesterId))
? own
: GitHubAvatarUrl(_requesterId);
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved

void ForgetViewer() {
_viewer = null;
ResolveRequesterAvatar();
}

/// Only a `github:` id names an avatar; a WorkOS-only user keeps the initial.
static string? GitHubAvatarUrl(string? userId) =>
userId is not null && userId.StartsWith("github:", StringComparison.Ordinal)
&& long.TryParse(userId.AsSpan(7), NumberStyles.None, CultureInfo.InvariantCulture, out var id) && id > 0
Expand Down Expand Up @@ -282,6 +296,8 @@ void RaiseRelated() {
}

void ApplyReady(WorkContextRead read) {
_viewer = read.Viewer;
ResolveRequesterAvatar();
if (read.Primary is null) {
ClearCard();
ApplyLinks(read);
Expand Down
1 change: 1 addition & 0 deletions src/Capacitor.App/ViewModels/WorkContextViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ void Apply(WorkContextRead read) {
/// The server has just said the viewer may not have this data, so nothing of it stays visible.
void ApplyTerminal(WorkContextPhase phase) {
ClearServerProjections();
ForgetViewer();
Phase = phase;
IsStale = false;
}
Expand Down
1 change: 1 addition & 0 deletions src/Capacitor.Cli.Core/Models.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,7 @@ public sealed record CurationApplyResponse {
[JsonSerializable(typeof(WorkItems.WorkItemTopologyDto))]
[JsonSerializable(typeof(WorkItems.WorkItemDto))]
[JsonSerializable(typeof(WorkItems.SessionSummaryDto))]
[JsonSerializable(typeof(WorkItems.ViewerIdentityDto))]
[JsonSerializable(typeof(WorkItems.WorkItemErrorDto))]
[JsonSerializable(typeof(RepositoryPayload))]
[JsonSerializable(typeof(GitCacheEntry))]
Expand Down
1 change: 1 addition & 0 deletions src/Capacitor.Cli.Core/WorkItems/IWorkContextChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ public interface IWorkContextChannel {
Task<WorkContextOutcome<WorkItemDto>> GetWorkItemAsync(string workItemId, CancellationToken ct);
Task<WorkContextOutcome<WorkItemTopologyDto>> GetTopologyAsync(string workItemId, CancellationToken ct);
Task<WorkContextOutcome<SessionSummaryDto>> GetSessionSummaryAsync(string sessionId, CancellationToken ct);
Task<WorkContextOutcome<ViewerIdentityDto>> GetViewerIdentityAsync(CancellationToken ct);
}
17 changes: 17 additions & 0 deletions src/Capacitor.Cli.Core/WorkItems/ViewerIdentityDto.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System.Text.Json.Serialization;

namespace Capacitor.Cli.Core.WorkItems;

/// The body of <c>GET /api/users/me/identity</c>: the caller's canonical id, every id merged into it,
/// and the canonical account's picture. A server older than the picture omits it.
public sealed record ViewerIdentityDto {
[JsonPropertyName("user_id")] public required string UserId { get; init; }
[JsonPropertyName("avatar_url")] public string? AvatarUrl { get; init; }
[JsonPropertyName("ids")] public IReadOnlyList<string> Ids { get; init; } = [];

/// A bare number is a legacy GitHub id; the server lists it as `github:{n}`.
public bool Owns(string userId) {
var id = userId.Length > 0 && userId.All(char.IsAsciiDigit) ? $"github:{userId}" : userId;
return id == UserId || Ids.Contains(id);
}
}
3 changes: 3 additions & 0 deletions src/Capacitor.Cli.Core/WorkItems/WorkContextClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ public Task<WorkContextOutcome<SessionSummaryDto>> GetSessionSummaryAsync(string
? GetAsync($"{_base}/api/sessions/{Uri.EscapeDataString(id)}/summary", CapacitorJsonContext.Default.SessionSummaryDto, ct)
: Task.FromResult(Refused<SessionSummaryDto>());

public Task<WorkContextOutcome<ViewerIdentityDto>> GetViewerIdentityAsync(CancellationToken ct) =>
GetAsync($"{_base}/api/users/me/identity", CapacitorJsonContext.Default.ViewerIdentityDto, ct);

static WorkContextOutcome<T> Refused<T>() where T : class => new(0, null, null);

async Task<WorkContextOutcome<T>> GetAsync<T>(string url, JsonTypeInfo<T> typeInfo, CancellationToken ct) where T : class {
Expand Down
21 changes: 18 additions & 3 deletions src/Capacitor.Cli.Core/WorkItems/WorkContextReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ public enum WorkContextReadKind { Ready, SessionUnknown, SignedOut, NotInPlan, U
/// null <see cref="Primary"/> is the no-work-item state; <see cref="ItemFailed"/>,
/// <see cref="TopologyFailed"/> and <see cref="SummaryFailed"/> degrade a section without failing
/// the read. <see cref="Item"/> is served for the primary and may carry a different id than it.
/// <see cref="Viewer"/> is read only when asked for; <see cref="ViewerFailed"/> means no usable
/// answer arrived, so the caller should ask again: no response, a 2xx without a body, a 408, 429 or
/// 5xx. Any other refusal, such as an older server's 404, is an answer.
public sealed record WorkContextRead(
WorkContextReadKind Kind,
IReadOnlyList<SessionWorkItemAssignmentDto> Assignments,
Expand All @@ -17,6 +20,9 @@ public sealed record WorkContextRead(
bool TopologyFailed,
bool SummaryFailed,
string? Detail) {
public ViewerIdentityDto? Viewer { get; init; }
public bool ViewerFailed { get; init; }

public static WorkContextRead Of(WorkContextReadKind kind, string? detail = null) =>
new(kind, [], null, null, null, null, false, false, false, detail);
}
Expand All @@ -28,8 +34,12 @@ public static class WorkContextReader {
/// together, so both overlap the summary rather than queueing behind it. Every call started is
/// awaited before anything is classified. A final 401 anywhere signs the read out: the retry
/// handler has already spent the refresh, and only that outcome makes the source drop its client.
public static async Task<WorkContextRead> ReadAsync(IWorkContextChannel channel, string sessionId, CancellationToken ct) {
public static Task<WorkContextRead> ReadAsync(IWorkContextChannel channel, string sessionId, CancellationToken ct) =>
ReadAsync(channel, sessionId, readViewer: false, ct);

public static async Task<WorkContextRead> ReadAsync(IWorkContextChannel channel, string sessionId, bool readViewer, CancellationToken ct) {
var summaryTask = channel.GetSessionSummaryAsync(sessionId, ct);
var viewerTask = readViewer ? channel.GetViewerIdentityAsync(ct) : null;
var assignments = await channel.GetSessionAssignmentsAsync(sessionId, ct).ConfigureAwait(false);

var rows = assignments.Succeeded ? assignments.Body! : [];
Expand All @@ -40,8 +50,10 @@ public static async Task<WorkContextRead> ReadAsync(IWorkContextChannel channel,
var summary = await summaryTask.ConfigureAwait(false);
var item = itemTask is null ? null : await itemTask.ConfigureAwait(false);
var topology = topologyTask is null ? null : await topologyTask.ConfigureAwait(false);
var viewer = viewerTask is null ? null : await viewerTask.ConfigureAwait(false);

if (assignments.StatusCode == 401 || summary.StatusCode == 401 || item?.StatusCode == 401 || topology?.StatusCode == 401)
if (assignments.StatusCode == 401 || summary.StatusCode == 401 || item?.StatusCode == 401 || topology?.StatusCode == 401
|| viewer?.StatusCode == 401)
return WorkContextRead.Of(WorkContextReadKind.SignedOut);

switch (assignments) {
Expand All @@ -63,7 +75,10 @@ public static async Task<WorkContextRead> ReadAsync(IWorkContextChannel channel,
ItemFailed: item is { Succeeded: false },
TopologyFailed: topology is { Succeeded: false },
SummaryFailed: !summary.Succeeded,
Detail: null);
Detail: null) {
Viewer = viewer is { Succeeded: true } ? viewer.Body : null,
ViewerFailed = viewer is { StatusCode: 0 or 408 or 429 or >= 500 } or { StatusCode: >= 200 and < 300, Body: null },
};
}

/// Every work-item route shares the plan gate, and a plan change between the calls must not
Expand Down
73 changes: 70 additions & 3 deletions test/Capacitor.App.Tests.Unit/ServerWorkContextSourceTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Capacitor.Cli.Core.Auth;
using System.Net;
using Microsoft.Extensions.Time.Testing;
using System.Text;
using Capacitor.App.Services;
using Capacitor.Cli.Core;
Expand All @@ -22,14 +23,18 @@ sealed class ScriptedHandler : HttpMessageHandler {
public readonly Queue<TaskCompletionSource> Gates = new();
public int Sent;
public bool Disposed;
public readonly List<string> Paths = [];

protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken ct) {
Interlocked.Increment(ref Sent);
TaskCompletionSource? gate;
lock (Gates) gate = Gates.Count > 0 ? Gates.Dequeue() : null;
if (gate is not null) await gate.Task.WaitAsync(ct);
var path = request.RequestUri!.AbsolutePath;
var body = path.Contains("/summary", StringComparison.Ordinal) ? """{"session_id":"s","repositories":[],"pull_requests":[]}""" : "[]";
lock (Paths) Paths.Add(path);
var body = path.Contains("/summary", StringComparison.Ordinal) ? """{"session_id":"s","repositories":[],"pull_requests":[]}"""
: path.EndsWith("/identity", StringComparison.Ordinal) ? """{"user_id":"user_a","avatar_url":"https://example.test/a.png","ids":["user_a"]}"""
: "[]";
return new HttpResponseMessage(Status) { Content = new StringContent(body, Encoding.UTF8, "application/json") };
}

Expand All @@ -42,13 +47,15 @@ protected override void Dispose(bool disposing) {
static ProfileContext Profiles(ConfigRoot config) => Resolutions.At("http://server.test", config);

static (ServerWorkContextSource Source, List<ScriptedHandler> Handlers) Build(
ConfigRoot config, ProfileContext? profiles, Func<AuthStatus>? status = null) {
ConfigRoot config, ProfileContext? profiles, Func<AuthStatus>? status = null, Action<ScriptedHandler>? created = null,
TimeProvider? time = null) {
var handlers = new List<ScriptedHandler>();
var source = new ServerWorkContextSource(config, profiles, ProfileOverrides.None, MachineAuth.None, (_, _, _, _) => {
var handler = new ScriptedHandler();
created?.Invoke(handler);
handlers.Add(handler);
return Task.FromResult((new HttpClient(handler), status?.Invoke() ?? AuthStatus.Ok));
});
}, time);
return (source, handlers);
}

Expand All @@ -63,6 +70,66 @@ public async Task A_null_profile_reads_signed_out_without_building_a_client() {
await source.DisposeAsync();
}

[Test]
public async Task The_viewer_is_asked_once_per_credential_and_carried_on_every_read() {
var (source, handlers) = Build(Config.Root, Profiles(Config.Root));

var first = await source.ReadAsync(Session, CancellationToken.None);
var second = await source.ReadAsync(Session, CancellationToken.None);
source.InvalidateAuthentication();
await source.ReadAsync(Session, CancellationToken.None);

await Assert.That(first.Viewer!.AvatarUrl).IsEqualTo("https://example.test/a.png");
await Assert.That(second.Viewer!.AvatarUrl).IsEqualTo("https://example.test/a.png");
var asked = handlers.Sum(h => h.Paths.Count(p => p == "/api/users/me/identity"));
await Assert.That(asked).IsEqualTo(2);
await Assert.That(source.Cached(Session)!.Viewer!.AvatarUrl).IsEqualTo("https://example.test/a.png");
await source.DisposeAsync();
}

/// A token the recovery handler swaps in moves no generation, so only the lifetime brings the
/// identity back into question.
[Test]
public async Task A_settled_viewer_is_asked_again_once_its_lifetime_passes() {
var time = new FakeTimeProvider();
var (source, handlers) = Build(Config.Root, Profiles(Config.Root), time: time);
int Asked() => handlers.Sum(h => h.Paths.Count(p => p == "/api/users/me/identity"));

await source.ReadAsync(Session, CancellationToken.None);
time.Advance(ServerWorkContextSource.ViewerLifetime - TimeSpan.FromSeconds(1));
await source.ReadAsync(Session, CancellationToken.None);
await Assert.That(Asked()).IsEqualTo(1);

time.Advance(TimeSpan.FromSeconds(1));
await source.ReadAsync(Session, CancellationToken.None);
await Assert.That(Asked()).IsEqualTo(2);
await source.DisposeAsync();
}

/// The read parks all three requests before the credential changes, so its identity answer
/// arrives under a retired credential and must neither be served nor spare the next read its ask.
[Test]
public async Task A_viewer_read_under_a_retired_credential_settles_nothing() {
var gates = new[] { new TaskCompletionSource(), new TaskCompletionSource(), new TaskCompletionSource() };
var (source, handlers) = Build(Config.Root, Profiles(Config.Root), created: h => {
if (gates.Any(g => !g.Task.IsCompleted)) foreach (var gate in gates) h.Gates.Enqueue(gate);
});
var stale = source.ReadAsync(Session, CancellationToken.None);
await WorkspaceFixtures.WaitUntilAsync(
() => handlers.Count == 1 && handlers[0].Gates.Count == 0,
what: "the stale read's three requests parked on their gates");
source.InvalidateAuthentication();
foreach (var gate in gates) gate.SetResult();

var staleRead = await stale;
var next = await source.ReadAsync(Session, CancellationToken.None);

await Assert.That(staleRead.Viewer).IsNull();
await Assert.That(next.Viewer).IsNotNull();
await Assert.That(handlers[1].Paths).Contains("/api/users/me/identity");
await source.DisposeAsync();
}

[Test]
public async Task A_rejected_auth_status_disposes_the_client_it_was_handed() {
var (source, handlers) = Build(Config.Root, Profiles(Config.Root), () => AuthStatus.Expired);
Expand Down
Loading
Loading