diff --git a/CHANGELOG.md b/CHANGELOG.md index a5af2e0..f235f11 100644 --- a/CHANGELOG.md +++ b/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 (`\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 `…` tags around the user's email instead of rendering them bold. The subtitle diff --git a/cli/commands/refresh_marketplace.py b/cli/commands/refresh_marketplace.py index 9af28ae..4764f41 100644 --- a/cli/commands/refresh_marketplace.py +++ b/cli/commands/refresh_marketplace.py @@ -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 (`\\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: diff --git a/tests/test_cli_refresh_marketplace.py b/tests/test_cli_refresh_marketplace.py index fa1bf37..62b37fc 100644 --- a/tests/test_cli_refresh_marketplace.py +++ b/tests/test_cli_refresh_marketplace.py @@ -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 ` 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"])