perf(cli): use git ls-remote in refresh-marketplace --check (~8s -> ~1s) (#313)
* perf(cli): use git ls-remote in refresh-marketplace --check (~8s -> ~1s) The SessionStart hook fires agnes refresh-marketplace --check on every Claude Code session in every workspace. The detector used to run git fetch origin against the per-user marketplace bare repo just to update FETCH_HEAD for a HEAD-vs-FETCH_HEAD comparison — paying the full git object/metadata download (~8s) even on the (overwhelming) no-change path. Replace with git ls-remote origin HEAD: one HTTPS round-trip, one line of text, no objects transferred. Compare the returned SHA against local HEAD via the new _remote_head_sha and _local_head_sha helpers; emit the /update-agnes-plugins hook JSON on mismatch, silent on match. Same PAT wiring (AGNES_TOKEN in env, never on argv), same exit-code contract (remote-read failure -> exit 1 so the hook's || true swallows it). The default and --bootstrap paths still do real git fetch + reset --hard — they need the objects. * docs(cli): update --check help text to mention git ls-remote, not git fetch Followup to the previous commit — the --check flag's user-facing help string still described the old + FETCH_HEAD comparison. Updated to match the new ls-remote-based implementation. --------- Co-authored-by: Minas Arustamyan <arustamyan.minas@gmail.com>
This commit is contained in:
parent
8b5b0f8ef5
commit
7907b8082e
3 changed files with 142 additions and 65 deletions
13
CHANGELOG.md
13
CHANGELOG.md
|
|
@ -10,6 +10,19 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
### Changed
|
||||
- `agnes refresh-marketplace --check` (the SessionStart-hook detector
|
||||
that fires on every Claude Code session start in every workspace)
|
||||
now uses `git ls-remote origin HEAD` instead of `git fetch origin`
|
||||
to learn whether the remote marketplace has changed. ls-remote
|
||||
transfers one line of text (`<sha>\tHEAD`) over a single HTTPS
|
||||
round-trip — no git objects, no metadata — so the hook completes
|
||||
in ~0.5–1 s instead of the ~8 s a full fetch took. Detection logic
|
||||
is unchanged (compare local `HEAD` SHA to remote `HEAD` SHA, emit
|
||||
the `/update-agnes-plugins` hint JSON on mismatch, silent on
|
||||
match). The slash-command and `--bootstrap` paths still do real
|
||||
`git fetch + reset --hard` — they actually need the objects.
|
||||
|
||||
### Fixed
|
||||
- `/me/activity` hero subtitle showed literal `<strong>…</strong>` tags
|
||||
around the user's email instead of rendering them bold. The subtitle
|
||||
|
|
|
|||
|
|
@ -10,11 +10,13 @@ Three call paths share the same code:
|
|||
inside Claude Code so the user sees install/update progress in the
|
||||
transcript.
|
||||
- `agnes refresh-marketplace --check` — SessionStart hook context.
|
||||
Lightweight detector: `git fetch` only (no reset, no plugin
|
||||
install/update side effects), compares local `HEAD` vs `FETCH_HEAD`,
|
||||
emits a Claude Code hook JSON message pointing the user at
|
||||
`/update-agnes-plugins` when there are remote changes. Silent
|
||||
otherwise.
|
||||
Lightweight detector: `git ls-remote origin HEAD` only (no fetch,
|
||||
no reset, no plugin install/update side effects), compares the
|
||||
remote HEAD SHA against the local `HEAD` SHA, emits a Claude Code
|
||||
hook JSON message pointing the user at `/update-agnes-plugins`
|
||||
when they differ. Silent otherwise. ls-remote is ~0.5–1 s vs ~8 s
|
||||
for fetch — matters because every Claude Code session start in
|
||||
every workspace fires this hook.
|
||||
|
||||
Reconcile (default + --bootstrap paths) is version-aware (install
|
||||
missing / update on version diff / skip on match). Server-side stack
|
||||
|
|
@ -58,13 +60,14 @@ def refresh_marketplace(
|
|||
check: bool = typer.Option(
|
||||
False, "--check",
|
||||
help=(
|
||||
"Detect-only mode for the SessionStart hook. Runs `git fetch` "
|
||||
"and compares local HEAD with remote FETCH_HEAD. When they "
|
||||
"differ, emits a Claude Code hook JSON message hinting the "
|
||||
"user at `/update-agnes-plugins`. No `git reset`, no plugin "
|
||||
"install/update side effects — fast, invisible when nothing "
|
||||
"changed, fully recoverable interactively via the slash "
|
||||
"command."
|
||||
"Detect-only mode for the SessionStart hook. Runs "
|
||||
"`git ls-remote origin HEAD` and compares the returned SHA "
|
||||
"with local HEAD. When they differ, emits a Claude Code "
|
||||
"hook JSON message hinting the user at "
|
||||
"`/update-agnes-plugins`. No `git fetch`, no `git reset`, "
|
||||
"no plugin install/update side effects — fast, invisible "
|
||||
"when nothing changed, fully recoverable interactively "
|
||||
"via the slash command."
|
||||
),
|
||||
),
|
||||
bootstrap: bool = typer.Option(
|
||||
|
|
@ -128,11 +131,15 @@ def refresh_marketplace(
|
|||
|
||||
# --check: lightweight detector. Don't fetch+reset, don't reconcile
|
||||
# plugins — that's the slash command's job. Just check whether the
|
||||
# remote has new content and tell the user if so.
|
||||
# remote has new content and tell the user if so. `git ls-remote`
|
||||
# fetches one line of text (the remote HEAD ref) instead of all
|
||||
# git objects — ~0.5–1 s vs ~8 s for `git fetch`.
|
||||
if check:
|
||||
if not _git_fetch_only(token):
|
||||
remote_sha = _remote_head_sha(token)
|
||||
if remote_sha is None:
|
||||
raise typer.Exit(1)
|
||||
if _has_remote_changes():
|
||||
local_sha = _local_head_sha()
|
||||
if local_sha is not None and local_sha != remote_sha:
|
||||
_emit_check_hook_message()
|
||||
raise typer.Exit(0)
|
||||
|
||||
|
|
@ -366,27 +373,62 @@ def _git_fetch_only(token: str) -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def _has_remote_changes() -> bool:
|
||||
"""Return True iff local HEAD differs from remote FETCH_HEAD.
|
||||
def _remote_head_sha(token: str) -> Optional[str]:
|
||||
"""Return the remote `HEAD` SHA via `git ls-remote`, or None on failure.
|
||||
|
||||
Caller must have already run `git fetch origin`. Any rev-parse failure
|
||||
(missing FETCH_HEAD, broken repo) is treated as "no detectable changes"
|
||||
so the hook stays quiet rather than surfacing a misleading hint.
|
||||
`ls-remote` returns one line of text per ref (`<sha>\\tHEAD`); no git
|
||||
objects are transferred — orders of magnitude cheaper than a full
|
||||
`git fetch` for the SessionStart-hook detector path. Same PAT wiring
|
||||
as `_git_fetch_only`: token in env, never on argv. Surfaces stderr
|
||||
on failure so auth/network errors aren't swallowed silently — the
|
||||
`--check` caller turns failure into exit 1.
|
||||
"""
|
||||
env = {**os.environ, "AGNES_TOKEN": token}
|
||||
cmd = [
|
||||
"git",
|
||||
"-c", f"credential.helper={_CREDENTIAL_HELPER}",
|
||||
"-C", str(CLONE_DIR),
|
||||
"ls-remote", "origin", "HEAD",
|
||||
]
|
||||
try:
|
||||
result = subprocess.run(
|
||||
cmd, env=env, capture_output=True, text=True,
|
||||
encoding="utf-8", errors="replace", check=False,
|
||||
)
|
||||
except FileNotFoundError:
|
||||
typer.echo("error: `git` not found in PATH; cannot check marketplace.", err=True)
|
||||
return None
|
||||
if result.returncode != 0:
|
||||
if result.stdout:
|
||||
typer.echo(result.stdout, err=True)
|
||||
if result.stderr:
|
||||
typer.echo(result.stderr, err=True)
|
||||
return None
|
||||
first_line = result.stdout.strip().splitlines()[:1]
|
||||
if not first_line:
|
||||
return None
|
||||
sha = first_line[0].split()[0].strip()
|
||||
return sha or None
|
||||
|
||||
|
||||
def _local_head_sha() -> Optional[str]:
|
||||
"""Return the local `HEAD` SHA, or None on any rev-parse failure.
|
||||
|
||||
None means "can't determine local state" — the `--check` caller
|
||||
treats that as "stay silent" rather than emitting a misleading
|
||||
updates-available hint built on a missing left-hand side.
|
||||
"""
|
||||
try:
|
||||
local = subprocess.run(
|
||||
result = subprocess.run(
|
||||
["git", "-C", str(CLONE_DIR), "rev-parse", "HEAD"],
|
||||
capture_output=True, text=True, encoding="utf-8", errors="replace", check=False,
|
||||
)
|
||||
remote = subprocess.run(
|
||||
["git", "-C", str(CLONE_DIR), "rev-parse", "FETCH_HEAD"],
|
||||
capture_output=True, text=True, encoding="utf-8", errors="replace", check=False,
|
||||
)
|
||||
except FileNotFoundError:
|
||||
return False
|
||||
if local.returncode != 0 or remote.returncode != 0:
|
||||
return False
|
||||
return local.stdout.strip() != remote.stdout.strip()
|
||||
return None
|
||||
if result.returncode != 0:
|
||||
return None
|
||||
sha = result.stdout.strip()
|
||||
return sha or None
|
||||
|
||||
|
||||
def _git_fetch_and_reset(token: str) -> bool:
|
||||
|
|
|
|||
|
|
@ -663,27 +663,31 @@ def test_bootstrap_with_existing_clone_skips_clone_proceeds_to_refresh(
|
|||
# --- --check flag (SessionStart-hook detector mode) -----------------------------
|
||||
|
||||
|
||||
def _stage_rev_parse(monkeypatch, recorder, *, head: str, fetch_head: str) -> None:
|
||||
"""Wrap recorder.run so `git rev-parse HEAD` and
|
||||
`git rev-parse FETCH_HEAD` return scripted SHAs while every other
|
||||
command falls through to the recorder's normal handling.
|
||||
def _stage_rev_parse(monkeypatch, recorder, *, head: str, remote_head: str) -> None:
|
||||
"""Wrap recorder.run so `git rev-parse HEAD` returns the local SHA
|
||||
and `git ls-remote origin HEAD` returns the remote SHA, while every
|
||||
other command falls through to the recorder's normal handling.
|
||||
|
||||
Used by --check tests to drive the HEAD-vs-FETCH_HEAD comparison
|
||||
independently of the (mocked) git fetch.
|
||||
Used by --check tests to drive the local-HEAD vs remote-HEAD
|
||||
comparison independently of the (mocked) git invocation.
|
||||
"""
|
||||
real_run = recorder.run
|
||||
|
||||
def staged_run(cmd, *args, **kwargs):
|
||||
# Match the trailing rev-parse target so `-C <path>` injection
|
||||
# doesn't break the prefix.
|
||||
if "rev-parse" in cmd:
|
||||
recorder.calls.append(
|
||||
_RecordedCall(cmd=list(cmd), env=dict(kwargs.get("env") or {}))
|
||||
)
|
||||
target = cmd[-1]
|
||||
stdout = head if target == "HEAD" else fetch_head if target == "FETCH_HEAD" else ""
|
||||
return subprocess.CompletedProcess(
|
||||
args=list(cmd), returncode=0, stdout=stdout + "\n", stderr="",
|
||||
args=list(cmd), returncode=0, stdout=head + "\n", stderr="",
|
||||
)
|
||||
if "ls-remote" in cmd:
|
||||
recorder.calls.append(
|
||||
_RecordedCall(cmd=list(cmd), env=dict(kwargs.get("env") or {}))
|
||||
)
|
||||
return subprocess.CompletedProcess(
|
||||
args=list(cmd), returncode=0,
|
||||
stdout=f"{remote_head}\tHEAD\n", stderr="",
|
||||
)
|
||||
return real_run(cmd, *args, **kwargs)
|
||||
|
||||
|
|
@ -693,13 +697,13 @@ def _stage_rev_parse(monkeypatch, recorder, *, head: str, fetch_head: str) -> No
|
|||
def test_check_emits_hook_json_when_remote_changed(
|
||||
with_clone, with_token, claude_in_path, recorder, monkeypatch, tmp_path,
|
||||
):
|
||||
"""`--check` + local HEAD differs from remote FETCH_HEAD →
|
||||
"""`--check` + local HEAD differs from remote HEAD →
|
||||
Claude Code hook JSON on stdout pointing the user at
|
||||
`/update-agnes-plugins`. The hook never installs anything itself."""
|
||||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
monkeypatch.chdir(workspace)
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc123", fetch_head="def456")
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc123", remote_head="def456")
|
||||
|
||||
result = runner.invoke(refresh_marketplace_app, ["--check"])
|
||||
assert result.exit_code == 0
|
||||
|
|
@ -716,13 +720,13 @@ def test_check_emits_hook_json_when_remote_changed(
|
|||
def test_check_silent_when_remote_unchanged(
|
||||
with_clone, with_token, claude_in_path, recorder, monkeypatch, tmp_path,
|
||||
):
|
||||
"""`--check` + HEAD == FETCH_HEAD → silent exit 0, no JSON output.
|
||||
Avoids spamming the user with "updates available" on every session
|
||||
start when nothing actually changed."""
|
||||
"""`--check` + local HEAD == remote HEAD → silent exit 0, no JSON
|
||||
output. Avoids spamming the user with "updates available" on every
|
||||
session start when nothing actually changed."""
|
||||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
monkeypatch.chdir(workspace)
|
||||
_stage_rev_parse(monkeypatch, recorder, head="samehash", fetch_head="samehash")
|
||||
_stage_rev_parse(monkeypatch, recorder, head="samehash", remote_head="samehash")
|
||||
|
||||
result = runner.invoke(refresh_marketplace_app, ["--check"])
|
||||
assert result.exit_code == 0
|
||||
|
|
@ -740,7 +744,7 @@ def test_check_does_not_call_claude_plugin_anything(
|
|||
workspace.mkdir()
|
||||
monkeypatch.chdir(workspace)
|
||||
# Even WITH a remote diff, --check must stay read-only.
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc", fetch_head="def")
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc", remote_head="def")
|
||||
|
||||
result = runner.invoke(refresh_marketplace_app, ["--check"])
|
||||
assert result.exit_code == 0
|
||||
|
|
@ -766,7 +770,7 @@ def test_check_does_not_git_reset(
|
|||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
monkeypatch.chdir(workspace)
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc", fetch_head="def")
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc", remote_head="def")
|
||||
|
||||
result = runner.invoke(refresh_marketplace_app, ["--check"])
|
||||
assert result.exit_code == 0
|
||||
|
|
@ -777,32 +781,46 @@ def test_check_does_not_git_reset(
|
|||
)
|
||||
|
||||
|
||||
def test_check_runs_git_fetch(
|
||||
def test_check_runs_git_ls_remote_not_fetch(
|
||||
with_clone, with_token, claude_in_path, recorder, monkeypatch, tmp_path,
|
||||
):
|
||||
"""`--check` must run `git fetch origin` (otherwise FETCH_HEAD is
|
||||
stale and we'd compare against an old snapshot, missing real
|
||||
remote changes)."""
|
||||
"""`--check` must use `git ls-remote origin HEAD` — one HTTPS
|
||||
round-trip, no objects downloaded — and must NOT run `git fetch`.
|
||||
This is the whole point of the SessionStart-hook detector: ~0.5–1 s
|
||||
instead of ~8 s. If somebody regresses this back to fetch, this
|
||||
test catches it."""
|
||||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
monkeypatch.chdir(workspace)
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc", fetch_head="abc")
|
||||
_stage_rev_parse(monkeypatch, recorder, head="abc", remote_head="abc")
|
||||
|
||||
result = runner.invoke(refresh_marketplace_app, ["--check"])
|
||||
assert result.exit_code == 0
|
||||
|
||||
fetch_calls = [
|
||||
ls_remote_calls = [
|
||||
c for c in recorder.calls
|
||||
if c.cmd and c.cmd[0] == "git" and "fetch" in c.cmd and "origin" in c.cmd
|
||||
if c.cmd and c.cmd[0] == "git" and "ls-remote" in c.cmd
|
||||
and "origin" in c.cmd and "HEAD" in c.cmd
|
||||
]
|
||||
assert fetch_calls, (
|
||||
f"--check must run `git fetch origin`; got: {[c.cmd for c in recorder.calls]!r}"
|
||||
assert ls_remote_calls, (
|
||||
f"--check must run `git ls-remote origin HEAD`; got: "
|
||||
f"{[c.cmd for c in recorder.calls]!r}"
|
||||
)
|
||||
# Same credential helper wiring as the default mode — PAT in env, not argv.
|
||||
fetch = fetch_calls[0]
|
||||
assert "-c" in fetch.cmd
|
||||
assert fetch.cmd[fetch.cmd.index("-c") + 1].startswith("credential.helper=")
|
||||
assert fetch.env.get("AGNES_TOKEN") == with_token
|
||||
ls_remote = ls_remote_calls[0]
|
||||
assert "-c" in ls_remote.cmd
|
||||
assert ls_remote.cmd[ls_remote.cmd.index("-c") + 1].startswith("credential.helper=")
|
||||
assert ls_remote.env.get("AGNES_TOKEN") == with_token
|
||||
|
||||
# No `git fetch` — that's the slow path we replaced.
|
||||
fetch_calls = [
|
||||
c for c in recorder.calls
|
||||
if c.cmd and c.cmd[0] == "git" and "fetch" in c.cmd
|
||||
]
|
||||
assert fetch_calls == [], (
|
||||
f"--check must NOT run `git fetch` (slow path); got: "
|
||||
f"{[c.cmd for c in fetch_calls]!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_check_no_clone_silent_exit_zero(tmp_path, monkeypatch, with_token, recorder):
|
||||
|
|
@ -817,15 +835,19 @@ def test_check_no_clone_silent_exit_zero(tmp_path, monkeypatch, with_token, reco
|
|||
assert recorder.calls == []
|
||||
|
||||
|
||||
def test_check_fetch_failure_exits_one(
|
||||
def test_check_ls_remote_failure_exits_one(
|
||||
with_clone, with_token, claude_in_path, recorder, monkeypatch, tmp_path,
|
||||
):
|
||||
"""A failed `git fetch` (network down, auth rejected, etc.) → exit 1
|
||||
so the surrounding `|| true` in the hook command swallows it cleanly.
|
||||
No hook JSON is emitted (we don't know if the remote changed)."""
|
||||
"""A failed `git ls-remote` (network down, auth rejected, etc.) →
|
||||
exit 1 so the surrounding `|| true` in the hook command swallows it
|
||||
cleanly. No hook JSON is emitted (we don't know if the remote
|
||||
changed)."""
|
||||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
monkeypatch.chdir(workspace)
|
||||
# `("git", "-c")` matches the credential-helper wiring shared by
|
||||
# ls-remote and fetch — fine here since ls-remote is the only git
|
||||
# subprocess --check runs.
|
||||
recorder.script(("git", "-c"), returncode=1, stderr="fatal: unable to access ...")
|
||||
|
||||
result = runner.invoke(refresh_marketplace_app, ["--check"])
|
||||
|
|
|
|||
Loading…
Reference in a new issue