fix(cli): fail install when skill links fail

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

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
 

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")
 

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'")]