fix(ci):resolve the skill_review errors (#5121)

* fix(ci):resolve the skill_review errors

* fix(ci): split skill creator fixes from waiver rollout

.github/skill-review-waivers.v1.json

@@ -0,0 +1,27 @@
+{
+  "schema_version": "deerflow.skill-review-waivers.v1",
+  "waivers": [
+    {
+      "package": "skills/public/skill-creator",
+      "source": "skillscan",
+      "rule_id": "python-subprocess",
+      "path": "scripts/improve_description.py",
+      "line": 35,
+      "evidence": "subprocess.run",
+      "file_sha256": "sha256:87d864570220b699fac52da309d2d6efdb060647bfebc74f768128e646accf80",
+      "reason": "Required Claude CLI invocation uses a fixed executable, an argv list, prompt input over stdin, and shell=False.",
+      "expires_on": "2027-02-28"
+    },
+    {
+      "package": "skills/public/skill-creator",
+      "source": "skillscan",
+      "rule_id": "python-subprocess",
+      "path": "scripts/run_eval.py",
+      "line": 85,
+      "evidence": "subprocess.Popen",
+      "file_sha256": "sha256:43e3b8f80dbf69c343967ba77e268fae991d9fa3ed68b32a0ff02532cd48657f",
+      "reason": "Required Claude CLI invocation uses a fixed executable, an argv list, captured output streams, and shell=False.",
+      "expires_on": "2027-02-28"
+    }
+  ]
+}

.github/workflows/skill-review-ci.yml

@@ -8,6 +8,8 @@ on:
       - "backend/packages/harness/deerflow/skills/review/**"
       - "contracts/skill_review/**"
       - "scripts/review_changed_public_skills.py"
+      - "scripts/skill_review_waivers.py"
+      - ".github/skill-review-waivers.v1.json"
       - "backend/pyproject.toml"
       - "backend/uv.lock"
       - ".github/workflows/skill-review-ci.yml"
@@ -18,6 +20,8 @@ on:
       - "backend/packages/harness/deerflow/skills/review/**"
       - "contracts/skill_review/**"
       - "scripts/review_changed_public_skills.py"
+      - "scripts/skill_review_waivers.py"
+      - ".github/skill-review-waivers.v1.json"
       - "backend/pyproject.toml"
       - "backend/uv.lock"
       - ".github/workflows/skill-review-ci.yml"

AGENTS.md

@@ -100,6 +100,14 @@ Skill quality review note:
   tag-neutralized; full raw payloads stay in tool artifacts. See
   [backend/AGENTS.md](backend/AGENTS.md) for the non-activation, SkillScan, and
   `skill-creator` ownership boundaries.
+- CI waivers live in `.github/skill-review-waivers.v1.json` and are enforced by
+  `scripts/review_changed_public_skills.py`. Pull requests may validate waiver
+  edits from their head revision, but only the manifest from the trusted base
+  revision can suppress that run. Entries match one error finding exactly,
+  include the reviewed file's SHA-256 and an expiry date, remain visible in CI
+  output, and can never waive blocker findings. Adding a waiver and relying on
+  it therefore requires two steps: merge the reviewed waiver first, then update
+  the affected public skill in a later pull request.
 
 Scheduled-task note:
 - The scheduled-task MVP adds a workspace page at `/workspace/scheduled-tasks` plus a background scheduler service gated by `config.yaml -> scheduler.enabled`.

backend/tests/test_review_changed_public_skills.py

@@ -3,7 +3,9 @@ from __future__ import annotations
 import subprocess
 from pathlib import Path, PurePosixPath
 
+import pytest
 import review_changed_public_skills as runner
+from skill_review_waivers import EMPTY_MANIFEST, WaiverManifest
 
 
 def _completed(command: list[str], *, stdout: bytes = b"", returncode: int = 0) -> subprocess.CompletedProcess[bytes]:
@@ -17,6 +19,11 @@ def _write_skill(repo_root: Path, package: str) -> Path:
     return skill_md
 
 
+@pytest.fixture(autouse=True)
+def _empty_waiver_manifests(monkeypatch) -> None:
+    monkeypatch.setattr(runner, "load_waiver_manifests", lambda args, repo_root: (EMPTY_MANIFEST, EMPTY_MANIFEST))
+
+
 def test_main_skips_successfully_when_no_public_skill_changed(tmp_path: Path, monkeypatch, capsys) -> None:
     def fake_run(command, **kwargs):
         assert command == [
@@ -83,7 +90,7 @@ def test_main_reviews_changed_public_skill_and_skips_deleted_skill_md(
         assert command[:3] == ["git", "diff", "--name-status"]
         return _completed(command, stdout=diff_output)
 
-    def fake_review(package: Path, repo_root: Path, python_executable: str) -> int:
+    def fake_review(package: Path, repo_root: Path, python_executable: str, manifest: WaiverManifest) -> int:
         assert repo_root == tmp_path
         assert python_executable
         reviewed.append(package.relative_to(repo_root).as_posix())
@@ -178,7 +185,7 @@ def test_main_reviews_package_when_skill_md_deleted_but_sibling_file_remains(
     def fake_git_diff(command, **kwargs):
         return _completed(command, stdout=diff_output)
 
-    def fake_review(package: Path, repo_root: Path, python_executable: str) -> int:
+    def fake_review(package: Path, repo_root: Path, python_executable: str, manifest: WaiverManifest) -> int:
         reviewed.append(package.relative_to(repo_root).as_posix())
         return 1
 
@@ -216,7 +223,7 @@ def test_main_reviews_package_when_only_support_file_changed(
         assert command[-1] == runner.PUBLIC_SKILL_PACKAGE_PATHSPEC
         return _completed(command, stdout=diff_output)
 
-    def fake_review(package: Path, repo_root: Path, python_executable: str) -> int:
+    def fake_review(package: Path, repo_root: Path, python_executable: str, manifest: WaiverManifest) -> int:
         reviewed.append(package.relative_to(repo_root).as_posix())
         return 0
 
@@ -252,7 +259,7 @@ def test_main_maps_eval_fixture_changes_to_owner_package(
     def fake_git_diff(command, **kwargs):
         return _completed(command, stdout=diff_output)
 
-    def fake_review(package: Path, repo_root: Path, python_executable: str) -> int:
+    def fake_review(package: Path, repo_root: Path, python_executable: str, manifest: WaiverManifest) -> int:
         reviewed.append(package.relative_to(repo_root).as_posix())
         return 0
 
@@ -290,15 +297,16 @@ def test_main_exits_nonzero_when_review_cli_reports_error(tmp_path: Path, monkey
             "deerflow.skills.review.cli",
             "skills/public/bad",
             "--format",
-            "text",
+            "json",
             "--fail-on",
-            "error",
-            "--fail-on-incomplete",
+            "never",
         ]
         assert kwargs["cwd"] == tmp_path
         assert "backend/packages/harness" in kwargs["env"]["PYTHONPATH"]
+        assert kwargs["capture_output"] is True
+        assert kwargs["text"] is True
         assert kwargs["check"] is False
-        return _completed(command, returncode=1)
+        return subprocess.CompletedProcess(command, 1, stdout="", stderr="analyzer error\n")
 
     monkeypatch.setattr(runner.subprocess, "run", fake_run)
 
@@ -315,11 +323,12 @@ def test_main_exits_nonzero_when_review_cli_reports_error(tmp_path: Path, monkey
         ]
     )
 
-    output = capsys.readouterr().out
+    captured = capsys.readouterr()
     assert exit_code == 1
     assert [call[0] for call in calls] == ["git", "test-python"]
-    assert "Failed: skills/public/bad (exit 1)" in output
-    assert "One or more skill reviews failed." in output
+    assert "Failed: skills/public/bad" in captured.out
+    assert "One or more skill reviews failed." in captured.out
+    assert "Analyzer failed for skills/public/bad (exit 1)." in captured.err
 
 
 def test_main_falls_back_to_empty_tree_when_push_before_is_missing(tmp_path: Path, monkeypatch, capsys) -> None:
@@ -334,7 +343,7 @@ def test_main_falls_back_to_empty_tree_when_push_before_is_missing(tmp_path: Pat
             return subprocess.CompletedProcess(command, 128, stdout=b"", stderr=b"fatal: bad object before")
         return _completed(command, stdout=diff_output)
 
-    def fake_review(package: Path, repo_root: Path, python_executable: str) -> int:
+    def fake_review(package: Path, repo_root: Path, python_executable: str, manifest: WaiverManifest) -> int:
         reviewed.append(package.relative_to(repo_root).as_posix())
         return 0
 

backend/tests/test_skill_review_waivers.py

@@ -0,0 +1,260 @@
+from __future__ import annotations
+
+import hashlib
+import json
+from datetime import date
+from pathlib import Path
+from types import SimpleNamespace
+
+import pytest
+import review_changed_public_skills as runner
+import skill_review_waivers as waiver_support
+from jsonschema import Draft202012Validator, FormatChecker
+from skill_review_waivers import (
+    EMPTY_MANIFEST,
+    SCHEMA_VERSION,
+    SkillReviewWaiver,
+    WaiverManifest,
+    WaiverManifestError,
+    matching_waiver,
+    parse_manifest,
+    validate_manifest_against_facts,
+)
+
+from deerflow.skills.review.analyzer import analyze_skill_package
+from deerflow.skills.review.readers import LocalDirectoryReader
+
+REPO_ROOT = Path(__file__).resolve().parents[2]
+
+
+def _write_target(repo_root: Path, content: bytes = b"safe subprocess invocation\n") -> tuple[Path, str]:
+    target = repo_root / "skills/public/demo/scripts/run.py"
+    target.parent.mkdir(parents=True, exist_ok=True)
+    target.write_bytes(content)
+    return target, f"sha256:{hashlib.sha256(content).hexdigest()}"
+
+
+def _waiver(*, digest: str, expires_on: date = date(2027, 2, 28)) -> SkillReviewWaiver:
+    return SkillReviewWaiver(
+        package="skills/public/demo",
+        source="skillscan",
+        rule_id="python-subprocess",
+        path="scripts/run.py",
+        line=12,
+        evidence="subprocess.run",
+        file_sha256=digest,
+        reason="Fixed executable and argv invocation with shell disabled.",
+        expires_on=expires_on,
+    )
+
+
+def _finding(*, severity: str = "error", line: int = 12) -> dict[str, object]:
+    return {
+        "source": "skillscan",
+        "rule_id": "python-subprocess",
+        "path": "scripts/run.py",
+        "line": line,
+        "evidence": "subprocess.run",
+        "severity": severity,
+        "message": "Subprocess usage detected.",
+    }
+
+
+def _payload(*, digest: str, path: str = "scripts/run.py", duplicate: bool = False) -> bytes:
+    entry = {
+        "package": "skills/public/demo",
+        "source": "skillscan",
+        "rule_id": "python-subprocess",
+        "path": path,
+        "line": 12,
+        "evidence": "subprocess.run",
+        "file_sha256": digest,
+        "reason": "Fixed executable and argv invocation with shell disabled.",
+        "expires_on": "2027-02-28",
+    }
+    return json.dumps({"schema_version": SCHEMA_VERSION, "waivers": [entry, entry] if duplicate else [entry]}).encode()
+
+
+def test_committed_manifest_matches_schema_and_strict_parser() -> None:
+    manifest_path = REPO_ROOT / ".github/skill-review-waivers.v1.json"
+    schema = json.loads((REPO_ROOT / "contracts/skill_review/waiver_manifest.v1.schema.json").read_text(encoding="utf-8"))
+    payload = json.loads(manifest_path.read_text(encoding="utf-8"))
+
+    Draft202012Validator(schema, format_checker=FormatChecker()).validate(payload)
+    parsed = parse_manifest(manifest_path.read_bytes(), source=str(manifest_path))
+
+    assert len(parsed.waivers) == 2
+
+
+def test_skill_creator_waivers_match_current_error_findings() -> None:
+    package = REPO_ROOT / "skills/public/skill-creator"
+    manifest = parse_manifest((REPO_ROOT / ".github/skill-review-waivers.v1.json").read_bytes(), source="committed manifest")
+    facts = analyze_skill_package(LocalDirectoryReader(package).read(), profile="deerflow")
+
+    validation_errors = validate_manifest_against_facts(
+        manifest,
+        facts_by_package={"skills/public/skill-creator": facts},
+        repo_root=REPO_ROOT,
+        today=date(2026, 8, 31),
+    )
+    assert validation_errors == []
+
+
+@pytest.mark.parametrize("path", ["../run.py", "/tmp/run.py", "scripts\\run.py", "scripts/../run.py"])
+def test_parser_rejects_noncanonical_or_traversing_paths(tmp_path: Path, path: str) -> None:
+    _, digest = _write_target(tmp_path)
+
+    with pytest.raises(WaiverManifestError, match="canonical relative POSIX path|backslashes"):
+        parse_manifest(_payload(digest=digest, path=path), source="test manifest")
+
+
+def test_parser_rejects_duplicate_exact_waivers(tmp_path: Path) -> None:
+    _, digest = _write_target(tmp_path)
+
+    with pytest.raises(WaiverManifestError, match="duplicates an earlier waiver"):
+        parse_manifest(_payload(digest=digest, duplicate=True), source="test manifest")
+
+
+def test_missing_manifest_at_ref_means_no_waivers(tmp_path: Path, monkeypatch) -> None:
+    monkeypatch.setattr(
+        waiver_support.subprocess,
+        "run",
+        lambda *args, **kwargs: waiver_support.subprocess.CompletedProcess(args[0], 128, stdout=b"", stderr=b"missing"),
+    )
+
+    manifest = waiver_support.load_manifest_at_ref(tmp_path, "a" * 40, label="trusted base")
+
+    assert manifest is EMPTY_MANIFEST
+
+
+def test_matching_waiver_requires_exact_finding_and_current_file_hash(tmp_path: Path) -> None:
+    target, digest = _write_target(tmp_path)
+    manifest = WaiverManifest((_waiver(digest=digest),))
+
+    assert matching_waiver(_finding(), package="skills/public/demo", manifest=manifest, repo_root=tmp_path, today=date(2026, 8, 31)) is manifest.waivers[0]
+    assert matching_waiver(_finding(line=13), package="skills/public/demo", manifest=manifest, repo_root=tmp_path, today=date(2026, 8, 31)) is None
+
+    target.write_bytes(b"changed\n")
+    assert matching_waiver(_finding(), package="skills/public/demo", manifest=manifest, repo_root=tmp_path, today=date(2026, 8, 31)) is None
+
+
+def test_matching_waiver_rejects_symlinked_package_outside_repository(tmp_path: Path) -> None:
+    external_root = tmp_path.parent / f"{tmp_path.name}-external"
+    external_target, digest = _write_target(external_root)
+    public_root = tmp_path / "skills/public"
+    public_root.mkdir(parents=True)
+    (public_root / "demo").symlink_to(external_target.parents[1], target_is_directory=True)
+
+    assert (
+        matching_waiver(
+            _finding(),
+            package="skills/public/demo",
+            manifest=WaiverManifest((_waiver(digest=digest),)),
+            repo_root=tmp_path,
+            today=date(2026, 8, 31),
+        )
+        is None
+    )
+
+
+def test_expired_waiver_is_rejected_and_does_not_match(tmp_path: Path) -> None:
+    _, digest = _write_target(tmp_path)
+    waiver = _waiver(digest=digest, expires_on=date(2026, 8, 30))
+    manifest = WaiverManifest((waiver,))
+    facts = {"findings": [_finding()]}
+
+    errors = validate_manifest_against_facts(
+        manifest,
+        facts_by_package={waiver.package: facts},
+        repo_root=tmp_path,
+        today=date(2026, 8, 31),
+    )
+
+    assert len(errors) == 1
+    assert "expired on 2026-08-30" in errors[0]
+    assert matching_waiver(_finding(), package=waiver.package, manifest=manifest, repo_root=tmp_path, today=date(2026, 8, 31)) is None
+
+
+def test_manifest_validation_refuses_blocker_waiver(tmp_path: Path) -> None:
+    _, digest = _write_target(tmp_path)
+    waiver = _waiver(digest=digest)
+
+    errors = validate_manifest_against_facts(
+        WaiverManifest((waiver,)),
+        facts_by_package={waiver.package: {"findings": [_finding(severity="blocker")]}},
+        repo_root=tmp_path,
+        today=date(2026, 8, 31),
+    )
+
+    assert "blockers can never be waived" in errors[0]
+
+
+def test_pr_head_manifest_is_validated_but_cannot_self_apply(tmp_path: Path, monkeypatch) -> None:
+    _, digest = _write_target(tmp_path)
+    proposed = WaiverManifest((_waiver(digest=digest),))
+    loaded: list[tuple[str, str]] = []
+
+    def fake_load(repo_root: Path, ref: str, *, label: str) -> WaiverManifest:
+        loaded.append((ref, label))
+        return EMPTY_MANIFEST if label == "trusted base" else proposed
+
+    monkeypatch.setattr(runner, "load_manifest_at_ref", fake_load)
+    args = SimpleNamespace(base_ref="base-sha", head_ref="head-sha", before=None, after=None)
+
+    effective, head = runner.load_waiver_manifests(args, tmp_path)
+
+    assert effective is EMPTY_MANIFEST
+    assert head is proposed
+    assert loaded == [("base-sha", "trusted base"), ("head-sha", "proposed head")]
+
+
+def test_main_fails_closed_on_malformed_head_manifest(tmp_path: Path, monkeypatch, capsys) -> None:
+    def fail_load(args, repo_root):
+        raise WaiverManifestError("proposed head: invalid JSON")
+
+    monkeypatch.setattr(runner, "load_waiver_manifests", fail_load)
+    monkeypatch.setattr(runner.subprocess, "run", lambda *args, **kwargs: pytest.fail("diff must not run"))
+
+    exit_code = runner.main(["--base-ref", "base", "--head-ref", "head", "--repo-root", str(tmp_path)])
+
+    assert exit_code == 1
+    assert "Invalid waiver manifest" in capsys.readouterr().err
+
+
+def test_workflow_triggers_on_waiver_implementation_and_manifest() -> None:
+    workflow = (REPO_ROOT / ".github/workflows/skill-review-ci.yml").read_text(encoding="utf-8")
+
+    assert workflow.count('"scripts/skill_review_waivers.py"') == 2
+    assert workflow.count('".github/skill-review-waivers.v1.json"') == 2
+
+
+def test_run_review_keeps_waived_error_visible_and_passes(tmp_path: Path, monkeypatch, capsys) -> None:
+    package = tmp_path / "skills/public/demo"
+    _, digest = _write_target(tmp_path)
+    facts = {
+        "summary": {"blockers": 0, "errors": 1, "warnings": 0, "infos": 0},
+        "completeness": {"not_assessed": []},
+        "findings": [_finding()],
+    }
+    monkeypatch.setattr(runner, "collect_review_facts", lambda *args: facts)
+
+    exit_code = runner.run_review(package, tmp_path, "python", WaiverManifest((_waiver(digest=digest),)))
+
+    output = capsys.readouterr().out
+    assert exit_code == 0
+    assert "- error python-subprocess" in output
+    assert "[WAIVED until 2027-02-28:" in output
+    assert "Passed: skills/public/demo (1 waived finding(s))" in output
+
+
+def test_run_review_still_fails_for_unwaived_error(tmp_path: Path, monkeypatch) -> None:
+    package = tmp_path / "skills/public/demo"
+    _write_target(tmp_path)
+    facts = {
+        "summary": {"blockers": 0, "errors": 1, "warnings": 0, "infos": 0},
+        "completeness": {"not_assessed": []},
+        "findings": [_finding()],
+    }
+    monkeypatch.setattr(runner, "collect_review_facts", lambda *args: facts)
+
+    assert runner.run_review(package, tmp_path, "python", EMPTY_MANIFEST) == 1

contracts/skill_review/waiver_manifest.v1.schema.json

@@ -0,0 +1,32 @@
+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "$id": "https://deerflow.dev/contracts/skill_review/waiver_manifest.v1.schema.json",
+  "title": "DeerFlow Skill Review Waiver Manifest v1",
+  "type": "object",
+  "required": ["schema_version", "waivers"],
+  "properties": {
+    "schema_version": { "const": "deerflow.skill-review-waivers.v1" },
+    "waivers": {
+      "type": "array",
+      "maxItems": 256,
+      "items": {
+        "type": "object",
+        "required": ["package", "source", "rule_id", "path", "line", "evidence", "file_sha256", "reason", "expires_on"],
+        "properties": {
+          "package": { "type": "string", "pattern": "^skills/public/", "minLength": 15 },
+          "source": { "type": "string", "minLength": 1 },
+          "rule_id": { "type": "string", "minLength": 1 },
+          "path": { "type": "string", "minLength": 1 },
+          "line": { "type": "integer", "minimum": 1 },
+          "evidence": { "type": "string", "minLength": 1 },
+          "file_sha256": { "type": "string", "pattern": "^sha256:[0-9a-f]{64}$" },
+          "reason": { "type": "string", "minLength": 20 },
+          "expires_on": { "type": "string", "format": "date" },
+          "approved_in": { "type": "string", "minLength": 1 }
+        },
+        "additionalProperties": false
+      }
+    }
+  },
+  "additionalProperties": false
+}

scripts/AGENTS.md

@@ -15,6 +15,23 @@ likewise prefix the target with `bash`. This keeps documented `make` commands
 working when a source archive, `core.fileMode=false`, or a non-POSIX filesystem
 does not preserve executable bits.
 
+## Public Skill Review Waivers
+
+`review_changed_public_skills.py` keeps the analyzer strict and applies narrow
+CI-only exceptions from `.github/skill-review-waivers.v1.json`. The manifest is
+versioned by `contracts/skill_review/waiver_manifest.v1.schema.json`; each entry
+must identify one current error by package, source, rule, path, line, and
+evidence, and pin the complete source file with SHA-256 plus an expiry date.
+Blockers are never waivable, and waived errors are still printed with their
+original severity and justification.
+
+For pull requests, only the base revision's manifest is effective. The head
+manifest is parsed and checked against the current analyzer output, but cannot
+self-authorize a finding in the same pull request. Push comparisons use the
+same before/after trust boundary. A waiver-only change can therefore land
+without weakening its own check, then become effective for later changes after
+it is part of the trusted base.
+
 ## Backend Static Analysis Commands
 
 The root `detect-thread-boundaries` target statically inventories execution

scripts/review_changed_public_skills.py

@@ -4,12 +4,23 @@
 from __future__ import annotations
 
 import argparse
+import json
 import os
 import subprocess
 import sys
 from collections.abc import Sequence
 from dataclasses import dataclass
 from pathlib import Path, PurePosixPath
+from typing import Any
+
+from skill_review_waivers import (
+    EMPTY_MANIFEST,
+    WaiverManifest,
+    WaiverManifestError,
+    load_manifest_at_ref,
+    matching_waiver,
+    validate_manifest_against_facts,
+)
 
 REPO_ROOT = Path(__file__).resolve().parents[1]
 HARNESS_PATH = REPO_ROOT / "backend" / "packages" / "harness"
@@ -31,6 +42,17 @@ def main(argv: Sequence[str] | None = None) -> int:
     repo_root = args.repo_root.resolve()
     diff_args = build_diff_args(args)
 
+    try:
+        effective_manifest, proposed_manifest = load_waiver_manifests(args, repo_root)
+    except WaiverManifestError as exc:
+        sys.stderr.write(f"[skill-review] Invalid waiver manifest: {exc}\n")
+        return 1
+
+    print(f"[skill-review] Trusted waivers: {len(effective_manifest.waivers)}")
+    print(f"[skill-review] Proposed waivers validated but not trusted in this run: {len(proposed_manifest.waivers)}")
+    if validate_proposed_manifest(proposed_manifest, repo_root, args.python) != 0:
+        return 1
+
     print(f"[skill-review] Repository: {repo_root}")
     print(f"[skill-review] Diff: git diff {' '.join(diff_args)}")
 
@@ -70,7 +92,7 @@ def main(argv: Sequence[str] | None = None) -> int:
     print(f"[skill-review] Reviewing {len(packages)} changed public skill package(s).")
     failed = False
     for package in packages:
-        if run_review(package, repo_root, args.python) != 0:
+        if run_review(package, repo_root, args.python, effective_manifest) != 0:
             failed = True
 
     if failed:
@@ -81,6 +103,39 @@ def main(argv: Sequence[str] | None = None) -> int:
     return 0
 
 
+def load_waiver_manifests(args: argparse.Namespace, repo_root: Path) -> tuple[WaiverManifest, WaiverManifest]:
+    """Load effective waivers only from the trusted side of the comparison."""
+    if args.base_ref and args.head_ref:
+        effective = load_manifest_at_ref(repo_root, str(args.base_ref), label="trusted base")
+        proposed = load_manifest_at_ref(repo_root, str(args.head_ref), label="proposed head")
+        return effective, proposed
+
+    before = str(args.before)
+    effective = EMPTY_MANIFEST if is_zero_sha(before) else load_manifest_at_ref(repo_root, before, label="trusted before")
+    proposed = load_manifest_at_ref(repo_root, str(args.after), label="proposed after")
+    return effective, proposed
+
+
+def validate_proposed_manifest(manifest: WaiverManifest, repo_root: Path, python_executable: str) -> int:
+    """Verify every proposed entry still identifies one current error finding."""
+    if not manifest.waivers:
+        return 0
+
+    facts_by_package: dict[str, dict[str, Any]] = {}
+    for package_rel in sorted({waiver.package for waiver in manifest.waivers}):
+        package = repo_root / package_rel
+        print(f"[skill-review] Validating proposed waivers for: {package_rel}")
+        facts = collect_review_facts(package, repo_root, python_executable)
+        if facts is None:
+            return 1
+        facts_by_package[package_rel] = facts
+
+    errors = validate_manifest_against_facts(manifest, facts_by_package=facts_by_package, repo_root=repo_root)
+    for error in errors:
+        sys.stderr.write(f"[skill-review] Invalid proposed waiver: {error}\n")
+    return 1 if errors else 0
+
+
 def parse_args(argv: Sequence[str] | None) -> argparse.Namespace:
     parser = argparse.ArgumentParser(description=("Review public skill packages whose SKILL.md changed in a PR or push diff."))
     parser.add_argument(
@@ -245,7 +300,7 @@ def _is_eval_fixture_skill_md(path: PurePosixPath) -> bool:
     return is_eval_fixture_skill_md(path)
 
 
-def run_review(package: Path, repo_root: Path, python_executable: str) -> int:
+def collect_review_facts(package: Path, repo_root: Path, python_executable: str) -> dict[str, Any] | None:
     package_rel = package.relative_to(repo_root).as_posix()
     command = [
         python_executable,
@@ -253,36 +308,72 @@ def run_review(package: Path, repo_root: Path, python_executable: str) -> int:
         "deerflow.skills.review.cli",
         package_rel,
         "--format",
-        "text",
+        "json",
         "--fail-on",
-        "error",
-        "--fail-on-incomplete",
+        "never",
     ]
-    log_command = [
-        "python",
-        "-m",
-        "deerflow.skills.review.cli",
-        package_rel,
-        "--format",
-        "text",
-        "--fail-on",
-        "error",
-        "--fail-on-incomplete",
-    ]
-
-    print(f"[skill-review] Reviewing package: {package_rel}")
-    print(f"[skill-review] $ {' '.join(log_command)}")
     result = subprocess.run(
         command,
         cwd=repo_root,
         env=review_env(repo_root),
+        capture_output=True,
+        text=True,
         check=False,
     )
-    if result.returncode == 0:
-        print(f"[skill-review] Passed: {package_rel}")
-    else:
-        print(f"[skill-review] Failed: {package_rel} (exit {result.returncode})")
-    return result.returncode
+    if result.returncode != 0:
+        sys.stderr.write(f"[skill-review] Analyzer failed for {package_rel} (exit {result.returncode}).\n")
+        sys.stderr.write(result.stderr)
+        return None
+    try:
+        facts = json.loads(result.stdout)
+    except (json.JSONDecodeError, TypeError) as exc:
+        sys.stderr.write(f"[skill-review] Analyzer returned invalid JSON for {package_rel}: {exc}\n")
+        return None
+    if not isinstance(facts, dict):
+        sys.stderr.write(f"[skill-review] Analyzer returned a non-object payload for {package_rel}.\n")
+        return None
+    return facts
+
+
+def run_review(package: Path, repo_root: Path, python_executable: str, manifest: WaiverManifest = EMPTY_MANIFEST) -> int:
+    package_rel = package.relative_to(repo_root).as_posix()
+    print(f"[skill-review] Reviewing package: {package_rel}")
+    print(f"[skill-review] $ python -m deerflow.skills.review.cli {package_rel} --format json --fail-on never")
+    facts = collect_review_facts(package, repo_root, python_executable)
+    if facts is None:
+        print(f"[skill-review] Failed: {package_rel}")
+        return 1
+
+    summary = facts.get("summary", {})
+    completeness = facts.get("completeness", {})
+    print(f"[skill-review] Summary: {summary.get('blockers')} blocker(s), {summary.get('errors')} error(s), {summary.get('warnings')} warning(s), {summary.get('infos')} info(s)")
+    not_assessed = completeness.get("not_assessed") or []
+    failed = bool(not_assessed)
+    if not_assessed:
+        print(f"[skill-review] Incomplete review: {', '.join(str(item) for item in not_assessed)}")
+
+    waived_count = 0
+    for finding in facts.get("findings", []):
+        if not isinstance(finding, dict):
+            failed = True
+            continue
+        location = finding.get("path") or "<package>"
+        if finding.get("line") is not None:
+            location = f"{location}:{finding['line']}"
+        waiver = matching_waiver(finding, package=package_rel, manifest=manifest, repo_root=repo_root)
+        waiver_suffix = ""
+        if waiver is not None:
+            waived_count += 1
+            waiver_suffix = f" [WAIVED until {waiver.expires_on.isoformat()}: {waiver.reason}]"
+        elif finding.get("severity") in {"blocker", "error"}:
+            failed = True
+        print(f"- {finding.get('severity')} {finding.get('rule_id')} at {location}: {finding.get('message')}{waiver_suffix}")
+
+    if failed:
+        print(f"[skill-review] Failed: {package_rel}")
+        return 1
+    print(f"[skill-review] Passed: {package_rel} ({waived_count} waived finding(s))")
+    return 0
 
 
 def review_env(repo_root: Path) -> dict[str, str]:

scripts/skill_review_waivers.py

@@ -0,0 +1,272 @@
+"""Trusted, exact-match waivers for the public-skill CI review gate."""
+
+from __future__ import annotations
+
+import hashlib
+import json
+import re
+import subprocess
+from dataclasses import dataclass
+from datetime import date
+from pathlib import Path, PurePosixPath
+from typing import Any
+
+SCHEMA_VERSION = "deerflow.skill-review-waivers.v1"
+MANIFEST_PATH = PurePosixPath(".github/skill-review-waivers.v1.json")
+_SHA256_RE = re.compile(r"sha256:[0-9a-f]{64}\Z")
+_SAFE_REF_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._/-]*\Z")
+_REQUIRED_ENTRY_FIELDS = {
+    "package",
+    "source",
+    "rule_id",
+    "path",
+    "line",
+    "evidence",
+    "file_sha256",
+    "reason",
+    "expires_on",
+}
+_OPTIONAL_ENTRY_FIELDS = {"approved_in"}
+
+
+class WaiverManifestError(ValueError):
+    """Raised when a waiver manifest is missing required trust properties."""
+
+
+@dataclass(frozen=True)
+class SkillReviewWaiver:
+    package: str
+    source: str
+    rule_id: str
+    path: str
+    line: int
+    evidence: str
+    file_sha256: str
+    reason: str
+    expires_on: date
+    approved_in: str | None = None
+
+    @property
+    def finding_key(self) -> tuple[str, str, str, int, str]:
+        return (self.source, self.rule_id, self.path, self.line, self.evidence)
+
+
+@dataclass(frozen=True)
+class WaiverManifest:
+    waivers: tuple[SkillReviewWaiver, ...] = ()
+
+
+EMPTY_MANIFEST = WaiverManifest()
+
+
+def parse_manifest(payload: bytes | str, *, source: str) -> WaiverManifest:
+    """Parse the versioned waiver manifest with strict, fail-closed validation."""
+    try:
+        data = json.loads(payload)
+    except (json.JSONDecodeError, UnicodeDecodeError, TypeError) as exc:
+        raise WaiverManifestError(f"{source}: invalid JSON: {exc}") from exc
+
+    if not isinstance(data, dict):
+        raise WaiverManifestError(f"{source}: manifest root must be an object")
+    unknown_root = set(data) - {"schema_version", "waivers"}
+    if unknown_root:
+        raise WaiverManifestError(f"{source}: unknown root field(s): {', '.join(sorted(unknown_root))}")
+    if data.get("schema_version") != SCHEMA_VERSION:
+        raise WaiverManifestError(f"{source}: schema_version must be {SCHEMA_VERSION!r}")
+    raw_waivers = data.get("waivers")
+    if not isinstance(raw_waivers, list):
+        raise WaiverManifestError(f"{source}: waivers must be an array")
+    if len(raw_waivers) > 256:
+        raise WaiverManifestError(f"{source}: waivers must contain at most 256 entries")
+
+    waivers: list[SkillReviewWaiver] = []
+    identities: set[tuple[str, str, str, int, str, str]] = set()
+    for index, raw in enumerate(raw_waivers):
+        entry_source = f"{source}: waivers[{index}]"
+        if not isinstance(raw, dict):
+            raise WaiverManifestError(f"{entry_source}: entry must be an object")
+        fields = set(raw)
+        missing = _REQUIRED_ENTRY_FIELDS - fields
+        unknown = fields - _REQUIRED_ENTRY_FIELDS - _OPTIONAL_ENTRY_FIELDS
+        if missing:
+            raise WaiverManifestError(f"{entry_source}: missing field(s): {', '.join(sorted(missing))}")
+        if unknown:
+            raise WaiverManifestError(f"{entry_source}: unknown field(s): {', '.join(sorted(unknown))}")
+
+        package = _canonical_relative_path(raw["package"], field=f"{entry_source}.package")
+        if not package.startswith("skills/public/"):
+            raise WaiverManifestError(f"{entry_source}.package: must be below skills/public/")
+        path = _canonical_relative_path(raw["path"], field=f"{entry_source}.path")
+        source_name = _nonempty_string(raw["source"], field=f"{entry_source}.source")
+        rule_id = _nonempty_string(raw["rule_id"], field=f"{entry_source}.rule_id")
+        evidence = _nonempty_string(raw["evidence"], field=f"{entry_source}.evidence")
+        file_sha256 = _nonempty_string(raw["file_sha256"], field=f"{entry_source}.file_sha256")
+        if not _SHA256_RE.fullmatch(file_sha256):
+            raise WaiverManifestError(f"{entry_source}.file_sha256: must be sha256 followed by 64 lowercase hex characters")
+        reason = _nonempty_string(raw["reason"], field=f"{entry_source}.reason")
+        if len(reason) < 20:
+            raise WaiverManifestError(f"{entry_source}.reason: must contain at least 20 characters")
+        line = raw["line"]
+        if isinstance(line, bool) or not isinstance(line, int) or line < 1:
+            raise WaiverManifestError(f"{entry_source}.line: must be a positive integer")
+        expires_on = _parse_date(raw["expires_on"], field=f"{entry_source}.expires_on")
+        approved_in = raw.get("approved_in")
+        if approved_in is not None:
+            approved_in = _nonempty_string(approved_in, field=f"{entry_source}.approved_in")
+
+        waiver = SkillReviewWaiver(
+            package=package,
+            source=source_name,
+            rule_id=rule_id,
+            path=path,
+            line=line,
+            evidence=evidence,
+            file_sha256=file_sha256,
+            reason=reason,
+            expires_on=expires_on,
+            approved_in=approved_in,
+        )
+        identity = (*waiver.finding_key, waiver.package)
+        if identity in identities:
+            raise WaiverManifestError(f"{entry_source}: duplicates an earlier waiver")
+        identities.add(identity)
+        waivers.append(waiver)
+
+    return WaiverManifest(tuple(waivers))
+
+
+def load_manifest_at_ref(repo_root: Path, ref: str, *, label: str) -> WaiverManifest:
+    """Read a manifest from a Git ref; absence means no waivers at that ref."""
+    if not _SAFE_REF_RE.fullmatch(ref) or ref.startswith("-") or ".." in ref or ":" in ref:
+        raise WaiverManifestError(f"{label}: unsafe Git ref {ref!r}")
+    object_name = f"{ref}:{MANIFEST_PATH.as_posix()}"
+    result = subprocess.run(
+        ["git", "show", object_name],
+        cwd=repo_root,
+        capture_output=True,
+        check=False,
+    )
+    if result.returncode != 0:
+        return EMPTY_MANIFEST
+    return parse_manifest(result.stdout, source=f"{label} manifest at {ref}")
+
+
+def file_sha256(repo_root: Path, waiver: SkillReviewWaiver) -> str | None:
+    resolved_repo_root = repo_root.resolve()
+    package_candidate = resolved_repo_root / waiver.package
+    if package_candidate.is_symlink() or not package_candidate.is_dir():
+        return None
+    package_root = package_candidate.resolve()
+    try:
+        package_root.relative_to(resolved_repo_root / "skills" / "public")
+    except ValueError:
+        return None
+    candidate = package_root / waiver.path
+    if candidate.is_symlink() or not candidate.is_file():
+        return None
+    resolved = candidate.resolve()
+    try:
+        resolved.relative_to(package_root)
+    except ValueError:
+        return None
+    digest = hashlib.sha256(resolved.read_bytes()).hexdigest()
+    return f"sha256:{digest}"
+
+
+def finding_key(finding: dict[str, Any]) -> tuple[str, str, str, int, str] | None:
+    source = finding.get("source")
+    rule_id = finding.get("rule_id")
+    path = finding.get("path")
+    line = finding.get("line")
+    evidence = finding.get("evidence")
+    if not isinstance(source, str) or not isinstance(rule_id, str) or not isinstance(path, str) or isinstance(line, bool) or not isinstance(line, int) or not isinstance(evidence, str):
+        return None
+    return (source, rule_id, path, line, evidence)
+
+
+def matching_waiver(
+    finding: dict[str, Any],
+    *,
+    package: str,
+    manifest: WaiverManifest,
+    repo_root: Path,
+    today: date | None = None,
+) -> SkillReviewWaiver | None:
+    """Return an active waiver only for an exact error finding and file digest."""
+    if finding.get("severity") != "error":
+        return None
+    key = finding_key(finding)
+    if key is None:
+        return None
+    current_date = today or date.today()
+    for waiver in manifest.waivers:
+        if waiver.package != package or waiver.finding_key != key:
+            continue
+        if waiver.expires_on < current_date:
+            continue
+        if file_sha256(repo_root, waiver) != waiver.file_sha256:
+            continue
+        return waiver
+    return None
+
+
+def validate_manifest_against_facts(
+    manifest: WaiverManifest,
+    *,
+    facts_by_package: dict[str, dict[str, Any]],
+    repo_root: Path,
+    today: date | None = None,
+) -> list[str]:
+    """Reject expired, stale, hash-mismatched, or non-error waiver entries."""
+    errors: list[str] = []
+    current_date = today or date.today()
+    for waiver in manifest.waivers:
+        description = f"{waiver.package}/{waiver.path}:{waiver.line} ({waiver.source}/{waiver.rule_id})"
+        if waiver.expires_on < current_date:
+            errors.append(f"{description}: waiver expired on {waiver.expires_on.isoformat()}")
+            continue
+        facts = facts_by_package.get(waiver.package)
+        if facts is None:
+            errors.append(f"{description}: package review facts are unavailable")
+            continue
+        matches = [finding for finding in facts.get("findings", []) if isinstance(finding, dict) and finding_key(finding) == waiver.finding_key]
+        if not matches:
+            errors.append(f"{description}: no exact current finding matches this waiver")
+            continue
+        if len(matches) != 1:
+            errors.append(f"{description}: expected one exact current finding, found {len(matches)}")
+            continue
+        if any(finding.get("severity") != "error" for finding in matches):
+            errors.append(f"{description}: waivers may target error findings only; blockers can never be waived")
+            continue
+        actual_hash = file_sha256(repo_root, waiver)
+        if actual_hash != waiver.file_sha256:
+            errors.append(f"{description}: file digest changed (expected {waiver.file_sha256}, found {actual_hash or 'unavailable'})")
+    return errors
+
+
+def _canonical_relative_path(value: object, *, field: str) -> str:
+    text = _nonempty_string(value, field=field)
+    if "\\" in text:
+        raise WaiverManifestError(f"{field}: backslashes are not allowed")
+    path = PurePosixPath(text)
+    if path.is_absolute() or any(part in {"", ".", ".."} for part in path.parts) or path.as_posix() != text:
+        raise WaiverManifestError(f"{field}: must be a canonical relative POSIX path without traversal")
+    return text
+
+
+def _nonempty_string(value: object, *, field: str) -> str:
+    if not isinstance(value, str) or not value.strip() or value != value.strip():
+        raise WaiverManifestError(f"{field}: must be a non-empty string without surrounding whitespace")
+    return value
+
+
+def _parse_date(value: object, *, field: str) -> date:
+    text = _nonempty_string(value, field=field)
+    try:
+        parsed = date.fromisoformat(text)
+    except ValueError as exc:
+        raise WaiverManifestError(f"{field}: must be an ISO 8601 calendar date") from exc
+    if parsed.isoformat() != text:
+        raise WaiverManifestError(f"{field}: must use YYYY-MM-DD format")
+    return parsed