fix: lock evidence, review, and repository writes; give GitHub refreshes time

app/api/GitHub/EvidenceEndpoints.cs

@@ -5,6 +5,7 @@
 using Microsoft.EntityFrameworkCore;
 using SpecThread.Api.Data;
 using SpecThread.Api.Projects;
+using SpecThread.Api.Requirements;
 
 namespace SpecThread.Api.GitHub;
 
@@ -72,10 +73,7 @@ private static async Task<Results<Created<EvidenceResponse>, ValidationProblem,
         var here = existing.FindAll(e => e.RepositoryId == repository.RepositoryId);
         if (reference.Number is { } number && here.Exists(e => e.Number == number)) return AlreadyLinked($"#{number}");
         if (reference.Tag is { } wanted && here.Exists(e => e.Kind == "release" && e.Tag == wanted)) return AlreadyLinked($"Release {wanted}");
-        if (existing.Count >= MaxEvidence)
-        {
-            return TypedResults.Problem($"A requirement can have at most {MaxEvidence} evidence links.", statusCode: StatusCodes.Status409Conflict);
-        }
+        if (existing.Count >= MaxEvidence) return LimitReached();
 
         var found = reference switch
         {
@@ -126,16 +124,33 @@ private static async Task<Results<Created<EvidenceResponse>, ValidationProblem,
             : await ReadContentsAsync(github, repository, here.FindAll(e => e.Kind == "release"), [evidence], cancel);
         if (contents != GitHubFailure.None) return Failed(contents, fullName);
 
+        // GitHub was read without holding the team lock. Take it now and check again what the reads relied on.
+        await using var transaction = await db.Database.BeginTransactionAsync(cancel);
+        switch (await LockAndRecheckAsync(db, userId, requirementId, repository, cancel))
+        {
+            case Recheck.NotFound: return TypedResults.NotFound();
+            case Recheck.Archived: return ProjectEndpoints.Archived();
+            case Recheck.Changed: return Changed();
+        }
+        // Counted again under the lock, so links added at the same time cannot pass the limit together.
+        if (await db.RequirementEvidence.CountAsync(e => e.RequirementId == requirementId, cancel) >= MaxEvidence) return LimitReached();
+
         db.RequirementEvidence.Add(evidence);
         try
         {
             await db.SaveChangesAsync(cancel);
         }
+        catch (DbUpdateConcurrencyException)
+        {
+            // A linked release this link updates was removed meanwhile.
+            return Changed();
+        }
         catch (DbUpdateException)
         {
             // Two links of the same item at once: a unique index refused this one.
             return AlreadyLinked(Label(found.Value));
         }
+        await transaction.CommitAsync(cancel);
         return TypedResults.Created($"/requirements/{requirementId}/evidence/{evidence.Id}", ToResponse(evidence));
     }
 
@@ -145,7 +160,8 @@ private static async Task<Results<Created<EvidenceResponse>, ValidationProblem,
     private static async Task<Results<Ok<List<EvidenceResponse>>, NotFound, ProblemHttpResult>> Refresh(
         Guid requirementId, ClaimsPrincipal user, SpecThreadDbContext db, IGitHubClient github, CancellationToken cancel)
     {
-        var scope = await FindAsync(db, user.CallerId(), requirementId, cancel);
+        var userId = user.CallerId();
+        var scope = await FindAsync(db, userId, requirementId, cancel);
         if (scope is null) return TypedResults.NotFound();
         if (scope.Archived) return ProjectEndpoints.Archived();
         var repository = await db.ProjectRepositories.AsNoTracking().SingleOrDefaultAsync(r => r.ProjectId == scope.ProjectId, cancel);
@@ -185,17 +201,39 @@ private static async Task<Results<Ok<List<EvidenceResponse>>, NotFound, ProblemH
             (release, change) => before[release.Id].Sha == release.Sha && before[change.Id].Change == ChangeSha(change) &&
                 before[release.Id].Contains.GetValueOrDefault(change.Id));
         if (contents != GitHubFailure.None) return Failed(contents, fullName);
-        await db.SaveChangesAsync(cancel);
+
+        // GitHub was read without holding the team lock, which can take a while. Take it now and
+        // check again what the reads relied on.
+        await using var transaction = await db.Database.BeginTransactionAsync(cancel);
+        switch (await LockAndRecheckAsync(db, userId, requirementId, repository, cancel))
+        {
+            case Recheck.NotFound: return TypedResults.NotFound();
+            case Recheck.Archived: return ProjectEndpoints.Archived();
+            case Recheck.Changed: return Changed();
+        }
+        try
+        {
+            await db.SaveChangesAsync(cancel);
+        }
+        catch (DbUpdateConcurrencyException)
+        {
+            // A link was removed while GitHub was being read. Nothing is saved.
+            return Changed();
+        }
+        await transaction.CommitAsync(cancel);
         return TypedResults.Ok(await ReadAsync(db, requirementId, cancel));
     }
 
     private static async Task<Results<NoContent, NotFound, ProblemHttpResult>> Unlink(
         Guid requirementId, Guid evidenceId, ClaimsPrincipal user, SpecThreadDbContext db, CancellationToken cancel)
     {
-        var scope = await FindAsync(db, user.CallerId(), requirementId, cancel);
-        if (scope is null) return TypedResults.NotFound();
-        if (scope.Archived) return ProjectEndpoints.Archived();
+        // Serialize with archiving and membership changes in this project's team.
+        await using var transaction = await db.Database.BeginTransactionAsync(cancel);
+        var found = await RequirementEndpoints.LockForMember(db, user.CallerId(), requirementId, cancel);
+        if (found is null) return TypedResults.NotFound();
+        if (found.Archived) return ProjectEndpoints.Archived();
         var deleted = await db.RequirementEvidence.Where(e => e.Id == evidenceId && e.RequirementId == requirementId).ExecuteDeleteAsync(cancel);
+        await transaction.CommitAsync(cancel);
         return deleted == 0 ? TypedResults.NotFound() : TypedResults.NoContent();
     }
 
@@ -205,6 +243,22 @@ private static async Task<Results<NoContent, NotFound, ProblemHttpResult>> Unlin
             .Join(db.ProjectsFor(userId), r => r.ProjectId, p => p.Id, (r, p) => new Scope(p.Id, r.ArchivedAt != null || p.ArchivedAt != null))
             .SingleOrDefaultAsync(cancel);
 
+    private enum Recheck { Ready, NotFound, Archived, Changed }
+
+    // Call inside a transaction, after reading GitHub and before saving. Locks the project's team,
+    // then confirms that what the reads relied on still holds: the caller is a member, nothing is
+    // archived, and the same repository is still connected.
+    private static async Task<Recheck> LockAndRecheckAsync(
+        SpecThreadDbContext db, string userId, Guid requirementId, ProjectRepository repository, CancellationToken cancel)
+    {
+        var found = await RequirementEndpoints.LockForMember(db, userId, requirementId, cancel);
+        if (found is null) return Recheck.NotFound;
+        if (found.Archived) return Recheck.Archived;
+        var connected = await db.ProjectRepositories.AnyAsync(r => r.ProjectId == found.Requirement.ProjectId &&
+            r.InstallationId == repository.InstallationId && r.RepositoryId == repository.RepositoryId, cancel);
+        return connected ? Recheck.Ready : Recheck.Changed;
+    }
+
     // Oldest first by GitHub's own date, so the list reads as a timeline.
     private static async Task<List<EvidenceResponse>> ReadAsync(SpecThreadDbContext db, Guid requirementId, CancellationToken cancel) =>
         (await db.RequirementEvidence.AsNoTracking().Where(e => e.RequirementId == requirementId)
@@ -412,6 +466,12 @@ private static void ApplyChecks(RequirementEvidence evidence, GitHubResult<GitHu
     private static ProblemHttpResult NoRepository() =>
         TypedResults.Problem("Connect a GitHub repository to this project first.", statusCode: StatusCodes.Status409Conflict);
 
+    private static ProblemHttpResult LimitReached() =>
+        TypedResults.Problem($"A requirement can have at most {MaxEvidence} evidence links.", statusCode: StatusCodes.Status409Conflict);
+
+    private static ProblemHttpResult Changed() => TypedResults.Problem(
+        "The evidence changed while it was being read from GitHub. Reload and try again.", statusCode: StatusCodes.Status409Conflict);
+
     private static ProblemHttpResult AlreadyLinked(string label) =>
         TypedResults.Problem($"{label} is already linked to this requirement.", statusCode: StatusCodes.Status409Conflict);
 

app/api/GitHub/RepositoryEndpoints.cs

@@ -86,6 +86,14 @@ private static async Task<Results<Ok<ProjectRepositoryEnvelope>, ValidationProbl
         }
         if (installation != GitHubFailure.None) return Failed(installation);
 
+        // GitHub was asked without holding the team lock. Take it now and check the caller's role
+        // and the project's state again, so a demotion or an archive in the meantime wins.
+        await using var transaction = await db.Database.BeginTransactionAsync(cancel);
+        var locked = await ProjectEndpoints.LockProjectForCaller(db, projectId, userId, cancel);
+        if (locked is null) return TypedResults.NotFound();
+        if (!await db.CanManageTeam(locked.TeamId, userId, cancel)) return ProjectEndpoints.ManagersOnly();
+        if (locked.ArchivedAt is not null) return ProjectEndpoints.Archived();
+
         var repository = await db.ProjectRepositories.SingleOrDefaultAsync(r => r.ProjectId == projectId, cancel);
         if (repository is null)
         {
@@ -108,6 +116,7 @@ private static async Task<Results<Ok<ProjectRepositoryEnvelope>, ValidationProbl
             // Two connects at once: the other one created the row first.
             return TypedResults.Problem("The repository connection changed while saving. Reload and try again.", statusCode: StatusCodes.Status409Conflict);
         }
+        await transaction.CommitAsync(cancel);
 
         return TypedResults.Ok(new ProjectRepositoryEnvelope(ToResponse(repository)));
     }
