From ac829e8dee9bd2abc012195e0192faee45e7c1b5 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 15 Sep 2026 12:20:19 -0700 Subject: [PATCH] fix(web): gh auth refresh waits out a probe that started before it was asked for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the shared single-flight probe (previous commit) a `refresh=true` request that landed while a probe was already running simply joined it. That probe may have started before `gh auth login` completed, so the refresh returned "not authenticated" and cached it for the full 5-minute TTL — the composer pill kept offering /github-auth right after a successful login. A refresh now accepts only a probe that started at or after the refresh was requested: it awaits the in-flight one, then starts (or joins) the next. Still only one `gh` runs at a time. A finished task whose done-callback has not run yet is treated as absent so the loop cannot spin on it. Live probe (real route, fake `gh` reading login state at start, 1 s answer): PR head: refresh=true right after login -> authenticated False, cached False fixed: refresh=true right after login -> authenticated True, cached True; 5 concurrent requests -> peak concurrent probes 1 Co-authored-by: aron-intframe --- hermes_cli/web_routers/git.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/hermes_cli/web_routers/git.py b/hermes_cli/web_routers/git.py index f24f839909..a7537352f8 100644 --- a/hermes_cli/web_routers/git.py +++ b/hermes_cli/web_routers/git.py @@ -58,6 +58,7 @@ async def git_status_route(path: str): _GH_AUTH_TTL_S = 300.0 _gh_auth_cache: Optional[tuple] = None # (monotonic_ts, payload) _gh_auth_probe_task: Optional[asyncio.Task] = None +_gh_auth_probe_started = 0.0 # monotonic start of _gh_auth_probe_task def _probe_gh_auth() -> dict: @@ -83,16 +84,24 @@ def _clear_gh_auth_probe_task(completed_task: asyncio.Task) -> None: async def gh_auth_status_route(refresh: bool = False): """``{"available", "authenticated"}`` for the `gh` CLI; cached 5 min (``refresh=true`` bypasses so the pill withdraws right after a login).""" - global _gh_auth_cache, _gh_auth_probe_task - if not refresh and _gh_auth_cache and time.monotonic() - _gh_auth_cache[0] < _GH_AUTH_TTL_S: + global _gh_auth_cache, _gh_auth_probe_task, _gh_auth_probe_started + asked = time.monotonic() + if not refresh and _gh_auth_cache and asked - _gh_auth_cache[0] < _GH_AUTH_TTL_S: return _gh_auth_cache[1] - if _gh_auth_probe_task is None: - _gh_auth_probe_task = asyncio.create_task(asyncio.to_thread(_probe_gh_auth)) - _gh_auth_probe_task.add_done_callback(_clear_gh_auth_probe_task) - probe_task = _gh_auth_probe_task - # Shield the shared probe: disconnecting one requester must not cancel the - # probe that other refreshes/cache misses are awaiting. - payload = await asyncio.shield(probe_task) + while True: + if _gh_auth_probe_task is None or _gh_auth_probe_task.done(): + _gh_auth_probe_task = asyncio.create_task(asyncio.to_thread(_probe_gh_auth)) + _gh_auth_probe_started = time.monotonic() + _gh_auth_probe_task.add_done_callback(_clear_gh_auth_probe_task) + probe_task, started = _gh_auth_probe_task, _gh_auth_probe_started + # Shield the shared probe: disconnecting one requester must not cancel the + # probe that other refreshes/cache misses are awaiting. + payload = await asyncio.shield(probe_task) + # A refresh must not accept a probe that started before it was asked for (it may predate + # `gh auth login`, and its answer would then be cached for the full TTL): wait that one out, + # then start or join the next. Still only one `gh` runs at a time. + if not refresh or started >= asked: + break _gh_auth_cache = (time.monotonic(), payload) return payload