From 312f132ae0d571dc4b34e1275e5543de588ebdf8 Mon Sep 17 00:00:00 2001
From: cyre <Lawrence@cyre.me>
Date: Fri, 7 Aug 2026 06:39:13 +0800
Subject: [PATCH] fix(upgrade): loop-* skill copy doubles the skills/ path
 segment

_new_loop_assets() computed each new loop-* skill's destination as
dst_skills / src.relative_to(src_agent) -- but src.relative_to(src_agent)
already starts with "skills/" (src_agent is .agent/, not .agent/skills/),
and dst_skills is already .agent/skills/. Result: every genuinely-new
loop-* skill lands at .agent/skills/skills/loop-x/SKILL.md instead of
.agent/skills/loop-x/SKILL.md -- both the CLI's own --dry-run report and
the real copy were wrong, silently, since no existing test exercised a
fresh (non-pre-seeded) loop-* skill destination.

Fix: dst_agent / src.relative_to(src_agent), matching the sibling
non-loop skill-copy block earlier in the same function (line ~84), which
already gets this right.

New test: test_upgrade_copies_missing_loop_skills_to_the_correct_path.
Confirmed it fails on the old code (asserts the doubled path is absent)
and passes on the fix.
---
 harness_manager/upgrade.py      |  2 +-
 tests/test_loop_integrations.py | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/harness_manager/upgrade.py b/harness_manager/upgrade.py
index e9d7ad7..f187ecf 100644
--- a/harness_manager/upgrade.py
+++ b/harness_manager/upgrade.py
@@ -108,7 +108,7 @@ def _new_loop_assets(src_agent: Path, dst_agent: Path) -> list[tuple[Path, Path]
         if not skill_dir.is_dir() or (dst_skills / skill_dir.name).exists():
             continue
         for src in sorted(p for p in skill_dir.rglob("*") if p.is_file()):
-            actions.append((src, dst_skills / src.relative_to(src_agent)))
+            actions.append((src, dst_agent / src.relative_to(src_agent)))
     return actions
 
 
diff --git a/tests/test_loop_integrations.py b/tests/test_loop_integrations.py
index 01d4b1b..b277a98 100644
--- a/tests/test_loop_integrations.py
+++ b/tests/test_loop_integrations.py
@@ -42,6 +42,21 @@ def test_upgrade_adds_missing_loop_assets_but_preserves_authored_contract(tmp_pa
     assert (target / ".agent" / "runtime" / ".gitignore").exists()
 
 
+def test_upgrade_copies_missing_loop_skills_to_the_correct_path(tmp_path: Path):
+    """A genuinely-missing loop-* skill must land at .agent/skills/loop-x/,
+    not .agent/skills/skills/loop-x/ (a doubled path segment regression:
+    the existing coverage above only ever pre-seeds loop-triage, so it
+    exercises the "already exists, skip" branch and never the fresh-copy
+    branch that builds the destination path).
+    """
+    target = make_installed_project(tmp_path)
+    assert upgrade(target, ROOT, yes=True) == 0
+    for name in ("loop-constraints", "loop-guard", "loop-triage", "loop-verifier"):
+        dst = target / ".agent" / "skills" / name / "SKILL.md"
+        assert dst.is_file(), f"expected {dst}, not doubled under skills/skills/"
+        assert not (target / ".agent" / "skills" / "skills").exists()
+
+
 def test_upgrade_does_not_copy_runtime_children_or_overwrite_loop_skills(tmp_path: Path):
     target = make_installed_project(tmp_path)
     skill = target / ".agent" / "skills" / "loop-triage"