@@ -116,13 +125,16 @@ private static async Task<Results<NoContent, NotFound, ProblemHttpResult>> Disco
         Guid projectId, ClaimsPrincipal user, SpecThreadDbContext db, CancellationToken cancel)
     {
         var userId = user.CallerId();
-        var project = await db.ProjectsFor(userId).AsNoTracking().SingleOrDefaultAsync(p => p.Id == projectId, cancel);
+        // Serialize with archiving and role changes in this project's team.
+        await using var transaction = await db.Database.BeginTransactionAsync(cancel);
+        var project = await ProjectEndpoints.LockProjectForCaller(db, projectId, userId, cancel);
         if (project is null) return TypedResults.NotFound();
         if (!await db.CanManageTeam(project.TeamId, userId, cancel)) return ProjectEndpoints.ManagersOnly();
         if (project.ArchivedAt is not null) return ProjectEndpoints.Archived();
 
         // Idempotent: disconnecting a project with no repository succeeds.
         await db.ProjectRepositories.Where(r => r.ProjectId == projectId).ExecuteDeleteAsync(cancel);
+        await transaction.CommitAsync(cancel);
         return TypedResults.NoContent();
     }
 

app/api/Requirements/RequirementEndpoints.cs

@@ -162,7 +162,8 @@ private static async Task<Results<Ok<RequirementResponse>, NotFound, ProblemHttp
 
     // Like FindForMember, but locks the project's team first and then reads the requirement, so
     // the archived states and membership it sees cannot change before this transaction ends.
