fix: 上游巡检收窄集合对比范围并支持已审查暂缓

Collections install only their declared children: the client copies each selected child path at the pinned ref. Yet `outdated` compared the whole upstream repository, so CLI sources, end-to-end tests and changesets forced repins. Compare the declared child paths plus root license/notice files instead; undeclared upstream skills are still reported as `new-skill`. A pin kept on purpose after review was reported again every week, so the tracking issue could never close. A reviewed `upstream-health-hold.json` beside `entry.json` now reports its listed `upstream-path-change`/`outdated` findings as `held` (not actionable) while the pin and the installed-scope content at the reviewed upstream head are unchanged. Otherwise the findings return with an actionable `stale-hold`. Broken, unreachable, `new-skill` and `unpinned` findings cannot be held. CI tests require every hold to be well-formed and to match its entry's `source.ref`. A network error while checking a reviewed head stays `unreachable`. `outdated` rows in the tracking issue now link to the GitHub compare view. The summary JSON reports the held count, and the closing comment mentions remaining holds. Both READMEs document the comparison scope, the hold format and the links. Validation: 42 upstream-health, 10 collection-generator, 51 catalog-metadata and 19 i18n tests pass. Catalog metadata, i18n (0 errors, 106 existing warnings) and translation-freshness checks pass. actionlint and `git diff --check` are clean. A live run over 111 entries reports no broken or unknown entries, and humanlayer-skills is no longer `outdated`. A temporary geo-optimizer hold turned both of its findings into `held` with `actionable: false`; it is not committed. Refs #213.
2026-10-22 18:53:47 +08:00 · 2026-10-18 18:35:38 +08:00
parent a514b55b68
commit 10cb7656db
5 changed files with 442 additions and 33 deletions
--- a/.github/workflows/upstream-health.yml
+++ b/.github/workflows/upstream-health.yml
@@ -45,7 +45,7 @@ jobs:
          import json
          from pathlib import Path
          summary = json.loads(Path("summary.json").read_text())
-          for key in ("broken", "actionable", "unknown"):
+          for key in ("broken", "actionable", "unknown", "held"):
              print(f"{key}={str(summary[key]).lower()}")
          PY
          cat report.md >> "$GITHUB_STEP_SUMMARY"
@@ -55,6 +55,7 @@ jobs:
          GH_TOKEN: ${{ github.token }}
          ACTIONABLE: ${{ steps.check.outputs.actionable }}
          UNKNOWN: ${{ steps.check.outputs.unknown }}
+          HELD: ${{ steps.check.outputs.held }}
          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
        run: |
          set -euo pipefail
@@ -65,7 +66,7 @@ jobs:
          {
            echo "_Updated by [Upstream Health]($RUN_URL) on $(date -u +%Y-%m-%d)._"
            echo
-            echo "Fix entries through a normal PR: repoint or remove \`gone\` entries, correct \`bad-*\` paths, and review upstream changes before bumping \`source.ref\` for \`outdated\` entries (rerun \`scripts/gen-collection-children.py\` for collections). \`unreachable\` means a network error, not a confirmed failure."
+            echo "Fix entries through a normal PR: repoint or remove \`gone\` entries, correct \`bad-*\` paths, and review upstream changes before bumping \`source.ref\` for \`outdated\` entries (rerun \`scripts/gen-collection-children.py\` for collections); a reviewed decision to keep the current pin can be recorded in \`<kind>/<id>/upstream-health-hold.json\`. \`unreachable\` means a network error, not a confirmed failure."
            echo
            cat report.md
          } > body.md
@@ -78,7 +79,11 @@ jobs:
              gh issue create --title "Upstream health report" --label "$label" --body-file body.md
            fi
          elif [ -n "$issue" ]; then
-            gh issue comment "$issue" --body "All external entries resolve upstream and are current as of $RUN_URL. Closing."
+            closing="All external entries resolve upstream and are current as of $RUN_URL."
+            if [ "${HELD:-0}" -gt 0 ]; then
+              closing="All external entries resolve upstream as of $RUN_URL; $HELD finding(s) remain under reviewed holds (see the run summary)."
+            fi
+            gh issue comment "$issue" --body "$closing Closing."
            gh issue close "$issue"
          fi

--- a/README.md
+++ b/README.md
@@ -237,9 +237,23 @@ uv run scripts/catalog/check_upstream_health.py

 Collection checking requires Git and network access. Mutable collections are reported and skipped because their generated inventory is not reproducible. Omitting `--check` runs the generator in write mode; review both the regenerated children and the corresponding sidecar facts before committing.

-The upstream health check is read-only and exits non-zero when an entry is `gone` or has a `bad-branch`, `bad-ref`, or `bad-path`. `unreachable` is a network error, not a confirmed failure. The [upstream health workflow](.github/workflows/upstream-health.yml) runs it weekly and keeps one `upstream-health` issue current; fixes and `source.ref` bumps still go through reviewed PRs.
+The upstream health check is read-only and exits non-zero when an entry is `gone` or has a `bad-branch`, `bad-ref`, or `bad-path`. `unreachable` is a network error, not a confirmed failure. The [upstream health workflow](.github/workflows/upstream-health.yml) runs it weekly and keeps one `upstream-health` issue current, where `outdated` rows for GitHub-hosted upstreams link to the `<ref>...<head>` compare view; fixes and `source.ref` bumps still go through reviewed PRs.

