fix: 修复上游健康巡检并同步已审查的市场引用 (#207)

Upstream health checks could select a different branch ending in `main`,
index internal skills or lowercase reference documents, treat a valid
pinned path as broken after an upstream move, and close the tracking
issue while unpinned or unknown entries remained. Resolve exact refs and
tracked filenames, compare the installed snapshot, and drive issue
decisions from structured results.

Refresh 26 reviewed pointers, pin DingTalk and Matt Pocock collections,
remove the deleted merge-conflict skill, and add the six new public
children with reviewed translations. Synchronize sidecars, observed
release facts, license evidence and external dependency disclosures.
Identical-tree Agent pins and their versions remain unchanged. The
per-entry review is recorded in
`docs/upstream-health-189-remediation.md`.

Validation: 99 unit tests pass; all nine pinned collections reproduce;
catalog/i18n and translation-freshness checks pass; online health checks
all 41 external entries with zero broken, actionable or unknown results.
Existing catalog warnings decrease from 123 to 105 with no new warning
groups. Python/workflow shell syntax and whitespace checks pass.

Closes #189.

scripts/catalog/check_upstream_health.py

@@ -14,17 +14,17 @@ script looks at each upstream and reports, without rewriting anything:
   bad-branch  source.repoBranch no longer exists upstream
   bad-ref     pinned source.ref can no longer be fetched (history rewritten)
   bad-path    source.path, the root SKILL.md, or a declared child path is missing at
-              branch head
+              the installed snapshot (source.ref, or branch head when unpinned)
   unreachable network error other than "not found"; treated as unknown, not broken
   new-skill   branch head has collection children that `children` does not declare
               (same discovery rules as scripts/gen-collection-children.py)
-  outdated    pinned source.ref is behind branch head; an update is available
+  upstream-path-change  a declared path moved at branch head but the pin still works
+  outdated    pinned content differs from branch head; review before updating
   unpinned    entry follows a moving branch without source.ref
   ok          nothing to report
 
-Path checks run against branch head: they answer "will this pointer still work
-after the next ref bump", while `bad-ref` answers "does the pinned ref still
-resolve". Only skills get path checks; agents and teams are AgentFS repositories.
+Path checks distinguish the installed snapshot from changes at branch head.
+Only skills get path checks; agents and teams are AgentFS repositories.
 
 Usage:
   uv run scripts/catalog/check_upstream_health.py                  # text table
@@ -55,7 +55,8 @@ REPO_ROOT = Path(__file__).resolve().parents[2]
 KINDS = ("skills", "agents", "teams")
 
 BROKEN = ("gone", "bad-branch", "bad-ref", "bad-path")
-ORDER = (*BROKEN, "unreachable", "new-skill", "outdated", "unpinned", "ok")
+ACTIONABLE = (*BROKEN, "upstream-path-change", "new-skill", "outdated", "unpinned")
+ORDER = (*BROKEN, "unreachable", "upstream-path-change", "new-skill", "outdated", "unpinned", "ok")
 
 # Git asks for credentials when a public repository disappears; never prompt.
 GIT_ENV = {**os.environ, "GIT_TERMINAL_PROMPT": "0", "GCM_INTERACTIVE": "never"}
@@ -100,12 +101,14 @@ def check_git(entry: dict, kind: str) -> list[tuple[str, str]]:
     ref = source.get("ref")
     issues: list[tuple[str, str]] = []
 
-    query = ["--heads", url, branch] if branch else [url, "HEAD"]
+    advertised_ref = f"refs/heads/{branch}" if branch else "HEAD"
+    query = ["--heads", url, advertised_ref] if branch else [url, "HEAD"]
     remote = git("ls-remote", *query, timeout=90)
     if remote.returncode != 0:
         detail = last_line(remote.stderr)
         return [("gone" if NOT_FOUND.search(remote.stderr) else "unreachable", detail)]
-    heads = [line.split()[0] for line in remote.stdout.splitlines() if line.strip()]
+    heads = [line.split()[0] for line in remote.stdout.splitlines()
+             if len(line.split()) == 2 and line.split()[1] == advertised_ref]
     if branch and not heads:
         return [("bad-branch", f"branch '{branch}' not found upstream")]
     if not heads:
@@ -121,30 +124,55 @@ def check_git(entry: dict, kind: str) -> list[tuple[str, str]]:
         if cloned.returncode != 0:
             return [("unreachable", f"clone failed: {last_line(cloned.stderr)}")]
 
+        # The branch can advance between advertisement and clone. Inspect the
+        # actual cloned snapshot throughout this check.
+        cloned_head = git("rev-parse", "HEAD", cwd=repo)
+        if cloned_head.returncode != 0:
+            return [("unreachable", f"cannot resolve clone: {last_line(cloned_head.stderr)}")]
+        head = cloned_head.stdout.strip()
+
         if ref and ref != head:
             fetched = git("fetch", "--quiet", "--depth", "1", "origin", ref, cwd=repo, timeout=180)
             if fetched.returncode != 0:
-                issues.append(("bad-ref", f"pinned {ref[:12]} is no longer fetchable"))
+                missing = re.search(r"not our ref|couldn't find remote ref|unadvertised object|not found",
+                                    fetched.stderr, re.IGNORECASE)
+                status = "bad-ref" if missing else "unreachable"
+                return [(status, f"cannot fetch pinned {ref[:12]}: {last_line(fetched.stderr)}")]
 
-        if kind == "skills":
-            issues += check_skill_paths(entry, repo)
+        try:
+            if kind == "skills":
+                if ref and ref != head:
+                    issues += check_skill_paths(entry, repo, ref, discover=False)
+                    upstream = check_skill_paths(entry, repo, head)
+                    issues += [("upstream-path-change" if status == "bad-path" else status, detail)
+                               for status, detail in upstream
+                               if status != "bad-path" or ("bad-path", detail) not in issues]
+                else:
+                    issues += check_skill_paths(entry, repo, head)
+        except RuntimeError as err:
+            return [("unreachable", str(err)[:300])]
+
+        if ref and ref != head:
+            # A single-skill pointer installs only source.path. Upstream feeds,
+            # site builds and unrelated skills must not cause endless repins.
+            # Root licensing changes still need review even for a subtree.
+            path = (source.get("path") or "").strip("/")
+            scope = ["--", path, ":(top,glob)LICENSE*", ":(top,glob)NOTICE*", ":(top,glob)COPYING*"] \
+                if kind == "skills" and path else []
+            difference = git("diff", "--quiet", ref, head, *scope, cwd=repo)
+            if difference.returncode == 1:
+                issues.append(("outdated", f"{ref[:7]} -> {head[:7]}"))
+            elif difference.returncode != 0:
+                issues.append(("unreachable", f"cannot compare snapshots: {last_line(difference.stderr)}"))
 
-    if ref and ref != head:
-        issues.append(("outdated", f"{ref[:7]} -> {head[:7]}"))
     if not ref:
         issues.append(("unpinned", f"follows '{branch or 'HEAD'}' at {head[:7]}"))
     return issues
 
 
-def check_skill_paths(entry: dict, repo: Path) -> list[tuple[str, str]]:
-    # Materialise only SKILL.md files; the generator reads their frontmatter.
-    git("sparse-checkout", "set", "--no-cone", "SKILL.md", cwd=repo)
-    checkout = git("checkout", "--quiet", "HEAD", cwd=repo, timeout=300)
-    if checkout.returncode != 0:
-        return [("unreachable", f"checkout failed: {last_line(checkout.stderr)}")]
-
-    skill_dirs = {p.parent.relative_to(repo).as_posix() for p in repo.rglob("SKILL.md")
-                  if ".git" not in p.parts}
+def check_skill_paths(entry: dict, repo: Path, revision: str = "HEAD", *,
+                      discover: bool = True) -> list[tuple[str, str]]:
+    skill_dirs = {Path(path).parent.as_posix() for path in GENERATOR.skill_paths(repo, revision)}
     skill_dirs = {"" if d == "." else d for d in skill_dirs}
     issues: list[tuple[str, str]] = []
     children = entry.get("children") or []
@@ -154,7 +182,8 @@ def check_skill_paths(entry: dict, repo: Path) -> list[tuple[str, str]]:
         for child in children:
             if child["path"].strip("/") not in skill_dirs:
                 issues.append(("bad-path", f"child '{child['id']}' -> '{child['path']}' has no SKILL.md"))
-        discovered = [c for c in GENERATOR.discover_children(repo) if c["path"] not in declared]
+        discovered = [c for c in GENERATOR.discover_children(repo, revision)
+                      if c["path"] not in declared] if discover else []
         declared_ids = {c["id"] for c in children}
         extra = [c["path"] for c in discovered if c["id"] not in declared_ids]
         if extra:
@@ -216,6 +245,7 @@ def render_markdown(results: list[dict]) -> str:
     sections = [
         ("Broken — installation fails", BROKEN),
         ("Unreachable — network error, status unknown", ("unreachable",)),
+        ("Upstream path changes — pinned installation still resolves", ("upstream-path-change",)),
         ("Undeclared upstream skills", ("new-skill",)),
         ("Updates available", ("outdated",)),
         ("Unpinned", ("unpinned",)),
@@ -235,11 +265,22 @@ def render_markdown(results: list[dict]) -> str:
     return "\n".join(out)
 
 
+def summarize(results: list[dict]) -> dict:
+    statuses = {issue["status"] for result in results for issue in result["issues"]}
+    return {
+        "checked": len(results),
+        "broken": bool(statuses.intersection(BROKEN)),
+        "actionable": bool(statuses.intersection(ACTIONABLE)),
+        "unknown": "unreachable" in statuses,
+    }
+
+
 def main(argv: list[str] | None = None) -> int:
     parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
     parser.add_argument("ids", nargs="*", help="entry IDs to check (default: every external entry)")
     parser.add_argument("--format", choices=("text", "markdown", "json"), default="text")
     parser.add_argument("--jobs", type=int, default=8, help="parallel upstream checks")
+    parser.add_argument("--summary-file", type=Path, help="write machine-readable workflow decisions")
     args = parser.parse_args(argv)
 
     paths = [p for kind in KINDS for p in sorted((REPO_ROOT / kind).glob("*/entry.json"))]
@@ -253,13 +294,16 @@ def main(argv: list[str] | None = None) -> int:
     with ThreadPoolExecutor(max_workers=max(1, args.jobs)) as pool:
         results = list(pool.map(check_entry, paths))
 
+    if args.summary_file:
+        args.summary_file.write_text(json.dumps(summarize(results), indent=2) + "\n", encoding="utf-8")
+
     if args.format == "json":
         print(json.dumps(results, ensure_ascii=False, indent=2))
     elif args.format == "markdown":
         print(render_markdown(results))
     else:
         print(render_text(results))
-    return 1 if any(i["status"] in BROKEN for r in results for i in r["issues"]) else 0
+    return 1 if summarize(results)["broken"] else 0
 
 
 if __name__ == "__main__":

scripts/catalog/test_check_upstream_health.py

@@ -14,6 +14,7 @@ import sys
 import tempfile
 import unittest
 from pathlib import Path
+from unittest.mock import patch
 
 
 CHECKER_PATH = Path(__file__).with_name("check_upstream_health.py")
@@ -86,11 +87,19 @@ class UpstreamHealthTests(unittest.TestCase):
         self.assertIn("bad-path", result)
 
     def test_newer_head_is_outdated(self) -> None:
-        self.commit({"README.md": "x"})
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
         result = self.statuses(self.entry(ref=self.first, path="skills/alpha"))
         self.assertIn("outdated", result)
         self.assertNotIn("bad-ref", result)
 
+    def test_unrelated_feed_refresh_does_not_offer_a_skill_update(self) -> None:
+        self.commit({"data/latest.json": '{"updated": true}', "README.md": "Updated website.\n"})
+        self.assertEqual(self.statuses(self.entry(ref=self.first, path="skills/alpha")), {"ok": ""})
+
+    def test_root_license_change_still_needs_review_for_a_subtree(self) -> None:
+        self.commit({"LICENSE": "New licensing terms.\n"})
+        self.assertIn("outdated", self.statuses(self.entry(ref=self.first, path="skills/alpha")))
+
     def test_unfetchable_ref_is_bad_ref(self) -> None:
         result = self.statuses(self.entry(ref="0" * 40, path="skills/alpha"))
         self.assertIn("bad-ref", result)
@@ -99,6 +108,62 @@ class UpstreamHealthTests(unittest.TestCase):
         result = self.statuses(self.entry(path="skills/alpha"))
         self.assertIn("unpinned", result)
 
+    def test_branch_suffix_is_not_the_requested_branch(self) -> None:
+        sh("git", "branch", "-m", "changeset-release/main", cwd=self.upstream)
+        self.assertIn("bad-branch", self.statuses(self.entry(ref=self.first, path="skills/alpha")))
+
+    def test_exact_branch_ignores_another_main_suffix(self) -> None:
+        sh("git", "branch", "changeset-release/main", cwd=self.upstream)
+        head = self.commit({"README.md": "New content.\n"})
+        self.assertEqual(self.statuses(self.entry(ref=head, path="skills/alpha")), {"ok": ""})
+
+    def test_identical_tree_does_not_offer_an_update(self) -> None:
+        sh("git", "commit", "--quiet", "--allow-empty", "-m", "merge metadata", cwd=self.upstream)
+        self.assertEqual(self.statuses(self.entry(ref=self.first, path="skills/alpha")), {"ok": ""})
+
+    def test_moved_upstream_path_does_not_break_valid_pin(self) -> None:
+        (self.upstream / "skills/alpha/SKILL.md").unlink()
+        self.commit({"skills/beta/SKILL.md": "---\nname: beta\ndescription: B.\n---\n"})
+        result = self.statuses(self.entry(ref=self.first, path="skills/alpha"))
+        self.assertIn("upstream-path-change", result)
+        self.assertNotIn("bad-path", result)
+        self.assertIn("bad-path", self.statuses(self.entry(path="skills/alpha")))
+
+    def test_lowercase_reference_is_not_a_skill(self) -> None:
+        head = self.commit({"skills/beta/skill.md": "Reference documentation.\n"})
+        self.assertIn("bad-path", self.statuses(self.entry(ref=head, path="skills/beta")))
+
+    def test_fetch_network_error_is_unknown(self) -> None:
+        self.commit({"README.md": "New content.\n"})
+        real_git = CHECKER.git
+
+        def failing_fetch(*args, **kwargs):
+            if args[0] == "fetch":
+                return subprocess.CompletedProcess(args, 128, "", "Connection reset by peer")
+            return real_git(*args, **kwargs)
+
+        with patch.object(CHECKER, "git", side_effect=failing_fetch):
+            result = self.statuses(self.entry(ref=self.first, path="skills/alpha"))
+        self.assertIn("unreachable", result)
+        self.assertNotIn("bad-ref", result)
+
+    def test_internal_child_is_not_offered(self) -> None:
+        head = self.commit({"skills/mono/SKILL.md": "---\nname: dws\nmetadata:\n  internal: true\n---\n"})
+        result = self.statuses(self.entry(ref=head, children=[{"id": "alpha", "path": "skills/alpha"}]))
+        self.assertEqual(result, {"ok": ""})
+
+    def test_summary_keeps_unpinned_and_unknown_entries_visible(self) -> None:
+        def results(status):
+            return [{"id": "a", "issues": [{"status": status, "detail": ""}]}]
+
+        self.assertEqual(CHECKER.summarize(results("unpinned")),
+                         {"checked": 1, "broken": False, "actionable": True, "unknown": False})
+        self.assertEqual(CHECKER.summarize(results("upstream-path-change"))["actionable"], True)
+        self.assertEqual(CHECKER.summarize(results("unreachable")),
+                         {"checked": 1, "broken": False, "actionable": False, "unknown": True})
+        self.assertEqual(CHECKER.summarize(results("ok")),
+                         {"checked": 1, "broken": False, "actionable": False, "unknown": False})
+
     def test_collection_reports_missing_and_new_children(self) -> None:
         head = self.commit({"skills/beta/SKILL.md": "---\nname: beta\ndescription: B.\n---\n"})
         children = [{"id": "alpha", "path": "skills/alpha"}, {"id": "gamma", "path": "skills/gamma"}]

scripts/catalog/test_collection_generator.py

@@ -9,6 +9,7 @@ from __future__ import annotations
 
 import importlib.util
 import json
+import subprocess
 import sys
 import tempfile
 import unittest
@@ -76,9 +77,65 @@ class CollectionGeneratorCheckTests(unittest.TestCase):
             [{"id": "child-one", "path": "skills/child-one"}],
         )
 
+    def test_discovery_excludes_internal_and_reference_documents(self) -> None:
+        repo = Path(self.tempdir.name) / "repo"
+        files = {
+            "skills/public/SKILL.md": "---\nname: public\nmetadata:\n  internal: false\n---\n",
+            "skills/mono/SKILL.md": "---\nname: dws\nmetadata:\n  internal: true\n---\n",
+            "skills/public/references/skill.md": "Reference documentation.\n",
+            "skills/invalid/SKILL.md": "Not a skill frontmatter.\n",
+        }
+        for path, text in files.items():
+            target = repo / path
+            target.parent.mkdir(parents=True, exist_ok=True)
+            target.write_text(text, encoding="utf-8")
+        expected = [{"id": "public", "path": "skills/public"}]
+        self.assertEqual(GENERATOR.discover_children(repo), expected)
+
+        for args in (("init", "--quiet"), ("add", "."),
+                     ("-c", "user.name=t", "-c", "user.email=t@example.com",
+                      "commit", "--quiet", "-m", "fixture")):
+            subprocess.run(["git", *args], cwd=repo, check=True, capture_output=True)
+        self.assertEqual(GENERATOR.discover_children(repo), expected)
+
+    def test_discovery_excludes_symlink_skills(self) -> None:
+        repo = Path(self.tempdir.name) / "repo"
+        real = repo / "skills/public/SKILL.md"
+        real.parent.mkdir(parents=True)
+        real.write_text("---\nname: public\n---\n", encoding="utf-8")
+        linked = repo / "skills/link/SKILL.md"
+        linked.parent.mkdir()
+        linked.symlink_to(real)
+        self.assertEqual(GENERATOR.discover_children(repo), [{"id": "public", "path": "skills/public"}])
+
+    def test_child_order_preserves_directory_sorting(self) -> None:
+        repo = Path(self.tempdir.name) / "repo"
+        for child in ("alpha-extra", "alpha"):
+            path = repo / "skills" / child / "SKILL.md"
+            path.parent.mkdir(parents=True)
+            path.write_text(f"---\nname: {child}\n---\n", encoding="utf-8")
+        self.assertEqual([c["id"] for c in GENERATOR.discover_children(repo)], ["alpha", "alpha-extra"])
+
     def test_check_accepts_current_children_without_writing(self) -> None:
         self.assertTrue(self.run_check(self.children))
 
+    def test_reviewed_translation_is_reproducible(self) -> None:
+        overrides = {"child-one": {"i18n": {"zh-CN": {"shortDesc": "已审阅的中文说明。"},
+                                           "en-US": {"shortDesc": "Reviewed summary."}}}}
+        (self.entry_path.parent / "collection-child-overrides.json").write_text(json.dumps(overrides))
+        expected = [{**self.children[0], **overrides["child-one"]}]
+        self.write_entry(expected)
+        self.assertTrue(self.run_check(self.children))
+
+    def test_overrides_cannot_hide_missing_child_or_change_source_facts(self) -> None:
+        path = self.entry_path.parent / "collection-child-overrides.json"
+        for overrides in ({"missing": {"i18n": {}}}, {"child-one": {"path": "elsewhere"}},
+                          {"child-one": {"version": "9.0.0"}},
+                          {"child-one": {"i18n": {"en-US": {"shortDesc": ""}}}}):
+            with self.subTest(overrides=overrides):
+                path.write_text(json.dumps(overrides))
+                self.assertFalse(self.run_check(self.children))
+
     def test_check_rejects_stale_children_without_writing(self) -> None:
         self.assertFalse(self.run_check([{"id": "child-two", "path": "skills/child-two"}]))