-    private static async Task<MemberRequirement?> LockForMember(
+    // Call inside a transaction. Every requirement-scoped write (evidence, reviews) uses it.
+    internal static async Task<MemberRequirement?> LockForMember(
         SpecThreadDbContext db, string userId, Guid requirementId, CancellationToken cancel)
     {
         var projectId = await db.Requirements.Where(r => r.Id == requirementId).Select(r => (Guid?)r.ProjectId).SingleOrDefaultAsync(cancel);
@@ -209,7 +210,10 @@ private static ProblemHttpResult Stale() => TypedResults.Problem(
         r.Id, r.ProjectId, r.Title, r.Description, r.CreatedBy, r.Version, r.CreatedAt, r.UpdatedAt, r.ArchivedAt,
         criteria.Select(c => new CriterionResponse(c.Id, c.Text, c.Position)).ToList());
 
-    private sealed record MemberRequirement(Requirement Requirement, bool ProjectArchived);
+    internal sealed record MemberRequirement(Requirement Requirement, bool ProjectArchived)
+    {
+        public bool Archived => ProjectArchived || Requirement.ArchivedAt is not null;
+    }
 
     private sealed record ValidInput(string Title, string Description, List<string> Criteria);
 }

app/api/Requirements/ReviewEndpoints.cs

@@ -28,7 +28,7 @@ public static void MapReviewEndpoints(this IEndpointRouteBuilder app)
     private static async Task<Results<Ok<List<ReviewResponse>>, NotFound>> List(
         Guid requirementId, ClaimsPrincipal user, SpecThreadDbContext db, CancellationToken cancel)
     {
-        if (await FindAsync(db, user.CallerId(), requirementId, cancel) is null) return TypedResults.NotFound();
+        if (!await IsMemberAsync(db, user.CallerId(), requirementId, cancel)) return TypedResults.NotFound();
         var reviews = await db.RequirementReviews.AsNoTracking().Where(r => r.RequirementId == requirementId)
             .OrderByDescending(r => r.DecidedAt).ThenByDescending(r => r.Id).ToListAsync(cancel);
         return TypedResults.Ok(reviews.ConvertAll(ToResponse));
@@ -46,16 +46,20 @@ private static async Task<Results<Created<ReviewResponse>, ValidationProblem, No
         if (errors.Any) return TypedResults.ValidationProblem(errors.ToDictionary());
 
         var userId = user.CallerId();
-        var scope = await FindAsync(db, userId, requirementId, cancel);
-        if (scope is null) return TypedResults.NotFound();
-        if (scope.Archived) return ProjectEndpoints.Archived();
+        // Serialize with edits, archiving, and membership changes in this project's team, so the
+        // decision is checked against what is there when it is saved.
+        await using var transaction = await db.Database.BeginTransactionAsync(cancel);
+        var found = await RequirementEndpoints.LockForMember(db, userId, requirementId, cancel);
+        if (found is null) return TypedResults.NotFound();
+        if (found.Archived) return ProjectEndpoints.Archived();
+        var requirement = found.Requirement;
         // A second person has to look: nobody decides on a requirement they wrote.
-        if (scope.CreatedBy == userId)
+        if (requirement.CreatedBy == userId)
         {
             return TypedResults.Problem("You created this requirement, so another project member has to review it.", statusCode: StatusCodes.Status403Forbidden);
         }
         // Nobody decides on text they have not seen.
-        if (scope.Version != request.Version)
+        if (requirement.Version != request.Version)
         {
             return TypedResults.Problem("This requirement changed since it was loaded. Reload it and review the current version.", statusCode: StatusCodes.Status409Conflict);
         }
@@ -69,13 +73,14 @@ private static async Task<Results<Created<ReviewResponse>, ValidationProblem, No
             RequirementId = requirementId,
             Decision = decision!,
             Note = note!,
-            RequirementVersion = scope.Version,
+            RequirementVersion = requirement.Version,
             Evidence = JsonSerializer.Serialize(evidence.ConvertAll(e => new ReviewedEvidence(e.Id, e.Kind, Label(e), e.Title, e.State)), SnapshotJson),
             DecidedBy = userId,
             DecidedAt = Clock.UtcNow(),
         };
         db.RequirementReviews.Add(review);
         await db.SaveChangesAsync(cancel);
+        await transaction.CommitAsync(cancel);
         return TypedResults.Created($"/requirements/{requirementId}/reviews/{review.Id}", ToResponse(review));
     }
 
@@ -94,17 +99,14 @@ private static List<ReviewedEvidence> ReadSnapshot(RequirementReview review) =>
         _ => evidence.Sha is { Length: >= 7 } sha ? sha[..7] : "",
     };
 