-Paths are checked at the installed snapshot. A path removed only at branch head is reported as `upstream-path-change` while a valid pin remains usable. `outdated` means the snapshot content differs, not that a new release version exists; commits with identical trees do not trigger it. For single-skill subtree pointers, comparison covers `source.path` and root license/notice files, so unrelated feed or website updates do not trigger repins. `--summary-file <path>` writes the workflow's `broken`, `actionable` and `unknown` decisions as JSON. Unpinned entries remain actionable, and network uncertainty keeps the tracking issue open.
+Paths are checked at the installed snapshot. A path removed only at branch head is reported as `upstream-path-change` while a valid pin remains usable. `outdated` means the installed content differs, not that a new release version exists; commits with identical trees do not trigger it. Comparison covers what an installation copies: `source.path` for a single-skill subtree pointer or the declared child paths for a collection, plus root `LICENSE*`, `NOTICE*` and `COPYING*` files. Unrelated feeds, websites, CLI sources, tests or changesets therefore do not trigger repins; undeclared upstream skills are still reported separately as `new-skill`. Root-level single skills, agents and teams compare the whole snapshot. `--summary-file <path>` writes the workflow's `broken`, `actionable` and `unknown` decisions and the `held` finding count as JSON. Unpinned entries remain actionable, and network uncertainty keeps the tracking issue open.
+
+When keeping the current pin is a reviewed decision, add `<kind>/<id>/upstream-health-hold.json` beside `entry.json`:
+
+```json
+{
+  "statuses": ["upstream-path-change", "outdated"],
+  "ref": "<40-hex pinned source.ref>",
+  "head": "<40-hex upstream commit that was reviewed>",
+  "reviewedAt": "YYYY-MM-DD",
+  "reason": "Why the current pin stays."
+}
+```
+
+Only `upstream-path-change` and `outdated` can be held; broken statuses, `unreachable`, `new-skill` and `unpinned` never can. A hold applies only while `ref` equals the entry's `source.ref` and the installed-scope content at upstream head is identical to that at `head`. Held findings are reported as `held` in a "Reviewed holds" section and are not actionable, so the tracking issue can close; the closing comment states how many held findings remain. When a hold no longer applies because the pin changed, upstream changed within the installed scope, the reviewed head no longer exists, the file is invalid, or no held finding remains, any original findings return together with an actionable `stale-hold` that asks for re-review or removal. A network error while checking the reviewed head is reported as `unreachable`, not as a stale hold. CI tests require every hold file to be well-formed and its `ref` to match the entry's `source.ref`, so bumping a pin means updating or deleting its hold. A hold records a reviewed decision; it is not a way to silence a broken entry.

 The [validation workflow](.github/workflows/i18n-validate.yml) is the source of truth for CI. It detects relevant paths and can skip catalog validation for documentation-only changes. A green skipped job is not evidence that Markdown links or bilingual documentation were checked; review those separately.

--- a/README.zh-CN.md
+++ b/README.zh-CN.md
@@ -237,9 +237,23 @@ uv run scripts/catalog/check_upstream_health.py

 集合检查需要 Git 和网络。可变来源集合会被报告并跳过，因为无法保证生成清单可复现。省略 `--check` 会进入写入模式；提交前应同时检查重新生成的子技能和对应 sidecar 事实。

-上游健康检查只读，不改写条目；出现 `gone`、`bad-branch`、`bad-ref` 或 `bad-path` 时以非零状态退出。`unreachable` 表示网络错误，不等于确认失效。[上游健康工作流](.github/workflows/upstream-health.yml)每周运行一次，并维护唯一一个 `upstream-health` Issue；修复条目和更新 `source.ref` 仍需通过经过评审的 PR。
+上游健康检查只读，不改写条目；出现 `gone`、`bad-branch`、`bad-ref` 或 `bad-path` 时以非零状态退出。`unreachable` 表示网络错误，不等于确认失效。[上游健康工作流](.github/workflows/upstream-health.yml)每周运行一次，并维护唯一一个 `upstream-health` Issue；对托管在 GitHub 的上游，Issue 中的 `outdated` 行会链接到 `<ref>...<head>` 对比页面。修复条目和更新 `source.ref` 仍需通过经过评审的 PR。

