diff --git a/src/rgit/cli.py b/src/rgit/cli.py index 860ef04..8cadee3 100644 --- a/src/rgit/cli.py +++ b/src/rgit/cli.py @@ -399,7 +399,8 @@ def _render_install_result(res: dict) -> None: print(f" • would run: {' '.join(cmd)}") if res.get("links"): if res.get("ran"): - print(f" ✓ skills linked into {res.get('skills_dir')}") + mark = "✗" if res.get("errors") else "✓" + print(f" {mark} skills linked into {res.get('skills_dir')}") else: print(f" • would link {len(res['links'])} skill(s) into " f"{res.get('skills_dir')}") @@ -426,10 +427,18 @@ def _render_install_result(res: dict) -> None: print(f" ✓ guidance {action}: {path}".rstrip(": ")) if g.get("hint"): print(f" hint: {g['hint']}") - if res.get("instructions"): + if res.get("instructions") and not res.get("errors"): print(f" → {res['instructions']}") +def _install_result_failed(res: dict) -> bool: + if res.get("errors"): + return True + if (res.get("guidance") or {}).get("action") == "skipped_error": + return True + return any(r.get("rc", 0) != 0 for r in res.get("results", [])) + + def _sole_open_proposal(store: Store) -> str: """The only open proposal's id; ValueError otherwise. @@ -863,13 +872,14 @@ def _dispatch(args, parser) -> int: # installs yield one entry per detected client. payload = results[0] if args.platform else results print(json.dumps(payload, indent=2, ensure_ascii=False)) - return 0 + return 1 if any(_install_result_failed(r) for r in results) else 0 for res in results: _render_install_result(res) - if not args.uninstall: + failed = any(_install_result_failed(r) for r in results) + if not args.uninstall and not failed: print("\nrestart your CLI/agent session to pick up the skills") print("note: `rgit install-hooks` enables per-commit capture (opt-in)") - return 0 + return 1 if failed else 0 if args.cmd == "install-hooks": from .hooks import install_hooks, uninstall_hooks diff --git a/tests/test_cli.py b/tests/test_cli.py index 5dd559d..901c6b7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -46,6 +46,71 @@ def test_install_list_and_dry_run(capsys): assert "marketplace" in out2 and "research-git@research-git" in out2 +def _skill_link_failure_result(): + return { + "platform": "codex", + "links": [{"link": "/home/.agents/skills/rgit-capture", + "target": "/pkg/rgit/_plugin/skills/rgit-capture"}], + "skills_dir": "/home/.agents/skills", + "errors": [{"link": "/home/.agents/skills/rgit-capture", + "error": "privilege not held", + "hint": "enable Developer Mode"}], + "guidance": {"action": "skipped_error", + "path": "/home/.codex/AGENTS.md", + "error": "skill symlink failed"}, + "instructions": "Skills symlinked into /home/.agents/skills. MCP config: {}", + "ran": True, + } + + +def test_install_returns_nonzero_when_skill_links_fail(monkeypatch, capsys): + from rgit import installer + + monkeypatch.setattr(installer, "install", + lambda *a, **k: _skill_link_failure_result()) + + assert cli.main(["install", "codex", "--guidance", "default"]) == 1 + captured = capsys.readouterr() + assert "privilege not held" in captured.out + assert "skill symlink failed" in captured.out + assert "Skills symlinked into" not in captured.out + assert "restart your CLI/agent session" not in captured.out + + +def test_install_json_returns_nonzero_when_skill_links_fail(monkeypatch, capsys): + from rgit import installer + + monkeypatch.setattr(installer, "install", + lambda *a, **k: _skill_link_failure_result()) + + assert cli.main(["install", "codex", "--json", "--guidance", "default"]) == 1 + res = json.loads(capsys.readouterr().out) + assert res["errors"][0]["error"] == "privilege not held" + + +def test_install_returns_nonzero_when_guidance_write_fails(monkeypatch, capsys): + from rgit import installer + + monkeypatch.setattr( + installer, "install", + lambda *a, **k: { + "platform": "codex", + "links": [{"link": "/home/.agents/skills/rgit-capture", + "target": "/pkg/rgit/_plugin/skills/rgit-capture"}], + "skills_dir": "/home/.agents/skills", + "guidance": {"action": "skipped_error", + "path": "/home/.codex/AGENTS.md", + "error": "permission denied"}, + "ran": True, + }, + ) + + assert cli.main(["install", "codex", "--guidance", "default"]) == 1 + captured = capsys.readouterr() + assert "permission denied" in captured.out + assert "restart your CLI/agent session" not in captured.out + + def test_run_from_links_variant_and_refreshes_guide(git_repo, monkeypatch, tmp_path): from rgit.store.models import Capsule, CodeSlice monkeypatch.chdir(git_repo) @@ -739,7 +804,7 @@ def test_run_without_store_suggests_init_flag(git_repo, monkeypatch, capsys): def test_run_with_init_flag_bootstraps_store(git_repo, monkeypatch): monkeypatch.chdir(git_repo) cli._SEGMENTER = MockSegmenter([]) - assert cli.main(["run", "--init", "--", "true"]) == 0 + assert cli.main(["run", "--init", "--", sys.executable, "-c", "pass"]) == 0 assert (git_repo / ".rgit" / "graph.db").exists() assert not (git_repo / ".git" / "hooks" / "post-commit").exists() # --init never installs hooks diff --git a/tests/test_installer.py b/tests/test_installer.py index cf44570..997320a 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -26,6 +26,21 @@ def fake_home(tmp_path, monkeypatch): return tmp_path +@pytest.fixture +def symlink_home(fake_home): + """Fake home for integration tests that require real directory symlinks.""" + target = fake_home / "symlink-target" + link = fake_home / "symlink-probe" + target.mkdir() + try: + link.symlink_to(target, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"directory symlink creation unavailable: {exc}") + else: + link.unlink() + return fake_home + + def guidance_path(res): return Path(res["guidance"]["path"]) @@ -134,7 +149,8 @@ def test_guidance_target_opencode_honors_xdg_config_home(tmp_path, monkeypatch): == xdg / "opencode" / "AGENTS.md") -def test_install_codex_mode_none_skips_guidance_write(fake_home): +def test_install_codex_mode_none_skips_guidance_write(symlink_home): + fake_home = symlink_home res = installer.install("codex", mode="none") assert res["ran"] is True assert (fake_home / ".agents" / "skills" / "rgit-capture").is_symlink() @@ -158,14 +174,16 @@ def test_install_claude_code_mode_none_dry_run(fake_home): assert res["guidance"] == {"action": "disabled"} -def test_install_codex_mode_manual_only_pins_mode(fake_home): +def test_install_codex_mode_manual_only_pins_mode(symlink_home): + fake_home = symlink_home res = installer.install("codex", mode="manual-only") assert res["guidance"]["action"] == "created" text = (fake_home / ".codex" / "AGENTS.md").read_text(encoding="utf-8") assert "Current mode: manual-only" in text -def test_explicit_mode_overrides_previously_pinned_mode(fake_home): +def test_explicit_mode_overrides_previously_pinned_mode(symlink_home): + fake_home = symlink_home installer.install("codex", mode="manual-only") installer.install("codex", mode="default") text = (fake_home / ".codex" / "AGENTS.md").read_text(encoding="utf-8") @@ -227,7 +245,8 @@ def fail(plan): assert agent_guidance._START_RE.search(guidance.read_text(encoding="utf-8")) -def test_install_codex_writes_guidance_and_symlinks_under_fake_home(fake_home): +def test_install_codex_writes_guidance_and_symlinks_under_fake_home(symlink_home): + fake_home = symlink_home res = installer.install("codex") assert res["ran"] is True @@ -238,7 +257,8 @@ def test_install_codex_writes_guidance_and_symlinks_under_fake_home(fake_home): assert res["guidance"]["action"] == "created" -def test_install_codex_is_idempotent_for_guidance(fake_home): +def test_install_codex_is_idempotent_for_guidance(symlink_home): + fake_home = symlink_home installer.install("codex") res = installer.install("codex") @@ -282,6 +302,7 @@ def boom(path, *, mode=None, dry_run=False): raise OSError("nope") monkeypatch.setattr(agent_guidance, "upsert_managed_block", boom) + monkeypatch.setattr(Path, "symlink_to", lambda *a, **k: None) res = installer.install("codex") diff --git a/tests/test_review_fixes.py b/tests/test_review_fixes.py index 159de58..b3c5dd6 100644 --- a/tests/test_review_fixes.py +++ b/tests/test_review_fixes.py @@ -1,5 +1,6 @@ """Regression tests for the PR #2 code-review findings (mine + Codex).""" import io +import sys import tarfile import pytest @@ -115,7 +116,7 @@ def test_provenance_tolerates_binary_artifact_file(git_repo): def test_cli_run_with_unknown_capsule_returns_nonzero(git_repo, capsys, monkeypatch): monkeypatch.chdir(git_repo) Store.init(git_repo) - rc = main(["run", "--with", "nope", "--", "true"]) + rc = main(["run", "--with", "nope", "--", sys.executable, "-c", "pass"]) assert rc == 1 assert "no capsule" in capsys.readouterr().out.lower() @@ -124,7 +125,7 @@ def test_cli_run_with_resolves_name_to_active_edge(git_repo, capsys, monkeypatch monkeypatch.chdir(git_repo) store = Store.init(git_repo) a = _cap(store, "A") - rc = main(["run", "--with", "A", "--", "true"]) # name, not id + rc = main(["run", "--with", "A", "--", sys.executable, "-c", "pass"]) assert rc == 0 dsts = [r["dst"] for r in store.conn.execute("SELECT dst FROM edges WHERE type='active'")]