-    // Null when the requirement does not exist or the caller is not a member of its project.
-    private static Task<Scope?> FindAsync(SpecThreadDbContext db, string userId, Guid requirementId, CancellationToken cancel) =>
-        db.Requirements.AsNoTracking().Where(r => r.Id == requirementId)
-            .Join(db.ProjectsFor(userId), r => r.ProjectId, p => p.Id,
-                (r, p) => new Scope(r.CreatedBy, r.Version, r.ArchivedAt != null || p.ArchivedAt != null))
-            .SingleOrDefaultAsync(cancel);
+    // False when the requirement does not exist or the caller is not a member of its project.
+    private static Task<bool> IsMemberAsync(SpecThreadDbContext db, string userId, Guid requirementId, CancellationToken cancel) =>
+        db.Requirements.Where(r => r.Id == requirementId)
+            .Join(db.ProjectsFor(userId), r => r.ProjectId, p => p.Id, (r, p) => r.Id)
+            .AnyAsync(cancel);
 
     private static ReviewResponse ToResponse(RequirementReview r) =>
         new(r.Id, r.RequirementId, r.Decision, r.Note, r.RequirementVersion, ReadSnapshot(r), r.DecidedBy, r.DecidedAt);
-
-    private sealed record Scope(string CreatedBy, int Version, bool Archived);
 }
 
 internal sealed record RecordReviewRequest(string? Decision, string? Note, int? Version);

app/web/src/app/projects/[projectId]/requirements/[requirementId]/page.tsx

@@ -10,6 +10,9 @@ import { formatDate } from "@/lib/projects";
 
 type Props = PageProps<"/projects/[projectId]/requirements/[requirementId]">;
 
+/** The evidence actions on this page may read GitHub for up to a minute (ADR-048). */
+export const maxDuration = 60;
+
 export async function generateMetadata({ params }: Props): Promise<Metadata> {
   const { projectId, requirementId } = await params;
   return { title: (await requireRequirement(projectId, requirementId)).title };

app/web/src/app/projects/evidence-actions.ts

@@ -10,6 +10,11 @@ export type EvidenceActionState = { error: string | null };
 
 const GITHUB_DOWN = "GitHub could not be reached. Try again in a moment.";
 const GITHUB_NOT_SET_UP = "GitHub is not set up for this SpecThread deployment yet.";
+/**
+ * Linking and refreshing read GitHub, one call per linked item and per release to compare, so
+ * they get longer than an ordinary API call. Keep this below the page's maxDuration (ADR-048).
+ */
+const GITHUB_WRITE_TIMEOUT_MS = 55_000;
 
 const refresh = (projectId: string, requirementId: string) =>
   revalidatePath(`/projects/${projectId}/requirements/${requirementId}`);
@@ -29,9 +34,10 @@ export async function linkEvidence(
   if (!isUuid(projectId) || !isUuid(requirementId)) return fail(failed);
 
   try {
-    const response = await apiFetch(`/requirements/${requirementId}/evidence`, { method: "POST", body: JSON.stringify({ reference }) });
+    const response = await apiFetch(`/requirements/${requirementId}/evidence`, { method: "POST", body: JSON.stringify({ reference }), timeoutMs: GITHUB_WRITE_TIMEOUT_MS });
     if (response.status === 400) return fail(fieldError(await response.json(), "reference") ?? failed);
-    // 409 explains itself: no repository, already linked, archived, the limit, or the app uninstalled.
+    // 409 explains itself: no repository, already linked, archived, the limit, the app uninstalled,
+    // or the evidence changing while GitHub was read.
     if (response.status === 409) return fail(problemDetail(await response.json()) ?? failed);
     if (response.status === 502) return fail(GITHUB_DOWN);
     if (response.status === 503) return fail(GITHUB_NOT_SET_UP);
@@ -50,7 +56,7 @@ export async function refreshEvidence(projectId: string, requirementId: string):
   const failed = "The evidence could not be refreshed. Please try again.";
   if (!isUuid(projectId) || !isUuid(requirementId)) return { error: failed };
   try {
-    const response = await apiFetch(`/requirements/${requirementId}/evidence/refresh`, { method: "POST" });
+    const response = await apiFetch(`/requirements/${requirementId}/evidence/refresh`, { method: "POST", timeoutMs: GITHUB_WRITE_TIMEOUT_MS });
     if (response.status === 409) return { error: problemDetail(await response.json()) ?? failed };
     if (response.status === 502) return { error: GITHUB_DOWN };
     if (response.status === 503) return { error: GITHUB_NOT_SET_UP };