-路径检查以实际安装的快照为准。只有分支头删除路径、固定版本仍可用时，报告 `upstream-path-change`。`outdated` 表示快照内容不同，不表示一定发布了新版本；文件树完全相同的提交不会触发更新提醒。单技能子目录指针比较 `source.path` 和仓库根的许可证、声明文件，不因无关的资讯数据或网站更新反复要求改动固定提交。`--summary-file <路径>` 将工作流使用的 `broken`、`actionable` 和 `unknown` 判定写入 JSON。未固定版本仍属于待处理事项；网络状态未知时保留跟踪 Issue。
+路径检查以实际安装的快照为准。只有分支头删除路径、固定版本仍可用时，报告 `upstream-path-change`。`outdated` 表示安装内容不同，不表示一定发布了新版本；文件树完全相同的提交不会触发更新提醒。比较范围与安装时复制的内容一致：单技能子目录指针比较 `source.path`，集合比较已声明的子技能路径，两者都包含仓库根的 `LICENSE*`、`NOTICE*` 和 `COPYING*` 文件。因此，无关的资讯数据、网站、CLI 源码、测试或 changeset 更新不会反复要求改动固定提交；未声明的上游技能仍单独报告为 `new-skill`。位于仓库根目录的单技能、智能体和团队比较整个快照。`--summary-file <路径>` 将工作流使用的 `broken`、`actionable` 和 `unknown` 判定以及 `held` 发现数量写入 JSON。未固定版本仍属于待处理事项；网络状态未知时保留跟踪 Issue。
+
+如果保留当前固定版本是经过审阅的决定，在 `entry.json` 旁添加 `<kind>/<id>/upstream-health-hold.json`：
+
+```json
+{
+  "statuses": ["upstream-path-change", "outdated"],
+  "ref": "<40 位十六进制的固定 source.ref>",
+  "head": "<40 位十六进制的已审阅上游提交>",
+  "reviewedAt": "YYYY-MM-DD",
+  "reason": "保留当前固定版本的原因。"
+}
+```
+
+只有 `upstream-path-change` 和 `outdated` 可以暂缓；失效状态、`unreachable`、`new-skill` 和 `unpinned` 一律不能暂缓。暂缓记录仅在 `ref` 等于条目的 `source.ref`，且上游分支头与 `head` 在安装范围内的内容完全相同时生效。被暂缓的发现报告为 `held`，列入“Reviewed holds”区块，不属于待处理事项，因此跟踪 Issue 可以关闭；关闭评论会说明仍有多少条被暂缓的发现。如果固定版本已变更、上游在安装范围内有新改动、已审阅的提交已不存在、文件无效，或已不存在被暂缓的发现，暂缓记录即失效：原有发现（如有）重新出现，并附带一条待处理的 `stale-hold`，要求重新审阅或删除该记录。检查已审阅提交时遇到网络错误会报告为 `unreachable`，不视为暂缓失效。CI 测试要求每个暂缓文件格式正确，且其 `ref` 与条目的 `source.ref` 一致，因此更新固定版本时必须同步更新或删除对应的暂缓文件。暂缓记录用于留存经过审阅的决定，不能用来掩盖已失效的条目。

 [校验工作流](.github/workflows/i18n-validate.yml)是 CI 行为的权威来源。它会检测相关路径，纯文档改动可能跳过目录校验。跳过后显示成功的任务，不代表 Markdown 链接或中英文文档已被检查；这些内容仍需单独审查。

--- a/scripts/catalog/check_upstream_health.py
+++ b/scripts/catalog/check_upstream_health.py
@@ -16,23 +16,37 @@ script looks at each upstream and reports, without rewriting anything:
  bad-path    source.path, the root SKILL.md, or a declared child path is missing at
              the installed snapshot (source.ref, or branch head when unpinned)
  unreachable network error other than "not found"; treated as unknown, not broken
+  stale-hold  a reviewed hold no longer applies (invalid file, pin or installed upstream
+              content changed since review, or nothing left to hold); re-review it
  new-skill   branch head has collection children that `children` does not declare
              (same discovery rules as scripts/gen-collection-children.py)
  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
+  held        upstream-path-change / outdated finding covered by a reviewed hold
  ok          nothing to report

 Path checks distinguish the installed snapshot from changes at branch head.
 Only skills get path checks; agents and teams are AgentFS repositories.

+Maintainers who review an upstream change and deliberately keep the current pin
+record that decision next to entry.json in `upstream-health-hold.json`:
+
+  {"statuses": ["upstream-path-change", "outdated"], "ref": "<source.ref>",
+   "head": "<reviewed upstream commit>", "reviewedAt": "YYYY-MM-DD", "reason": "..."}
+
+The listed findings are reported as `held` while source.ref still equals `ref`
+and the installed content at branch head is unchanged since `head`. Broken,
+unreachable, new-skill and unpinned findings can never be held.
+
 Usage:
  uv run scripts/catalog/check_upstream_health.py                  # text table
  uv run scripts/catalog/check_upstream_health.py --format markdown
  uv run scripts/catalog/check_upstream_health.py --format json
  uv run scripts/catalog/check_upstream_health.py dingtalk-cli humanlayer-skills

