Skip to content

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

Merged
TahubCS merged 1 commit into
mainfrom
fix/evidence-locks-and-timeouts
Oct 20, 2026
Merged

TahubCS merged 1 commit into
mainfrom
fix/evidence-locks-and-timeouts

Conversation

@TahubCS

@TahubCS TahubCS commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Problem

A full review of main found no failing checks, but five untested problems:

  1. Refreshing evidence failed for requirements with many links. The web app gives every API call 10 seconds (apiFetch), while a refresh makes several GitHub calls per linked item and one per release/change pair. Around 20 or more linked pull requests shows "The evidence could not be refreshed."
  2. A refresh that raced an unlink answered an unhandled 500. Refresh loads the links, reads GitHub, then saves; a row deleted in between made SaveChangesAsync throw DbUpdateConcurrencyException.
  3. Several writes skipped the team lock that requirement, project, and document writes take: evidence link/refresh/unlink, review decisions, and repository connect/disconnect. They could commit into a project being archived, or for a member being removed.
  4. The 50-link limit could be exceeded by two links added at once.
  5. Docs disagreed with the code (docs/API.md, docs/ARCHITECTURE.md).

Solution

  • Team locks (ADR-048). These six writes now run in a transaction that locks the project's team and re-reads membership and archived state, reusing RequirementEndpoints.LockForMember (now internal) and ProjectEndpoints.LockProjectForCaller. Link, refresh, and connect read GitHub before taking the lock, so a slow GitHub never stalls the team's other writes. They then lock, check again (still a member, nothing archived, same repository connected), and save.
  • Race handling. A row removed while GitHub was being read now answers 409 "The evidence changed while it was being read from GitHub. Reload and try again." and saves nothing. The web action already shows a 409's detail. DbUpdateConcurrencyException is caught before DbUpdateException, so a removed release is no longer misreported as "already linked".
  • Link limit. Links are counted again under the lock.
  • Timeout. apiFetch accepts an optional per-call timeoutMs. Linking and refreshing use 55 seconds, and the requirement page sets maxDuration = 60 so Vercel lets its server actions run that long. Every other call keeps the 10-second limit and the GET retry.
  • Docs.
    • API.md: repository writes are team Owner/Admin; the retired add-member paragraph is removed; a stray code fence that rendered the rest of the file as code is removed; a stale ownership note is removed; the new 409 is documented.
    • ARCHITECTURE.md: the GitHub App integration is no longer called pending, and a second stale ownership note is removed.
    • ADR-048 and a HANDOFF entry are added.

Validation

  • New tests first failed against the unchanged code, then passed with the fix:
    • tests/schema/evidence.spec.ts: link/refresh/unlink wait for the team lock and return 409 once the project is archived; a refresh racing an unlink returns 409 (not 500) and saves nothing; two concurrent links at 49 give one 201 and one 409, and the requirement holds 50.
    • tests/schema/reviews.spec.ts: a decision waits for the lock and is refused if the project was archived or the requirement edited meanwhile.
    • tests/schema/github-repository.spec.ts: connect and disconnect wait for the lock and refuse an archived project.
    • tests/e2e/evidence.spec.ts: a refresh delayed past 10 seconds by the test API proxy now completes (before: TimeoutError).
    • New helper tests/support/team-lock.ts holds a team's row lock from the test database, the way a concurrent write would.
  • CI=1 npm test: 347 passed (341 existing plus 6 new).
  • Passed: npm run lint, npm run typecheck, npm run build:api, dotnet format app/api --verify-no-changes --no-restore, dotnet ef migrations has-pending-model-changes (no changes), git diff --check.
  • Browser tests ran locally on Chromium 141 instead of the build Playwright 1.63 expects; CI uses the expected build.

Screenshots

None. No visual change.

Migrations and environment

None. There is no schema change and no new environment variable.

Known limitations

  • The first refresh of a requirement with many releases and many merged pull requests can still approach the 55-second limit, because each release/change pair is one GitHub compare call. Later refreshes reuse earlier "included" answers.
  • maxDuration = 60 has not been verified on Vercel. After merging, refresh a requirement with many links on spec-thread.com to confirm.
  • A write may now wait briefly for another write in the same team.

Generated by Claude Code

…hes time

- Evidence link/refresh/unlink, review decisions, and repository
  connect/disconnect now lock the project's team and re-check membership
  and archived state before saving, like requirement writes. GitHub is
  read before the lock is taken, so a slow GitHub never stalls the
  team's other writes.
- A refresh that races an unlink answers 409 instead of an unhandled
  500, and saves nothing.
- Links are counted again under the lock, so two links at once cannot
  pass the 50-link limit.
- apiFetch accepts a per-call timeoutMs. Linking and refreshing get 55
  seconds instead of 10, and the requirement page sets maxDuration = 60.
- Docs: repository writes are team Owner/Admin, remove the retired
  add-member paragraph, a stray code fence, and stale ownership notes,
  and stop calling the GitHub App integration pending. ADR-048 records
  the decision.
@vercel

vercel Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
web Ready Ready Preview Oct 20, 2026 6:13pm UTC

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-20T18:12:58.378028Z ed2e16b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f4bb5d2f-0b69-41e3-975c-bb0cd980d504
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@TahubCS
TahubCS merged commit b069093 into main Oct 20, 2026
6 checks passed

This branch was successfully deployed

1 active deployment
Preview — ed2e16b8 Deployed Oct 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants