diff --git a/README.md b/README.md index 6ec85e2..5fbfcfb 100644 --- a/README.md +++ b/README.md @@ -421,6 +421,14 @@ goodeye workflows sync target list [--json|--table] goodeye workflows sync target remove Remove a configured local sync target by its directory. +goodeye workflows sync auto [on [--interval SECONDS] | off] [--json|--table] + Turn opt-in automatic background pulls on or off, or (with no argument) + show the current setting and the last automatic-pull time. When on, the + CLI keeps the safe set of your configured targets (new and behind-registry + workflows) fresh in the background after a command finishes, no more often + than the interval (default 3600 seconds). It never overwrites local edits, + never deletes a local copy, and never blocks your command. Off by default. + goodeye workflows sync pull [SLUG...] [--target DIR] [--force] [--yes] [--json|--table] Pull registry workflows down to the configured directories, each written to //SKILL.md. Omit slugs to pull everything in scope. A diff --git a/src/goodeye_cli/app.py b/src/goodeye_cli/app.py index 813f2bd..ff54c20 100644 --- a/src/goodeye_cli/app.py +++ b/src/goodeye_cli/app.py @@ -2,15 +2,18 @@ from __future__ import annotations +import atexit import os import sys from collections.abc import Sequence +from datetime import UTC, datetime from pathlib import Path import typer from rich.console import Console -from goodeye_cli import __version__ +from goodeye_cli import __version__, sync +from goodeye_cli import background as background_sync from goodeye_cli import update as update_checks from goodeye_cli.commands import auth as auth_cmds from goodeye_cli.commands import design as design_cmd @@ -27,6 +30,7 @@ from goodeye_cli.commands import verifiers as verifiers_cmds from goodeye_cli.commands import whoami as whoami_cmd from goodeye_cli.commands import workflows as workflows_cmds +from goodeye_cli.config import get_api_key, get_config_paths from goodeye_cli.errors import GoodeyeError app = typer.Typer( @@ -97,6 +101,33 @@ def _maybe_emit_background_update_notice() -> None: return +def _maybe_register_auto_pull_tail() -> None: + """Register the automatic-pull tail when the cheap synchronous gate passes. + + Reuses the best-effort ethos of the update notice: a broken gate must never + break the user's command, so the whole body is wrapped in a swallow. The + network work runs only later, in the ``atexit`` tail, so even an eligible + invocation pays nothing here beyond a couple of small local JSON reads. + """ + try: + args = _get_background_notice_args() + paths = get_config_paths() + config = sync.load_sync_config(paths) + state = sync.load_sync_state(paths) + if background_sync.should_run_auto_pull( + args, + os.environ, + config, + state, + authenticated=get_api_key(paths) is not None, + now=datetime.now(UTC), + ): + atexit.register(background_sync.run_auto_pull_tail) + except Exception: + # A broken gate must never break the user's command. + return + + @app.callback() def _root( version: bool = typer.Option( @@ -110,6 +141,7 @@ def _root( """Global options processed before any subcommand.""" _ = version _maybe_emit_background_update_notice() + _maybe_register_auto_pull_tail() def main() -> None: diff --git a/src/goodeye_cli/background.py b/src/goodeye_cli/background.py new file mode 100644 index 0000000..1cfde5a --- /dev/null +++ b/src/goodeye_cli/background.py @@ -0,0 +1,208 @@ +"""Best-effort automatic-pull tail for the local workflow mirror. + +When the user opts in (`workflows sync auto on`), the CLI keeps the safe set of +its configured sync targets fresh in the background. The work runs as a tail +after the user's command finishes (registered through ``atexit`` in ``app.py``), +so it never delays or alters the exit status of the command the user actually +ran. Everything here is best-effort: the gate is a handful of local file reads, +and the tail swallows every exception so a broken automatic pull can never break +or change the outcome of the user's command. + +The split mirrors the existing background update notice: a cheap synchronous +gate decides whether to register the tail at all, and the tail does the network +work only when the gate passed. +""" + +from __future__ import annotations + +import contextlib +import errno +import os +import sys +import time +from collections.abc import Mapping, Sequence +from datetime import UTC, datetime +from pathlib import Path + +from goodeye_cli import sync +from goodeye_cli import update as update_checks +from goodeye_cli.client import GoodeyeClient +from goodeye_cli.config import ConfigPaths, get_api_key, get_config_paths, get_server + +# How long a lock file may sit before it is treated as abandoned and stolen. A +# crashed process that never released its lock must not wedge automatic pull +# forever; a real pull finishes well inside this bound. +LOCK_STALE_SECONDS = 15 * 60 + + +def should_run_auto_pull( + args: Sequence[str], + env: Mapping[str, str], + config: sync.SyncConfig, + state: sync.SyncState, + *, + authenticated: bool, + now: datetime, +) -> bool: + """Decide whether to register the automatic-pull tail for this invocation. + + All of the following must hold (each is a local read or a comparison, so the + common case is cheap): + + - the invocation is not suppressed (not CI, ``--json``, ``--help``, + ``--version``, the bare/``help``/``update`` forms, or an explicit + ``workflows sync ...`` command), + - credentials exist (the caller is authenticated), + - automatic pull is enabled and at least one target is configured, + - the throttle interval has elapsed since the last automatic pull. + """ + if update_checks.should_suppress_auto_pull(args, env): + return False + if not authenticated: + return False + if not config.auto.enabled: + return False + if not config.targets: + return False + return sync.auto_is_due(state, config, now) + + +def _acquire_lock(lock_file: Path, *, now: float) -> bool: + """Try to take the exclusive automatic-pull lock, stealing a stale one. + + Creates ``lock_file`` with ``O_CREAT | O_EXCL`` so only one process holds it + at a time. A lock older than ``LOCK_STALE_SECONDS`` is treated as abandoned + (the holder crashed without releasing) and stolen. Returns ``True`` when the + lock is held by this process, ``False`` when another live process holds it. + """ + lock_file.parent.mkdir(parents=True, exist_ok=True) + try: + fd = os.open(lock_file, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + except OSError as exc: + if exc.errno != errno.EEXIST: + raise + # Lock exists. Steal it if it is stale, then try once more. + try: + age = now - lock_file.stat().st_mtime + except OSError: + # Vanished between the failed create and the stat; race lost, skip. + return False + if age < LOCK_STALE_SECONDS: + return False + try: + lock_file.unlink() + except OSError: + return False + try: + fd = os.open(lock_file, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + except OSError: + # Another process stole it first; let it run. + return False + os.close(fd) + return True + + +def _release_lock(lock_file: Path) -> None: + """Remove the automatic-pull lock, ignoring an already-gone file.""" + with contextlib.suppress(OSError): + lock_file.unlink() + + +def format_auto_pull_summary(result: sync.PullResult) -> str | None: + """Build the one-line stderr summary for an automatic pull. + + Returns ``None`` when nothing was written and nothing was skipped (the + silent case). Otherwise summarizes what changed and always surfaces skipped + local edits with the exact follow-up command, so an opted-in user still + learns they have local work to reconcile. + """ + pulled = sum(1 for i in result.items if i.action == "pulled") + incomplete = sum(1 for i in result.items if i.action == "pulled-incomplete") + skipped = sum(1 for i in result.items if i.action in ("skipped-modified", "skipped-conflict")) + gone = sum(1 for i in result.items if i.action == "deleted-on-server") + up_to_date = sum(1 for i in result.items if i.action == "up-to-date") + + written = pulled + incomplete + if written == 0 and skipped == 0 and gone == 0: + return None + + parts: list[str] = [] + if written: + parts.append(f"{written} updated") + if up_to_date: + parts.append(f"{up_to_date} up to date") + summary = "auto-synced: " + (", ".join(parts) if parts else "no changes") + + follow_up = "" + if skipped: + summary += f"; {skipped} skipped (local edits)" + follow_up = " Next: goodeye workflows sync status" + if gone: + summary += f"; {gone} gone from the registry" + follow_up = " Next: goodeye workflows sync status" + return summary + follow_up + + +def run_auto_pull_tail( + paths: ConfigPaths | None = None, + *, + server: str | None = None, + api_key: str | None = None, + now: datetime | None = None, + transport: object | None = None, +) -> None: + """Run the automatic pull as a best-effort tail. + + Acquires the exclusive lock, claims the throttle window immediately (so a + transient failure waits out the next interval rather than retrying on every + command), builds the authenticated client, runs the safe-set pull in auto + mode, and prints a one-line stderr summary only when something changed or was + skipped. Every exception is swallowed: this can never break or change the + exit status of the user's command. + """ + try: + p = paths or get_config_paths() + moment = now or datetime.now(UTC) + + if not _acquire_lock(p.sync_lock_file, now=time.time()): + return + try: + config = sync.load_sync_config(p) + state = sync.load_sync_state(p) + + # Claim the throttle window before any network work. This closes the + # residual race where two processes both pass the gate before either + # stamps, and means a transient failure simply waits out the next + # interval rather than retrying on every subsequent command. + sync.stamp_auto_pull(state, moment) + sync.save_sync_state(state, p) + # Reload so the pull's own index save preserves the stamp we just + # wrote (the pull mutates and re-saves this state object). + state = sync.load_sync_state(p) + + resolved_server = server if server is not None else get_server(p) + resolved_key = api_key if api_key is not None else get_api_key(p) + client_kwargs: dict[str, object] = {"api_key": resolved_key} + if transport is not None: + client_kwargs["transport"] = transport + with GoodeyeClient(resolved_server, **client_kwargs) as client: # type: ignore[arg-type] + result = sync.pull( + client, + config, + state, + slugs=[], + target_path=None, + force=False, + yes=False, + auto=True, + paths=p, + ) + summary = format_auto_pull_summary(result) + if summary is not None: + print(summary, file=sys.stderr) + finally: + _release_lock(p.sync_lock_file) + except Exception: + # Best-effort: a failed automatic pull must never surface a traceback or + # change the exit status of the user's command. + return diff --git a/src/goodeye_cli/commands/workflows_sync.py b/src/goodeye_cli/commands/workflows_sync.py index 3683734..409956a 100644 --- a/src/goodeye_cli/commands/workflows_sync.py +++ b/src/goodeye_cli/commands/workflows_sync.py @@ -341,6 +341,128 @@ def target_remove( console.print(f"[yellow]No sync target[/yellow] found for {stored_path}") +auto_app = typer.Typer( + help="Turn automatic background pulls on or off, or show the current setting.", + invoke_without_command=True, +) +app.add_typer(auto_app, name="auto") + + +def _auto_status_payload(config: sync.SyncConfig, state: sync.SyncState) -> dict[str, object]: + """Build the reportable view of the automatic-pull setting and last run.""" + last = state.last_auto_pull_at + return { + "enabled": config.auto.enabled, + "interval_seconds": config.auto.interval_seconds, + "last_auto_pull_at": last.isoformat() if last is not None else None, + } + + +@auto_app.callback(invoke_without_command=True) +def _auto_root( + ctx: typer.Context, + json_output: bool = typer.Option(False, "--json", help="Print the setting as JSON."), + table_output: bool = typer.Option(False, "--table", help="Print the setting as a table."), +) -> None: + """Show whether automatic background pulls are on, with the interval and last run. + + Automatic pull is off by default. When on, the CLI refreshes the safe set of + your configured targets (new and behind-registry workflows) in the + background after a command finishes, no more often than the interval. It + never overwrites local edits, never deletes a local copy, and never blocks + your command. Run with `on` or `off` to change the setting. + """ + if ctx.invoked_subcommand is not None: + return + + mode = resolve_output_mode(json_output=json_output, table_output=table_output) + paths = get_config_paths() + config = sync.load_sync_config(paths) + state = sync.load_sync_state(paths) + payload = _auto_status_payload(config, state) + + if mode == "json": + echo_json(payload) + return + + console = Console() + status_word = "on" if config.auto.enabled else "off" + color = "green" if config.auto.enabled else "yellow" + console.print(f"Automatic pull is [{color}]{status_word}[/{color}].") + console.print(f"Interval: {config.auto.interval_seconds} seconds.") + last = payload["last_auto_pull_at"] + console.print(f"Last automatic pull: {last if last is not None else 'never'}.") + + +@auto_app.command("on") +def auto_on( + interval: int | None = typer.Option( + None, + "--interval", + help="Minimum seconds between automatic pulls (defaults to the current setting).", + ), + json_output: bool = typer.Option(False, "--json", help="Print the setting as JSON."), + table_output: bool = typer.Option(False, "--table", help="Print the setting as a table."), +) -> None: + """Turn automatic background pulls on, optionally setting the interval. + + Once on, the CLI keeps the safe set of your configured targets fresh in the + background. Local edits are always preserved and nothing is ever deleted + automatically. Requires at least one configured sync target to do anything. + """ + if interval is not None and interval <= 0: + raise ValidationFailed( + slug="validation_error", + message="--interval must be a positive number of seconds.", + ) + mode = resolve_output_mode(json_output=json_output, table_output=table_output) + paths = get_config_paths() + config = sync.load_sync_config(paths) + config.auto.enabled = True + if interval is not None: + config.auto.interval_seconds = interval + sync.save_sync_config(config, paths) + state = sync.load_sync_state(paths) + + if mode == "json": + echo_json(_auto_status_payload(config, state)) + return + + console = Console() + console.print( + f"[green]Automatic pull is on[/green] (interval: {config.auto.interval_seconds} seconds)." + ) + if not config.targets: + console.print( + "[yellow]Next:[/yellow] add a target with " + "`goodeye workflows sync target add ` so there is something to keep fresh." + ) + + +@auto_app.command("off") +def auto_off( + json_output: bool = typer.Option(False, "--json", help="Print the setting as JSON."), + table_output: bool = typer.Option(False, "--table", help="Print the setting as a table."), +) -> None: + """Turn automatic background pulls off. + + The interval setting is kept so turning it back on resumes the same cadence. + """ + mode = resolve_output_mode(json_output=json_output, table_output=table_output) + paths = get_config_paths() + config = sync.load_sync_config(paths) + config.auto.enabled = False + sync.save_sync_config(config, paths) + state = sync.load_sync_state(paths) + + if mode == "json": + echo_json(_auto_status_payload(config, state)) + return + + console = Console() + console.print("[yellow]Automatic pull is off.[/yellow]") + + _SKIPPED_ACTIONS = frozenset({"skipped-modified", "skipped-conflict"}) @@ -641,6 +763,9 @@ def push( __all__ = [ "app", + "auto_app", + "auto_off", + "auto_on", "pull", "push", "status", diff --git a/src/goodeye_cli/config.py b/src/goodeye_cli/config.py index b3cc482..d11f01e 100644 --- a/src/goodeye_cli/config.py +++ b/src/goodeye_cli/config.py @@ -56,6 +56,7 @@ class ConfigPaths: update_check_file: Path sync_file: Path sync_state_file: Path + sync_lock_file: Path def get_config_paths(env: dict[str, str] | None = None) -> ConfigPaths: @@ -75,6 +76,7 @@ def get_config_paths(env: dict[str, str] | None = None) -> ConfigPaths: update_check_file=config_dir / "update-check.json", sync_file=config_dir / "sync.json", sync_state_file=config_dir / "sync-state.json", + sync_lock_file=config_dir / "sync.lock", ) diff --git a/src/goodeye_cli/sync.py b/src/goodeye_cli/sync.py index d8bd79d..ab94f14 100644 --- a/src/goodeye_cli/sync.py +++ b/src/goodeye_cli/sync.py @@ -21,6 +21,7 @@ import os import re from dataclasses import dataclass +from datetime import datetime from pathlib import Path from typing import TYPE_CHECKING, Any, Literal @@ -92,6 +93,20 @@ class SyncTarget(_SyncBase): link: bool = False +class AutoConfig(_SyncBase): + """Opt-in automatic-pull settings for the local mirror. + + ``enabled`` is off by default, so a config written by an older CLI (which + has no ``auto`` block at all) loads with automatic pulls disabled and sees + no behavior change until the user opts in. ``interval_seconds`` is the + minimum gap between automatic pulls: a floor on how often the background + tail refreshes the mirror, not a freshness guarantee at any instant. + """ + + enabled: bool = False + interval_seconds: int = 3600 + + class SyncConfig(_SyncBase): """The full local sync configuration persisted to ``sync.json``.""" @@ -103,6 +118,9 @@ class SyncConfig(_SyncBase): # if a config-level binding is added later. identity: str | None = None targets: list[SyncTarget] = Field(default_factory=list) + # Defaulted so a config written before automatic pull existed (no ``auto`` + # key) loads with the feature off. + auto: AutoConfig = Field(default_factory=AutoConfig) def expand_target_path(path: str) -> Path: @@ -453,6 +471,34 @@ class SyncState(_SyncBase): version: int = 1 identity: str | None = None entries: list[SyncEntry] = Field(default_factory=list) + # When the last automatic pull claimed its throttle window. ``None`` until + # the first automatic pull runs; absent in indexes written by an older CLI, + # which load it as ``None``. + last_auto_pull_at: datetime | None = None + + +def auto_is_due(state: SyncState, config: SyncConfig, now: datetime) -> bool: + """Return whether enough time has elapsed for another automatic pull. + + The interval is a floor between automatic pulls. A never-run mirror + (``last_auto_pull_at is None``) is always due. Otherwise the gap since the + last claim must reach ``config.auto.interval_seconds``. Naive timestamps are + treated as the same wall clock as ``now`` so a comparison never raises on a + legacy index that stored a tz-naive value. + """ + last = state.last_auto_pull_at + if last is None: + return True + if last.tzinfo is None and now.tzinfo is not None: + last = last.replace(tzinfo=now.tzinfo) + elif last.tzinfo is not None and now.tzinfo is None: + now = now.replace(tzinfo=last.tzinfo) + return (now - last).total_seconds() >= config.auto.interval_seconds + + +def stamp_auto_pull(state: SyncState, now: datetime) -> None: + """Record ``now`` as the moment the latest automatic pull claimed its window.""" + state.last_auto_pull_at = now def load_sync_state(paths: ConfigPaths) -> SyncState: @@ -900,6 +946,7 @@ def pull( force: bool, yes: bool, paths: ConfigPaths, + auto: bool = False, ) -> PullResult: """Mirror registry workflows onto disk for the configured targets. @@ -918,6 +965,13 @@ def pull( sibling fetch interrupted) records no entry, so a re-run reports its ``SKILL.md`` as untracked and preserves it (it is never clobbered without ``force``); the resume guarantee covers completed workflows only. + + ``auto`` runs the same ``force=False`` pull in a report-only deletion mode: + a workflow gone server-side is still classified and reported as + ``deleted-on-server`` but its local directory is never removed and no + confirmation prompt is ever issued. This is the mode the background + automatic pull uses, where prompting is impossible and deletion is reserved + for an explicit manual pull. """ result = PullResult() # Guard before any work: a mismatched identity aborts here, and a first run @@ -936,7 +990,9 @@ def pull( for summary in selected: result.items.append(_pull_one(client, state, target, summary, force=force)) result.items.extend( - _reconcile_deletions(state, target, live, slug_args=slug_args, force=force, yes=yes) + _reconcile_deletions( + state, target, live, slug_args=slug_args, force=force, yes=yes, auto=auto + ) ) finally: # Persist whatever the index accumulated, including on a mid-loop raise. @@ -957,6 +1013,7 @@ def _reconcile_deletions( slug_args: list[str], force: bool, yes: bool, + auto: bool = False, ) -> list[PullItem]: """Remove local copies of tracked workflows that are gone server-side. @@ -974,6 +1031,13 @@ def _reconcile_deletions( guard, ``confirm_destructive`` auto-approves on a non-TTY (the agent path), so an agent run would otherwise delete a vanished workflow's directory along with edits that were never pushed. + + ``auto`` is the report-only mode used by the background automatic pull: a + gone-server-side entry is always reported ``deleted-on-server`` and kept on + disk. The destructive branch (``confirm_destructive`` plus + ``_remove_skill_dir``) is never reached, so the automatic path can never + delete a local directory and never prompts, even on a non-TTY where the + confirm would otherwise auto-approve. """ stored_target = normalize_target_path(target.path) wanted = set(slug_args) @@ -1001,6 +1065,22 @@ def _reconcile_deletions( surviving.append(entry) continue + # Report-only automatic path: a workflow gone server-side is surfaced as + # `deleted-on-server` and kept on disk. The automatic pull never deletes + # and never prompts, so it short-circuits here before the edit guard and + # the confirm. Deletion stays an explicit, confirmed manual pull. + if auto: + surviving.append(entry) + items.append( + PullItem( + slug=entry.slug, + target_path=stored_target, + action="deleted-on-server", + workflow_id=entry.workflow_id, + ) + ) + continue + # Protect un-pushed local edits. If the on-disk body or any tracked # sibling diverged from the recorded hash, removal would discard work the # registry never received, and the deleted workflow cannot be re-pulled to diff --git a/src/goodeye_cli/update.py b/src/goodeye_cli/update.py index 9b3e94e..1ee23ba 100644 --- a/src/goodeye_cli/update.py +++ b/src/goodeye_cli/update.py @@ -53,6 +53,20 @@ def should_suppress_background_notice(args: Sequence[str], env: Mapping[str, str return first_arg in {"--version", "help", "update", "upgrade"} +def should_suppress_auto_pull(args: Sequence[str], env: Mapping[str, str]) -> bool: + """Return whether a CLI invocation should skip the best-effort automatic pull. + + Suppressed for the same machine-readable and meta invocations the background + update notice skips (CI, ``--json``, ``--help``/``-h``, ``--version``, and + the bare/``help``/``update`` forms), plus any explicit ``workflows sync ...`` + command: an automatic pull must never shadow a sync the user is running by + hand (pull, push, status, or a target edit). + """ + if should_suppress_background_notice(args, env): + return True + return len(args) >= 2 and args[0] == "workflows" and args[1] == "sync" + + def format_update_notice(result: UpdateCheckResult) -> str: """Format the concise stderr notice for an available update.""" return f"goodeye {result.latest_version} is available; run: goodeye update" diff --git a/tests/conftest.py b/tests/conftest.py index 35050f1..b344104 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,6 +21,7 @@ def tmp_config_paths(tmp_path: Path) -> ConfigPaths: update_check_file=config_dir / "update-check.json", sync_file=config_dir / "sync.json", sync_state_file=config_dir / "sync-state.json", + sync_lock_file=config_dir / "sync.lock", ) diff --git a/tests/test_background.py b/tests/test_background.py new file mode 100644 index 0000000..3f529c6 --- /dev/null +++ b/tests/test_background.py @@ -0,0 +1,378 @@ +"""Tests for the best-effort automatic-pull tail and its synchronous gate.""" + +from __future__ import annotations + +import time +from datetime import UTC, datetime, timedelta + +import httpx +import pytest +import respx +from typer.testing import CliRunner + +import goodeye_cli.app as app_module +from goodeye_cli import background as background_sync +from goodeye_cli import sync +from goodeye_cli.app import app +from goodeye_cli.config import ConfigPaths + +SERVER = "https://example.test" +DEFAULT_EMAIL = "owner@example.com" + + +def _enabled_config() -> sync.SyncConfig: + return sync.SyncConfig( + targets=[sync.SyncTarget(path="~/skills", scope="owned")], + auto=sync.AutoConfig(enabled=True, interval_seconds=3600), + ) + + +# ----- synchronous gate ----- + + +def test_gate_registers_for_eligible_invocation() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + assert ( + background_sync.should_run_auto_pull( + ["logout"], + {}, + _enabled_config(), + sync.SyncState(), + authenticated=True, + now=now, + ) + is True + ) + + +@pytest.mark.parametrize( + "args,env", + [ + (["whoami", "--json"], {}), + (["--version"], {}), + (["logout"], {"CI": "1"}), + (["workflows", "sync", "pull"], {}), + ], +) +def test_gate_skips_suppressed_invocations(args: list[str], env: dict[str, str]) -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + assert ( + background_sync.should_run_auto_pull( + args, env, _enabled_config(), sync.SyncState(), authenticated=True, now=now + ) + is False + ) + + +def test_gate_skips_when_unauthenticated() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + assert ( + background_sync.should_run_auto_pull( + ["logout"], {}, _enabled_config(), sync.SyncState(), authenticated=False, now=now + ) + is False + ) + + +def test_gate_skips_when_auto_disabled() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + config = sync.SyncConfig(targets=[sync.SyncTarget(path="~/skills", scope="owned")]) + assert config.auto.enabled is False + assert ( + background_sync.should_run_auto_pull( + ["logout"], {}, config, sync.SyncState(), authenticated=True, now=now + ) + is False + ) + + +def test_gate_skips_when_no_targets() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + config = sync.SyncConfig(auto=sync.AutoConfig(enabled=True)) + assert ( + background_sync.should_run_auto_pull( + ["logout"], {}, config, sync.SyncState(), authenticated=True, now=now + ) + is False + ) + + +def test_gate_skips_when_not_yet_due() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + state = sync.SyncState(last_auto_pull_at=now - timedelta(seconds=10)) + assert ( + background_sync.should_run_auto_pull( + ["logout"], {}, _enabled_config(), state, authenticated=True, now=now + ) + is False + ) + + +# ----- lock ----- + + +def test_acquire_lock_succeeds_then_blocks_second(tmp_config_paths: ConfigPaths) -> None: + lock = tmp_config_paths.sync_lock_file + assert background_sync._acquire_lock(lock, now=time.time()) is True + # A second acquire while the lock is held returns False. + assert background_sync._acquire_lock(lock, now=time.time()) is False + background_sync._release_lock(lock) + # Released: acquirable again. + assert background_sync._acquire_lock(lock, now=time.time()) is True + background_sync._release_lock(lock) + + +def test_acquire_lock_steals_stale_lock(tmp_config_paths: ConfigPaths) -> None: + lock = tmp_config_paths.sync_lock_file + lock.parent.mkdir(parents=True, exist_ok=True) + lock.write_text("", encoding="utf-8") + # Pretend the lock is far older than the staleness bound. + far_future = time.time() + background_sync.LOCK_STALE_SECONDS + 60 + assert background_sync._acquire_lock(lock, now=far_future) is True + background_sync._release_lock(lock) + + +# ----- summary ----- + + +def _item(action: sync.PullAction, slug: str = "wf") -> sync.PullItem: + return sync.PullItem(slug=slug, target_path="~/skills", action=action) + + +def test_summary_none_when_nothing_changed() -> None: + result = sync.PullResult(items=[_item("up-to-date"), _item("up-to-date", "b")]) + assert background_sync.format_auto_pull_summary(result) is None + + +def test_summary_reports_updates() -> None: + result = sync.PullResult( + items=[_item("pulled"), _item("pulled", "b"), _item("up-to-date", "c")] + ) + summary = background_sync.format_auto_pull_summary(result) + assert summary is not None + assert "auto-synced" in summary + assert "2 updated" in summary + + +def test_summary_surfaces_skipped_with_follow_up() -> None: + result = sync.PullResult(items=[_item("pulled"), _item("skipped-modified", "b")]) + summary = background_sync.format_auto_pull_summary(result) + assert summary is not None + assert "1 skipped (local edits)" in summary + assert "goodeye workflows sync status" in summary + + +def test_summary_reports_gone_alongside_skipped() -> None: + # Gone workflows must always be surfaced, even when skipped local edits also + # exist; the gone count is never dropped because a skip is present. + result = sync.PullResult(items=[_item("skipped-modified"), _item("deleted-on-server", "b")]) + summary = background_sync.format_auto_pull_summary(result) + assert summary is not None + assert "1 skipped (local edits)" in summary + assert "1 gone from the registry" in summary + assert "goodeye workflows sync status" in summary + + +# ----- tail ----- + + +def _me_route(email: str = DEFAULT_EMAIL) -> respx.Route: + return respx.get(f"{SERVER}/v1/me").mock( + return_value=httpx.Response(200, json={"email": email}) + ) + + +def _list_response(items: list[dict], next_cursor: str | None = None) -> httpx.Response: + return httpx.Response(200, json={"items": items, "next_cursor": next_cursor}) + + +@respx.mock +def test_tail_claims_window_and_pulls(tmp_config_paths: ConfigPaths, tmp_path, capsys) -> None: + _me_route() + target_dir = tmp_path / "skills" + config = sync.SyncConfig( + targets=[sync.SyncTarget(path=str(target_dir), scope="owned")], + auto=sync.AutoConfig(enabled=True, interval_seconds=3600), + ) + sync.save_sync_config(config, tmp_config_paths) + + respx.get(f"{SERVER}/v1/workflows").mock( + return_value=_list_response( + [{"id": "skl_a", "name": "alpha", "current_version": 1, "version_token": "t1"}] + ) + ) + respx.get(f"{SERVER}/v1/workflows/skl_a").mock( + return_value=httpx.Response( + 200, + json={ + "id": "skl_a", + "name": "alpha", + "version": 1, + "body": "body", + "version_token": "t1", + "effective_role": "owner", + "verifiers": [], + }, + ) + ) + + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + background_sync.run_auto_pull_tail( + tmp_config_paths, + server=SERVER, + api_key="good_live_EXAMPLE", + now=now, + ) + + # The window was claimed (stamped) and the workflow landed on disk. + assert sync.load_sync_state(tmp_config_paths).last_auto_pull_at == now + assert (target_dir / "alpha" / "SKILL.md").read_text(encoding="utf-8") == "body" + err = capsys.readouterr().err + assert "auto-synced" in err + # The lock was released. + assert not tmp_config_paths.sync_lock_file.exists() + + +def test_tail_swallows_failure_and_claims_window(tmp_config_paths: ConfigPaths, capsys) -> None: + # No HTTP routes registered and a real (unreachable) server: the network call + # fails, but the tail must not raise and the window is already claimed so it + # waits out the next interval instead of retrying every command. + config = sync.SyncConfig( + targets=[sync.SyncTarget(path="~/skills", scope="owned")], + auto=sync.AutoConfig(enabled=True), + ) + sync.save_sync_config(config, tmp_config_paths) + + def fail(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("boom", request=request) + + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + # Must not raise. + background_sync.run_auto_pull_tail( + tmp_config_paths, + server=SERVER, + api_key="good_live_EXAMPLE", + now=now, + transport=httpx.MockTransport(fail), + ) + assert sync.load_sync_state(tmp_config_paths).last_auto_pull_at == now + assert not tmp_config_paths.sync_lock_file.exists() + + +def test_tail_skips_when_lock_held(tmp_config_paths: ConfigPaths) -> None: + # When another process holds the lock, the tail does no work: it does not + # even claim the window. + config = sync.SyncConfig( + targets=[sync.SyncTarget(path="~/skills", scope="owned")], + auto=sync.AutoConfig(enabled=True), + ) + sync.save_sync_config(config, tmp_config_paths) + tmp_config_paths.sync_lock_file.parent.mkdir(parents=True, exist_ok=True) + tmp_config_paths.sync_lock_file.write_text("", encoding="utf-8") + + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + background_sync.run_auto_pull_tail( + tmp_config_paths, + server=SERVER, + api_key="good_live_EXAMPLE", + now=now, + ) + # The window was never claimed because the lock was held. + assert sync.load_sync_state(tmp_config_paths).last_auto_pull_at is None + # The pre-existing (fresh) lock is untouched. + assert tmp_config_paths.sync_lock_file.exists() + + +# ----- app-entry wiring ----- + + +def _set_invocation_args(monkeypatch: pytest.MonkeyPatch, args: list[str]) -> None: + monkeypatch.setattr(app_module, "_get_background_notice_args", lambda: args, raising=False) + + +def _silence_update_notice(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + app_module.update_checks, "check_for_update_background", lambda *a, **k: None + ) + + +def _setup_env(monkeypatch: pytest.MonkeyPatch, tmp_config_paths: ConfigPaths) -> None: + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_config_paths.config_dir.parent)) + monkeypatch.delenv("CI", raising=False) + monkeypatch.setenv("GOODEYE_API_KEY", "good_live_EXAMPLE") + monkeypatch.setenv("GOODEYE_SERVER", SERVER) + + +def test_entry_registers_tail_for_eligible_invocation( + tmp_config_paths: ConfigPaths, monkeypatch: pytest.MonkeyPatch +) -> None: + _setup_env(monkeypatch, tmp_config_paths) + _silence_update_notice(monkeypatch) + _set_invocation_args(monkeypatch, ["logout"]) + sync.save_sync_config(_enabled_config(), tmp_config_paths) + + registered: list[object] = [] + monkeypatch.setattr(app_module.atexit, "register", registered.append) + + result = CliRunner().invoke(app, ["logout"]) + assert result.exit_code == 0, result.output + assert registered == [background_sync.run_auto_pull_tail] + + +def test_entry_does_not_register_when_disabled( + tmp_config_paths: ConfigPaths, monkeypatch: pytest.MonkeyPatch +) -> None: + _setup_env(monkeypatch, tmp_config_paths) + _silence_update_notice(monkeypatch) + _set_invocation_args(monkeypatch, ["logout"]) + # No sync.json at all: auto disabled by default. + + registered: list[object] = [] + monkeypatch.setattr(app_module.atexit, "register", registered.append) + + result = CliRunner().invoke(app, ["logout"]) + assert result.exit_code == 0, result.output + assert registered == [] + + +@pytest.mark.parametrize( + "args", + [ + ["--version"], + ["whoami", "--json"], + ["workflows", "sync", "pull"], + ], +) +def test_entry_does_not_register_for_suppressed_invocations( + tmp_config_paths: ConfigPaths, + monkeypatch: pytest.MonkeyPatch, + args: list[str], +) -> None: + _setup_env(monkeypatch, tmp_config_paths) + _silence_update_notice(monkeypatch) + _set_invocation_args(monkeypatch, args) + sync.save_sync_config(_enabled_config(), tmp_config_paths) + + registered: list[object] = [] + monkeypatch.setattr(app_module.atexit, "register", registered.append) + + CliRunner().invoke(app, args) + assert registered == [] + + +def test_entry_register_gate_never_raises_into_command( + tmp_config_paths: ConfigPaths, monkeypatch: pytest.MonkeyPatch +) -> None: + _setup_env(monkeypatch, tmp_config_paths) + _silence_update_notice(monkeypatch) + _set_invocation_args(monkeypatch, ["logout"]) + sync.save_sync_config(_enabled_config(), tmp_config_paths) + + def boom(*_a: object, **_k: object) -> bool: + raise RuntimeError("gate exploded") + + monkeypatch.setattr(background_sync, "should_run_auto_pull", boom) + + result = CliRunner().invoke(app, ["logout"]) + # The broken gate is swallowed; the command exits cleanly. + assert result.exit_code == 0, result.output diff --git a/tests/test_commands_workflows_sync.py b/tests/test_commands_workflows_sync.py index a111ba2..41346f9 100644 --- a/tests/test_commands_workflows_sync.py +++ b/tests/test_commands_workflows_sync.py @@ -1523,3 +1523,102 @@ def test_target_add_only_with_preset_appends_to_existing_preset_target( selected = items[0]["selected"] assert "existing-wf" in selected assert "new-wf" in selected + + +# ----- workflows sync auto ----- + + +def _load_auto(tmp_config_paths: ConfigPaths): + from goodeye_cli import sync + + return sync.load_sync_config(tmp_config_paths).auto + + +def test_auto_on_enables_and_defaults_to_compact_json( + tmp_config_paths: ConfigPaths, monkeypatch +) -> None: + _redirect_config(monkeypatch, tmp_config_paths) + runner = CliRunner() + result = runner.invoke(app, ["workflows", "sync", "auto", "on"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["enabled"] is True + assert payload["interval_seconds"] == 3600 + assert payload["last_auto_pull_at"] is None + # The config on disk reflects the change. + assert _load_auto(tmp_config_paths).enabled is True + + +def test_auto_on_with_interval_sets_interval(tmp_config_paths: ConfigPaths, monkeypatch) -> None: + _redirect_config(monkeypatch, tmp_config_paths) + runner = CliRunner() + result = runner.invoke(app, ["workflows", "sync", "auto", "on", "--interval", "120"]) + assert result.exit_code == 0, result.output + assert json.loads(result.output)["interval_seconds"] == 120 + assert _load_auto(tmp_config_paths).interval_seconds == 120 + + +def test_auto_on_rejects_non_positive_interval(tmp_config_paths: ConfigPaths, monkeypatch) -> None: + _redirect_config(monkeypatch, tmp_config_paths) + runner = CliRunner() + result = runner.invoke(app, ["workflows", "sync", "auto", "on", "--interval", "0"]) + assert result.exit_code != 0 + # The setting was not flipped on by a rejected request. + assert _load_auto(tmp_config_paths).enabled is False + + +def test_auto_off_disables_but_keeps_interval(tmp_config_paths: ConfigPaths, monkeypatch) -> None: + _redirect_config(monkeypatch, tmp_config_paths) + runner = CliRunner() + runner.invoke(app, ["workflows", "sync", "auto", "on", "--interval", "900"]) + result = runner.invoke(app, ["workflows", "sync", "auto", "off"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["enabled"] is False + # The interval is remembered for the next `on`. + assert payload["interval_seconds"] == 900 + assert _load_auto(tmp_config_paths).interval_seconds == 900 + + +def test_auto_status_reports_current_setting(tmp_config_paths: ConfigPaths, monkeypatch) -> None: + _redirect_config(monkeypatch, tmp_config_paths) + runner = CliRunner() + runner.invoke(app, ["workflows", "sync", "auto", "on", "--interval", "600"]) + result = runner.invoke(app, ["workflows", "sync", "auto"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload == { + "enabled": True, + "interval_seconds": 600, + "last_auto_pull_at": None, + } + + +def test_auto_status_human_mode_on_tty(tmp_config_paths: ConfigPaths, monkeypatch) -> None: + _redirect_config(monkeypatch, tmp_config_paths) + runner = CliRunner() + runner.invoke(app, ["workflows", "sync", "auto", "on"]) + # --table forces the human-readable view regardless of TTY detection. + result = runner.invoke(app, ["workflows", "sync", "auto", "--table"]) + assert result.exit_code == 0, result.output + assert "Automatic pull is" in result.output + assert "on" in result.output + assert "never" in result.output + + +def test_auto_status_reports_last_pull_time(tmp_config_paths: ConfigPaths, monkeypatch) -> None: + from datetime import UTC, datetime + + from goodeye_cli import sync + + _redirect_config(monkeypatch, tmp_config_paths) + config = sync.SyncConfig(auto=sync.AutoConfig(enabled=True, interval_seconds=300)) + sync.save_sync_config(config, tmp_config_paths) + state = sync.SyncState(last_auto_pull_at=datetime(2026, 6, 14, 17, 2, 11, tzinfo=UTC)) + sync.save_sync_state(state, tmp_config_paths) + + runner = CliRunner() + result = runner.invoke(app, ["workflows", "sync", "auto"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload["last_auto_pull_at"] == "2026-06-14T17:02:11+00:00" diff --git a/tests/test_config.py b/tests/test_config.py index 6ec6e36..e0c7cc5 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -34,6 +34,7 @@ def test_get_config_paths_uses_xdg(tmp_path: Path) -> None: assert paths.update_check_file == tmp_path / "goodeye" / "update-check.json" assert paths.sync_file == tmp_path / "goodeye" / "sync.json" assert paths.sync_state_file == tmp_path / "goodeye" / "sync-state.json" + assert paths.sync_lock_file == tmp_path / "goodeye" / "sync.lock" def test_get_config_paths_defaults_to_home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: diff --git a/tests/test_sync.py b/tests/test_sync.py index c054001..657e021 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from datetime import UTC, datetime, timedelta from pathlib import Path from unittest import mock @@ -15,6 +16,7 @@ from goodeye_cli.errors import Conflict, NotFound, ServerError, ValidationFailed from goodeye_cli.sync import ( PRESETS, + AutoConfig, FileState, PushItem, SyncConfig, @@ -24,6 +26,7 @@ SyncVerifierBinding, add_target, append_to_allowlist, + auto_is_due, body_sha256, ensure_identity, expand_target_path, @@ -47,6 +50,7 @@ select_for_target, server_moved, slug_in_target_scope, + stamp_auto_pull, status, untracked_local_slugs, upsert_entry, @@ -3256,3 +3260,282 @@ def test_append_to_allowlist_deduplicates_input_entries() -> None: assert added == ["new"] assert already_present == ["existing"] assert config.targets[0].selected == ["existing", "new"] + + +# ----- auto config + state model ----- + + +def test_auto_config_defaults_off() -> None: + auto = AutoConfig() + assert auto.enabled is False + assert auto.interval_seconds == 3600 + + +def test_sync_config_defaults_auto_block() -> None: + config = SyncConfig() + assert config.auto == AutoConfig() + assert config.auto.enabled is False + + +def test_older_config_without_auto_loads_clean(tmp_config_paths: ConfigPaths) -> None: + # A config written before automatic pull existed has no `auto` key. It must + # load with the feature off rather than failing validation. + tmp_config_paths.sync_file.parent.mkdir(parents=True, exist_ok=True) + tmp_config_paths.sync_file.write_text( + json.dumps({"version": 1, "targets": [{"path": "~/skills", "scope": "owned"}]}), + encoding="utf-8", + ) + config = load_sync_config(tmp_config_paths) + assert config.auto.enabled is False + assert config.auto.interval_seconds == 3600 + assert len(config.targets) == 1 + + +def test_auto_config_roundtrips_through_sync_json(tmp_config_paths: ConfigPaths) -> None: + config = SyncConfig( + targets=[SyncTarget(path="~/skills", scope="owned")], + auto=AutoConfig(enabled=True, interval_seconds=120), + ) + save_sync_config(config, tmp_config_paths) + reloaded = load_sync_config(tmp_config_paths) + assert reloaded.auto.enabled is True + assert reloaded.auto.interval_seconds == 120 + + +def test_state_defaults_last_auto_pull_at_none() -> None: + assert SyncState().last_auto_pull_at is None + + +def test_older_state_without_last_auto_pull_at_loads_clean(tmp_config_paths: ConfigPaths) -> None: + tmp_config_paths.sync_state_file.parent.mkdir(parents=True, exist_ok=True) + tmp_config_paths.sync_state_file.write_text( + json.dumps({"version": 1, "identity": "owner@example.com", "entries": []}), + encoding="utf-8", + ) + state = load_sync_state(tmp_config_paths) + assert state.last_auto_pull_at is None + assert state.identity == "owner@example.com" + + +def test_stamp_auto_pull_roundtrips_through_state(tmp_config_paths: ConfigPaths) -> None: + now = datetime(2026, 6, 14, 17, 2, 11, tzinfo=UTC) + state = SyncState() + stamp_auto_pull(state, now) + assert state.last_auto_pull_at == now + save_sync_state(state, tmp_config_paths) + reloaded = load_sync_state(tmp_config_paths) + assert reloaded.last_auto_pull_at == now + + +# ----- auto_is_due boundary behavior ----- + + +def test_auto_is_due_when_never_run() -> None: + config = SyncConfig(auto=AutoConfig(enabled=True, interval_seconds=3600)) + state = SyncState() + assert auto_is_due(state, config, datetime(2026, 6, 14, 12, 0, tzinfo=UTC)) is True + + +def test_auto_is_due_false_before_interval() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + config = SyncConfig(auto=AutoConfig(enabled=True, interval_seconds=3600)) + state = SyncState(last_auto_pull_at=now - timedelta(seconds=3599)) + assert auto_is_due(state, config, now) is False + + +def test_auto_is_due_true_at_exact_interval() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + config = SyncConfig(auto=AutoConfig(enabled=True, interval_seconds=3600)) + state = SyncState(last_auto_pull_at=now - timedelta(seconds=3600)) + assert auto_is_due(state, config, now) is True + + +def test_auto_is_due_true_after_interval() -> None: + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + config = SyncConfig(auto=AutoConfig(enabled=True, interval_seconds=3600)) + state = SyncState(last_auto_pull_at=now - timedelta(seconds=7200)) + assert auto_is_due(state, config, now) is True + + +def test_auto_is_due_tolerates_naive_last_timestamp() -> None: + # A legacy index could store a tz-naive timestamp; the comparison must not raise. + now = datetime(2026, 6, 14, 12, 0, tzinfo=UTC) + config = SyncConfig(auto=AutoConfig(enabled=True, interval_seconds=3600)) + state = SyncState(last_auto_pull_at=datetime(2026, 6, 14, 10, 0)) + assert auto_is_due(state, config, now) is True + + +# ----- auto-mode pull engine ----- + + +@respx.mock +def test_pull_auto_writes_safe_set(tmp_path: Path, tmp_config_paths: ConfigPaths) -> None: + # Auto mode writes new and behind-server workflows exactly like a force=False + # manual pull does. + _me_route() + target_dir = tmp_path / "skills" + config = SyncConfig(targets=[SyncTarget(path=str(target_dir), scope="owned")]) + respx.get(f"{SERVER}/v1/workflows").mock( + return_value=_list_response( + [{"id": "skl_new", "name": "fresh", "current_version": 1, "version_token": "t1"}] + ) + ) + respx.get(f"{SERVER}/v1/workflows/skl_new").mock( + return_value=_detail_response(id_="skl_new", name="fresh", body="body", token="t1") + ) + with GoodeyeClient(SERVER, api_key="good_live_EXAMPLE") as client: + result = pull( + client, + config, + SyncState(), + slugs=[], + target_path=None, + force=False, + yes=False, + auto=True, + paths=tmp_config_paths, + ) + assert [(i.slug, i.action) for i in result.items] == [("fresh", "pulled")] + assert (target_dir / "fresh" / "SKILL.md").read_text(encoding="utf-8") == "body" + + +@respx.mock +def test_pull_auto_skips_modified_local(tmp_path: Path, tmp_config_paths: ConfigPaths) -> None: + # Auto never passes force, so a locally edited workflow is preserved and + # reported skipped, never clobbered. + _me_route() + target_dir = tmp_path / "skills" + config = SyncConfig(targets=[SyncTarget(path=str(target_dir), scope="owned")]) + _write_skill(target_dir, "alpha", "locally edited") + state = SyncState( + entries=[ + _tracked_entry( + target_dir, id_="skl_a", slug="alpha", body="server original", token="t1" + ) + ] + ) + respx.get(f"{SERVER}/v1/workflows").mock( + return_value=_list_response( + [{"id": "skl_a", "name": "alpha", "current_version": 1, "version_token": "t1"}] + ) + ) + detail_route = respx.get(f"{SERVER}/v1/workflows/skl_a") + with GoodeyeClient(SERVER, api_key="good_live_EXAMPLE") as client: + result = pull( + client, + config, + state, + slugs=[], + target_path=None, + force=False, + yes=False, + auto=True, + paths=tmp_config_paths, + ) + assert [(i.slug, i.action) for i in result.items] == [("alpha", "skipped-modified")] + assert (target_dir / "alpha" / "SKILL.md").read_text(encoding="utf-8") == "locally edited" + assert detail_route.call_count == 0 + + +@respx.mock +def test_pull_auto_skips_conflict(tmp_path: Path, tmp_config_paths: ConfigPaths) -> None: + _me_route() + target_dir = tmp_path / "skills" + config = SyncConfig(targets=[SyncTarget(path=str(target_dir), scope="owned")]) + _write_skill(target_dir, "alpha", "locally edited") + state = SyncState( + entries=[ + _tracked_entry( + target_dir, id_="skl_a", slug="alpha", body="server original", token="t1" + ) + ] + ) + # Both sides moved: local edited AND server token advanced -> conflict. + respx.get(f"{SERVER}/v1/workflows").mock( + return_value=_list_response( + [{"id": "skl_a", "name": "alpha", "current_version": 2, "version_token": "t2"}] + ) + ) + with GoodeyeClient(SERVER, api_key="good_live_EXAMPLE") as client: + result = pull( + client, + config, + state, + slugs=[], + target_path=None, + force=False, + yes=False, + auto=True, + paths=tmp_config_paths, + ) + assert [(i.slug, i.action) for i in result.items] == [("alpha", "skipped-conflict")] + assert (target_dir / "alpha" / "SKILL.md").read_text(encoding="utf-8") == "locally edited" + + +@respx.mock +def test_pull_auto_reports_deleted_on_server_but_never_removes( + tmp_path: Path, tmp_config_paths: ConfigPaths, monkeypatch +) -> None: + # In auto mode a workflow gone server-side is reported deleted-on-server, but + # its local directory is kept and confirm_destructive is never called -- even + # with yes=False on a non-TTY where the confirm would otherwise auto-approve. + _me_route() + target_dir = tmp_path / "skills" + target = SyncTarget(path=str(target_dir), scope="owned") + config = SyncConfig(targets=[target]) + body = "registry body" + _write_skill(target_dir, "gone", body) + state = SyncState(entries=[_tracked_entry(target_dir, id_="skl_gone", slug="gone", body=body)]) + respx.get(f"{SERVER}/v1/workflows").mock( + return_value=_list_response( + [_summary_dict(id_="skl_gone", name="gone", archived_at="2026-01-01T00:00:00Z")] + ) + ) + + def _boom(*_a: object, **_k: object) -> bool: + raise AssertionError("confirm_destructive must not be called in auto mode") + + monkeypatch.setattr("goodeye_cli.sync.confirm_destructive", _boom) + + with GoodeyeClient(SERVER, api_key="good_live_EXAMPLE") as client: + result = pull( + client, + config, + state, + slugs=[], + target_path=None, + force=False, + yes=False, + auto=True, + paths=tmp_config_paths, + ) + assert [(i.slug, i.action) for i in result.items] == [("gone", "deleted-on-server")] + # The local copy survives and the entry stays tracked. + assert local_skill_path(target, "gone").read_text(encoding="utf-8") == body + assert [e.slug for e in load_sync_state(tmp_config_paths).entries] == ["gone"] + + +@respx.mock +def test_pull_auto_identity_mismatch_raises_and_is_catchable( + tmp_path: Path, tmp_config_paths: ConfigPaths +) -> None: + # The auto path runs the same identity guard. A mismatch raises Conflict + # before any work; the caller (the tail) is expected to swallow it. + _me_route("intruder@example.com") + target_dir = tmp_path / "skills" + config = SyncConfig(targets=[SyncTarget(path=str(target_dir), scope="owned")]) + list_route = respx.get(f"{SERVER}/v1/workflows").mock(return_value=_list_response([])) + state = SyncState(identity="owner@example.com") + with GoodeyeClient(SERVER, api_key="good_live_EXAMPLE") as client, pytest.raises(Conflict): + pull( + client, + config, + state, + slugs=[], + target_path=None, + force=False, + yes=False, + auto=True, + paths=tmp_config_paths, + ) + assert list_route.call_count == 0 diff --git a/tests/test_update.py b/tests/test_update.py index aeb448d..3b03d13 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -222,6 +222,7 @@ def test_background_check_returns_none_on_cache_write_failure(tmp_path: Path) -> update_check_file=config_dir / "update-check.json", sync_file=config_dir / "sync.json", sync_state_file=config_dir / "sync-state.json", + sync_lock_file=config_dir / "sync.lock", ) def respond(request: httpx.Request) -> httpx.Response: diff --git a/tests/test_update_cli.py b/tests/test_update_cli.py index ced9841..9f04b05 100644 --- a/tests/test_update_cli.py +++ b/tests/test_update_cli.py @@ -180,3 +180,48 @@ def test_should_not_suppress_background_notice_for_plain_text_commands( assert should_suppress is not None assert should_suppress(args, env) is False + + +@pytest.mark.parametrize( + "args,env", + [ + # Inherits every background-notice suppression case. + (["--version"], {}), + (["--help"], {}), + (["whoami", "--json"], {}), + (["logout"], {"CI": "1"}), + # Plus any explicit `workflows sync ...` command: never auto-pull while + # the user is running sync by hand. + (["workflows", "sync"], {}), + (["workflows", "sync", "pull"], {}), + (["workflows", "sync", "push"], {}), + (["workflows", "sync", "status"], {}), + (["workflows", "sync", "auto", "on"], {}), + (["workflows", "sync", "target", "list"], {}), + ], +) +def test_should_suppress_auto_pull_for_meta_and_sync_commands( + args: list[str], + env: dict[str, str], +) -> None: + should_suppress = getattr(update_module, "should_suppress_auto_pull", None) + + assert should_suppress is not None + assert should_suppress(args, env) is True + + +@pytest.mark.parametrize( + "args", + [ + ["logout"], + ["workflows", "list"], + # A non-sync workflows subcommand is eligible for an automatic pull tail. + ["workflows", "get", "some-slug"], + ["templates", "list"], + ], +) +def test_should_not_suppress_auto_pull_for_ordinary_commands(args: list[str]) -> None: + should_suppress = getattr(update_module, "should_suppress_auto_pull", None) + + assert should_suppress is not None + assert should_suppress(args, {}) is False