-Exit code: 1 when any entry is gone / bad-branch / bad-ref / bad-path, else 0.
+Exit code: 1 when any entry is gone / bad-branch / bad-ref / bad-path, else 0
+(held and stale-hold findings alone never fail the run).
 Needs Git and network access.
 """

@@ -49,14 +63,22 @@ import tempfile
 import urllib.error
 import urllib.request
 from concurrent.futures import ThreadPoolExecutor
+from datetime import date
 from pathlib import Path

 REPO_ROOT = Path(__file__).resolve().parents[2]
 KINDS = ("skills", "agents", "teams")

 BROKEN = ("gone", "bad-branch", "bad-ref", "bad-path")
-ACTIONABLE = (*BROKEN, "upstream-path-change", "new-skill", "outdated", "unpinned")
-ORDER = (*BROKEN, "unreachable", "upstream-path-change", "new-skill", "outdated", "unpinned", "ok")
+ACTIONABLE = (*BROKEN, "stale-hold", "upstream-path-change", "new-skill", "outdated", "unpinned")
+ORDER = (*BROKEN, "unreachable", "stale-hold", "upstream-path-change", "new-skill", "outdated", "unpinned",
+         "held", "ok")
+
+# A reviewed decision to keep the current pin, next to entry.json.
+HOLD_NAME = "upstream-health-hold.json"
+HOLD_KEYS = ("statuses", "ref", "head", "reviewedAt", "reason")
+HOLDABLE = ("upstream-path-change", "outdated")
+SHA = re.compile(r"[0-9a-f]{40}")

 # Git asks for credentials when a public repository disappears; never prompt.
 GIT_ENV = {**os.environ, "GIT_TERMINAL_PROMPT": "0", "GCM_INTERACTIVE": "never"}
@@ -65,6 +87,8 @@ NOT_FOUND = re.compile(
    r"not found|does not exist|does not appear to be a git repository",
    re.IGNORECASE,
 )
+# A fetch-by-SHA failure that means the commit is gone, not a network problem.
+MISSING_COMMIT = re.compile(r"not our ref|couldn't find remote ref|unadvertised object|not found", re.IGNORECASE)


 def _load_generator():
@@ -94,7 +118,27 @@ def last_line(text: str) -> str:
    return lines[-1][:200] if lines else "no output"


-def check_git(entry: dict, kind: str) -> list[tuple[str, str]]:
+def comparison_scope(entry: dict, kind: str) -> list[str]:
+    """Pathspec for snapshot comparisons: only what an installation copies.
+
+    A single-skill pointer installs only source.path, and a collection installs
+    only its declared children, so upstream feeds, site builds, CLI sources and
+    unrelated skills must not cause endless repins. Root licensing changes still
+    need review. Undeclared upstream children are reported separately as
+    `new-skill`. Agents, teams and root-level skills install the whole snapshot.
+    """
+    if kind != "skills":
+        return []
+    children = entry.get("children") or []
+    path = (entry["source"].get("path") or "").strip("/")
+    paths = sorted({c["path"].strip("/") for c in children}) if children else ([path] if path else [])
+    if not paths:
+        return []
+    return ["--", *paths, ":(top,glob)LICENSE*", ":(top,glob)NOTICE*", ":(top,glob)COPYING*"]
+
+
+def check_git(entry: dict, kind: str, hold: dict | None = None) -> tuple[list[tuple[str, str]], str | None]:
+    """Return the findings and the upstream commit they were checked against."""
    source = entry["source"]
    url = source["repoUrl"]
    branch = source.get("repoBranch")
@@ -106,13 +150,13 @@ def check_git(entry: dict, kind: str) -> list[tuple[str, str]]:
    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)]
+        return [("gone" if NOT_FOUND.search(remote.stderr) else "unreachable", detail)], None
    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")]
+        return [("bad-branch", f"branch '{branch}' not found upstream")], None
    if not heads:
-        return [("unreachable", "remote advertised no HEAD")]
+        return [("unreachable", "remote advertised no HEAD")], None
    head = heads[0]

    with tempfile.TemporaryDirectory(prefix=f"health-{entry['id']}-") as tmp:
@@ -122,22 +166,20 @@ def check_git(entry: dict, kind: str) -> list[tuple[str, str]]:
            clone += ["--single-branch", "-b", branch]
        cloned = git(*clone, url, str(repo), timeout=300)
        if cloned.returncode != 0:
-            return [("unreachable", f"clone failed: {last_line(cloned.stderr)}")]
+            return [("unreachable", f"clone failed: {last_line(cloned.stderr)}")], head

        # 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)}")]
+            return [("unreachable", f"cannot resolve clone: {last_line(cloned_head.stderr)}")], head
        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:
-                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)}")]
+                status = "bad-ref" if MISSING_COMMIT.search(fetched.stderr) else "unreachable"
+                return [(status, f"cannot fetch pinned {ref[:12]}: {last_line(fetched.stderr)}")], head

        try:
            if kind == "skills":
@@ -150,24 +192,21 @@ def check_git(entry: dict, kind: str) -> list[tuple[str, str]]:
                else:
                    issues += check_skill_paths(entry, repo, head)
        except RuntimeError as err:
-            return [("unreachable", str(err)[:300])]
+            return [("unreachable", str(err)[:300])], head

        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)
+            difference = git("diff", "--quiet", ref, head, *comparison_scope(entry, kind), 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 hold:
+            issues = apply_hold(issues, hold, entry, kind, repo, head)
+
    if not ref:
        issues.append(("unpinned", f"follows '{branch or 'HEAD'}' at {head[:7]}"))
-    return issues
+    return issues, head


 def check_skill_paths(entry: dict, repo: Path, revision: str = "HEAD", *,
@@ -197,6 +236,40 @@ def check_skill_paths(entry: dict, repo: Path, revision: str = "HEAD", *,
    return issues


+def apply_hold(issues: list[tuple[str, str]], hold: dict, entry: dict, kind: str, repo: Path,
+               head: str) -> list[tuple[str, str]]:
+    """Report reviewed findings as `held` while the review still covers them.
+
+    A hold covers one pinned ref and the installed scope at the reviewed
+    upstream head. Once either moves, or nothing it lists is found any more,
+    the findings are kept as they are and a `stale-hold` explains why.
+    """
+    if any(status == "unreachable" for status, _ in issues):
+        return issues  # unknown state: the review can be neither applied nor judged stale
+    ref, reviewed = entry["source"].get("ref"), hold["head"]
+    if ref != hold["ref"]:
+        pinned = ref[:7] if ref else "unpinned"
+        return [*issues, ("stale-hold", f"pinned ref changed since review ({hold['ref'][:7]} -> {pinned})")]
+    if reviewed not in (ref, head):
+        fetched = git("fetch", "--quiet", "--depth", "1", "origin", reviewed, cwd=repo, timeout=180)
+        if fetched.returncode != 0:
+            status = "stale-hold" if MISSING_COMMIT.search(fetched.stderr) else "unreachable"
+            return [*issues, (status, f"reviewed head {reviewed[:7]} cannot be fetched: "
+                                      f"{last_line(fetched.stderr)}")]
+    difference = git("diff", "--quiet", reviewed, head, *comparison_scope(entry, kind), cwd=repo)
+    if difference.returncode == 1:
+        return [*issues, ("stale-hold", f"upstream changed within the installed scope since {reviewed[:7]}")]
+    if difference.returncode != 0:
+        return [*issues, ("unreachable", f"cannot compare with reviewed head {reviewed[:7]}: "
+                                         f"{last_line(difference.stderr)}")]
+    note = f" — reviewed {hold['reviewedAt']}: {hold['reason']}"
+    held = [("held", f"{status}: {detail}{note}") if status in hold["statuses"] else (status, detail)
+            for status, detail in issues]
+    if not any(status == "held" for status, _ in held):
+        held.append(("stale-hold", f"no held finding remains ({', '.join(hold['statuses'])}); remove {HOLD_NAME}"))
+    return held
+
+
 def check_http(entry: dict) -> list[tuple[str, str]]:
    url = entry["source"].get("url") or entry["source"].get("repoUrl")
    request = urllib.request.Request(url, headers={"User-Agent": "desirecore-market-health", "Range": "bytes=0-0"})
@@ -211,15 +284,76 @@ def check_http(entry: dict) -> list[tuple[str, str]]:
        return [("unreachable", str(err)[:200])]


+def hold_errors(data: object) -> list[str]:
+    """Explain why a reviewed hold is unusable; an empty list means it is valid."""
+    if not isinstance(data, dict):
+        return ["must be a JSON object"]
+    errors = []
+    missing = [key for key in HOLD_KEYS if key not in data]
+    unknown = sorted(set(data) - set(HOLD_KEYS))
+    if missing:
+        errors.append(f"missing {', '.join(missing)}")
+    if unknown:
+        errors.append(f"unknown key(s) {', '.join(unknown)}")
+    if "statuses" in data:
+        statuses = data["statuses"]
+        if not isinstance(statuses, list) or not statuses:
+            errors.append("statuses must be a non-empty list")
+        elif any(status not in HOLDABLE for status in statuses):
+            errors.append(f"statuses may only list {' / '.join(HOLDABLE)}")
+        elif len(set(statuses)) != len(statuses):
+            errors.append("statuses must be unique")
+    for key in ("ref", "head"):
+        if key in data and not (isinstance(data[key], str) and SHA.fullmatch(data[key])):
+            errors.append(f"{key} must be a full lowercase 40-hex commit SHA")
+    if "reviewedAt" in data:
+        try:
+            reviewed = date.fromisoformat(data["reviewedAt"]).isoformat() == data["reviewedAt"]
+        except (TypeError, ValueError):
+            reviewed = False
+        if not reviewed:
+            errors.append("reviewedAt must be a YYYY-MM-DD date")
+    if "reason" in data and not (isinstance(data["reason"], str) and data["reason"].strip()):
+        errors.append("reason must be non-empty text")
+    return errors
+
+
+def load_hold(entry_path: Path) -> tuple[dict | None, list[str]]:
+    """Read the reviewed hold next to entry.json.
+
+    Returns (None, []) without a hold file, (hold, []) for a valid one and
+    (None, errors) when a hold file exists but cannot be used.
+    """
+    path = entry_path.with_name(HOLD_NAME)
+    if not path.exists():
+        return None, []
+    try:
+        data = json.loads(path.read_text(encoding="utf-8"))
+    except (OSError, ValueError) as err:
+        return None, [f"unreadable JSON: {err}"]
+    errors = hold_errors(data)
+    return (None, errors) if errors else (data, [])
+
+
 def check_entry(path: Path) -> dict:
    entry = json.loads(path.read_text(encoding="utf-8"))
    kind = path.parent.parent.name
    source = entry.get("source", {})
-    issues = check_git(entry, kind) if source.get("kind") == "git" else check_http(entry)
+    hold, hold_problems = load_hold(path)
+    if source.get("kind") == "git":
+        issues, head = check_git(entry, kind, hold)
+    else:
+        issues, head = check_http(entry), None
+    if hold_problems:
+        issues = [*issues, ("stale-hold", f"invalid {HOLD_NAME}: {'; '.join(hold_problems)}")]
+    elif hold and source.get("kind") != "git":
+        issues = [*issues, ("stale-hold", "holds apply only to git entries")]
    return {
        "id": entry["id"],
        "kind": kind.rstrip("s"),
        "url": source.get("repoUrl") or source.get("url"),
+        "ref": source.get("ref"),
+        "head": head,
        "issues": [{"status": s, "detail": d} for s, d in (issues or [("ok", "")])],
    }

@@ -228,6 +362,18 @@ def worst(result: dict) -> int:
    return min(ORDER.index(i["status"]) for i in result["issues"])


+# Every git pointer in the catalog is hosted on GitHub.
+GITHUB_REPO = re.compile(r"https://github\.com/([^/]+)/([^/]+?)(?:\.git)?/?")
+
+
+def compare_url(url: str | None, ref: str | None, head: str | None) -> str | None:
+    """Upstream page showing what changed between the pinned ref and the checked head."""
+    match = GITHUB_REPO.fullmatch(url or "")
+    if not (match and ref and head):
+        return None
+    return f"https://github.com/{match[1]}/{match[2]}/compare/{ref}...{head}"
+
+
 def render_text(results: list[dict]) -> str:
    rows = []
    for r in sorted(results, key=lambda r: (worst(r), r["id"])):
@@ -245,10 +391,12 @@ def render_markdown(results: list[dict]) -> str:
    sections = [
        ("Broken — installation fails", BROKEN),
        ("Unreachable — network error, status unknown", ("unreachable",)),
+        ("Stale holds — re-review", ("stale-hold",)),
        ("Upstream path changes — pinned installation still resolves", ("upstream-path-change",)),
        ("Undeclared upstream skills", ("new-skill",)),
        ("Updates available", ("outdated",)),
        ("Unpinned", ("unpinned",)),
+        ("Reviewed holds — not actionable", ("held",)),
    ]
    for title, statuses in sections:
        rows = [(r, i) for r in results for i in r["issues"] if i["status"] in statuses]
@@ -257,6 +405,9 @@ def render_markdown(results: list[dict]) -> str:
        out += [f"### {title} ({len(rows)})", "", "| Status | Entry | Detail |", "|---|---|---|"]
        for r, i in sorted(rows, key=lambda x: (ORDER.index(x[1]["status"]), x[0]["id"])):
            detail = i["detail"].replace("|", "\\|")
+            compare = compare_url(r["url"], r.get("ref"), r.get("head"))
+            if i["status"] == "outdated" and compare:
+                detail = f"[{r['ref'][:7]} → {r['head'][:7]}]({compare})"
            out.append(f"| `{i['status']}` | [{r['kind']}/{r['id']}]({r['url']}) | {detail} |")
        out.append("")
    ok = sorted(r["id"] for r in results if worst(r) == ORDER.index("ok"))
@@ -272,6 +423,7 @@ def summarize(results: list[dict]) -> dict:
        "broken": bool(statuses.intersection(BROKEN)),
        "actionable": bool(statuses.intersection(ACTIONABLE)),
        "unknown": "unreachable" in statuses,
+        "held": sum(issue["status"] == "held" for result in results for issue in result["issues"]),
    }


--- a/scripts/catalog/test_check_upstream_health.py
+++ b/scripts/catalog/test_check_upstream_health.py
@@ -157,12 +157,199 @@ class UpstreamHealthTests(unittest.TestCase):
            return [{"id": "a", "issues": [{"status": status, "detail": ""}]}]

        self.assertEqual(CHECKER.summarize(results("unpinned")),
-                         {"checked": 1, "broken": False, "actionable": True, "unknown": False})
+                         {"checked": 1, "broken": False, "actionable": True, "unknown": False, "held": 0})
        self.assertEqual(CHECKER.summarize(results("upstream-path-change"))["actionable"], True)
        self.assertEqual(CHECKER.summarize(results("unreachable")),
-                         {"checked": 1, "broken": False, "actionable": False, "unknown": True})
+                         {"checked": 1, "broken": False, "actionable": False, "unknown": True, "held": 0})
        self.assertEqual(CHECKER.summarize(results("ok")),
-                         {"checked": 1, "broken": False, "actionable": False, "unknown": False})
+                         {"checked": 1, "broken": False, "actionable": False, "unknown": False, "held": 0})
+
+    def hold(self, entry_path: Path, **fields) -> Path:
+        head = sh("git", "rev-parse", "HEAD", cwd=self.upstream)
+        data = {"statuses": ["outdated"], "ref": self.first, "head": head,
+                "reviewedAt": "2026-10-01", "reason": "New layout cannot be installed as a subtree."}
+        path = entry_path.parent / "upstream-health-hold.json"
+        path.write_text(json.dumps({**data, **fields}), encoding="utf-8")
+        return path
+
+    def test_reviewed_hold_replaces_outdated_while_head_is_unchanged(self) -> None:
+        head = self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry)
+        reviewed = "reviewed 2026-10-01: New layout cannot be installed as a subtree."
+        self.assertEqual(self.statuses(entry), {"held": f"outdated: {self.first[:7]} -> {head[:7]} — {reviewed}"})
+
+    def test_hold_survives_upstream_changes_outside_the_installed_scope(self) -> None:
+        reviewed = self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry, head=reviewed)
+        self.commit({"README.md": "New website.\n"})
+        self.assertEqual(set(self.statuses(entry)), {"held"})
+
+    def test_in_scope_change_after_review_reraises_outdated(self) -> None:
+        reviewed = self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry, head=reviewed)
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated again.\n---\n"})
+        result = self.statuses(entry)
+        self.assertIn("outdated", result)
+        self.assertNotIn("held", result)
+        self.assertIn(f"since {reviewed[:7]}", result["stale-hold"])
+
+    def test_unfetchable_reviewed_head_is_a_stale_hold(self) -> None:
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry, head="f" * 40)
+        result = self.statuses(entry)
+        self.assertIn("outdated", result)
+        self.assertIn("cannot be fetched", result["stale-hold"])
+
+    def test_network_error_on_reviewed_head_is_unknown_not_stale(self) -> None:
+        head = self.commit({"README.md": "Unrelated.\n"})
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry, head=head)
+        real_git = CHECKER.git
+
+        def failing_fetch(*args, **kwargs):
+            if args[0] == "fetch" and head in args:
+                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(entry)
+        self.assertIn("outdated", result)
+        self.assertIn("unreachable", result)
+        self.assertNotIn("stale-hold", result)
+
+    def test_unknown_comparison_neither_applies_nor_stales_the_hold(self) -> None:
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry)
+        real_git = CHECKER.git
+
+        def failing_diff(*args, **kwargs):
+            if args[0] == "diff":
+                return subprocess.CompletedProcess(args, 128, "", "fatal: unable to read tree")
+            return real_git(*args, **kwargs)
+
+        with patch.object(CHECKER, "git", side_effect=failing_diff):
+            self.assertEqual(set(self.statuses(entry)), {"unreachable"})
+
+    def test_changed_pin_makes_the_hold_stale(self) -> None:
+        second = self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated again.\n---\n"})
+        entry = self.entry(ref=second, path="skills/alpha")
+        self.hold(entry)
+        result = self.statuses(entry)
+        self.assertIn("outdated", result)
+        self.assertIn("pinned ref changed", result["stale-hold"])
+
+    def test_hold_without_a_remaining_finding_is_stale(self) -> None:
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry)
+        result = self.statuses(entry)
+        self.assertEqual(set(result), {"stale-hold"})
+        self.assertIn("no held finding remains", result["stale-hold"])
+
+    def test_hold_covers_upstream_path_change_and_reports_unlisted_findings(self) -> None:
+        (self.upstream / "skills/alpha/SKILL.md").unlink()
+        self.commit({"skills/beta/SKILL.md": "---\nname: beta\ndescription: B.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        self.hold(entry, statuses=["upstream-path-change"])
+        result = self.statuses(entry)
+        self.assertTrue(result["held"].startswith("upstream-path-change: source.path 'skills/alpha' has no"))
+        self.assertIn("outdated", result)
+        self.assertNotIn("upstream-path-change", result)
+        self.hold(entry, statuses=["upstream-path-change", "outdated"])
+        issues = CHECKER.check_entry(entry)["issues"]
+        self.assertEqual([i["status"] for i in issues], ["held", "held"])
+
+    def test_hold_validation_rejects_unholdable_statuses_bad_shas_and_extra_keys(self) -> None:
+        valid = {"statuses": ["upstream-path-change", "outdated"], "ref": "a" * 40, "head": "b" * 40,
+                 "reviewedAt": "2026-10-01", "reason": "Kept after review."}
+        self.assertEqual(CHECKER.hold_errors(valid), [])
+        invalid = [{"statuses": [status]} for status in
+                   (*CHECKER.BROKEN, "unreachable", "new-skill", "unpinned", "held", "stale-hold", "ok")]
+        invalid += [{"statuses": []}, {"statuses": "outdated"}, {"statuses": ["outdated", "outdated"]},
+                    {"ref": "A" * 40}, {"ref": "a" * 12}, {"head": "g" * 40}, {"head": None},
+                    {"reviewedAt": "2026-13-01"}, {"reviewedAt": "18/10/2026"}, {"reason": " "}, {"reason": 1},
+                    {"note": "extra"}]
+        for fields in invalid:
+            with self.subTest(fields=fields):
+                self.assertTrue(CHECKER.hold_errors({**valid, **fields}))
+        self.assertTrue(CHECKER.hold_errors({k: v for k, v in valid.items() if k != "reason"}))
+        self.assertTrue(CHECKER.hold_errors(["outdated"]))
+
+    def test_invalid_hold_file_is_stale_and_hides_nothing(self) -> None:
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        entry = self.entry(ref=self.first, path="skills/alpha")
+        hold = self.hold(entry, statuses=["outdated", "gone"])
+        result = self.statuses(entry)
+        self.assertIn("outdated", result)
+        self.assertIn("invalid", result["stale-hold"])
+        hold.write_text("{", encoding="utf-8")
+        self.assertIn("invalid", self.statuses(entry)["stale-hold"])
+
+    def test_hold_on_a_non_git_entry_is_stale(self) -> None:
+        entry = self.entry(kind="web", url="https://example.com/skill.zip")
+        self.hold(entry)
+        with patch.object(CHECKER, "check_http", return_value=[]):
+            result = self.statuses(entry)
+        self.assertEqual(set(result), {"stale-hold"})
+        self.assertIn("git entries", result["stale-hold"])
+
+    def test_held_alone_is_not_actionable_but_stale_hold_is(self) -> None:
+        def results(*statuses):
+            return [{"id": "a", "issues": [{"status": s, "detail": ""} for s in statuses]}]
+
+        self.assertEqual(CHECKER.summarize(results("held", "held")),
+                         {"checked": 1, "broken": False, "actionable": False, "unknown": False, "held": 2})
+        self.assertEqual(CHECKER.summarize(results("stale-hold")),
+                         {"checked": 1, "broken": False, "actionable": True, "unknown": False, "held": 0})
+
+    def test_markdown_lists_reviewed_holds_separately(self) -> None:
+        results = [
+            {"id": "a", "kind": "skill", "url": "u", "issues": [{"status": "held", "detail": "outdated: x"}]},
+            {"id": "b", "kind": "skill", "url": "u", "issues": [{"status": "stale-hold", "detail": "y"}]},
+        ]
+        text = CHECKER.render_markdown(results)
+        self.assertIn("### Reviewed holds — not actionable (1)", text)
+        self.assertLess(text.index("Stale holds — re-review"), text.index("Reviewed holds"))
+        self.assertNotIn("OK (", text)
+
+    def test_repository_hold_files_are_valid_and_match_the_pin(self) -> None:
+        for kind in CHECKER.KINDS:
+            for path in sorted((CHECKER.REPO_ROOT / kind).glob("*/upstream-health-hold.json")):
+                with self.subTest(hold=f"{kind}/{path.parent.name}"):
+                    entry_path = path.with_name("entry.json")
+                    self.assertTrue(entry_path.is_file(), "a hold belongs next to an external entry.json")
+                    hold, errors = CHECKER.load_hold(entry_path)
+                    self.assertEqual(errors, [])
+                    source = json.loads(entry_path.read_text(encoding="utf-8"))["source"]
+                    self.assertEqual(source.get("kind"), "git")
+                    self.assertEqual(source.get("ref"), hold["ref"],
+                                     "bumping source.ref must update or remove the reviewed hold")
+
+    def test_collection_ignores_changes_outside_declared_children(self) -> None:
+        self.commit({"cmd/main.go": "package main\n", "README.md": "Release notes.\n"})
+        children = [{"id": "alpha", "path": "skills/alpha"}]
+        self.assertEqual(self.statuses(self.entry(ref=self.first, children=children)), {"ok": ""})
+
+    def test_collection_child_change_is_outdated(self) -> None:
+        self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        children = [{"id": "alpha", "path": "skills/alpha"}]
+        self.assertIn("outdated", self.statuses(self.entry(ref=self.first, children=children)))
+
+    def test_root_license_change_still_needs_review_for_a_collection(self) -> None:
+        self.commit({"NOTICE": "New attribution.\n"})
+        children = [{"id": "alpha", "path": "skills/alpha"}]
+        self.assertIn("outdated", self.statuses(self.entry(ref=self.first, children=children)))
+
+    def test_result_records_pinned_ref_and_checked_head(self) -> None:
+        head = self.commit({"skills/alpha/SKILL.md": "---\nname: alpha\ndescription: Updated.\n---\n"})
+        result = CHECKER.check_entry(self.entry(ref=self.first, path="skills/alpha"))
+        self.assertEqual((result["ref"], result["head"]), (self.first, head))

    def test_collection_reports_missing_and_new_children(self) -> None:
        head = self.commit({"skills/beta/SKILL.md": "---\nname: beta\ndescription: B.\n---\n"})
@@ -179,6 +366,43 @@ class UpstreamHealthTests(unittest.TestCase):
        text = CHECKER.render_markdown(results)
        self.assertLess(text.index("Broken"), text.index("Updates available"))

+    def test_markdown_links_outdated_entry_to_upstream_compare(self) -> None:
+        ref, head = "a19a171" + "0" * 33, "94f06c1" + "0" * 33
+        results = [{"id": "a", "kind": "skill", "url": "https://github.com/o/r.git", "ref": ref, "head": head,
+                    "issues": [{"status": "outdated", "detail": "a19a171 -> 94f06c1"}]}]
+        self.assertIn(f"| [a19a171 → 94f06c1](https://github.com/o/r/compare/{ref}...{head}) |",
+                      CHECKER.render_markdown(results))
+
+    def test_compare_url_accepts_github_url_variants(self) -> None:
+        ref, head = "a" * 40, "b" * 40
+        for url in ("https://github.com/o/r.git", "https://github.com/o/r", "https://github.com/o/r/",
+                    "https://github.com/o/r.git/"):
+            with self.subTest(url=url):
+                self.assertEqual(CHECKER.compare_url(url, ref, head),
+                                 f"https://github.com/o/r/compare/{ref}...{head}")
+
+    def test_markdown_keeps_plain_detail_without_compare_url(self) -> None:
+        ref, head = "a" * 40, "b" * 40
+        for url, pinned, checked in (("https://git.example.com/o/r.git", ref, head),
+                                     ("https://github.com/o/r.git", None, head),
+                                     ("https://github.com/o/r.git", ref, None),
+                                     (None, ref, head)):
+            with self.subTest(url=url, ref=pinned, head=checked):
+                self.assertIsNone(CHECKER.compare_url(url, pinned, checked))
+                results = [{"id": "a", "kind": "skill", "url": url, "ref": pinned, "head": checked,
+                            "issues": [{"status": "outdated", "detail": "aaaaaaa -> bbbbbbb"}]}]
+                text = CHECKER.render_markdown(results)
+                self.assertIn("| aaaaaaa -> bbbbbbb |", text)
+                self.assertNotIn("/compare/", text)
+
+    def test_markdown_links_only_outdated_rows(self) -> None:
+        results = [{"id": "a", "kind": "skill", "url": "https://github.com/o/r.git", "ref": "a" * 40,
+                    "head": "b" * 40, "issues": [{"status": "outdated", "detail": "aaaaaaa -> bbbbbbb"},
+                                                 {"status": "upstream-path-change", "detail": "x | y"}]}]
+        text = CHECKER.render_markdown(results)
+        self.assertEqual(text.count("/compare/"), 1)
+        self.assertIn("| x \\| y |", text)
+

 if __name__ == "__main__":
    unittest.main()