From 0401c4594b5c46ab84e206120c25546f0adb4849 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Thu, 22 Oct 2026 11:36:30 +0800 Subject: [PATCH 1/3] fix: stop stale knowledge sweeps from failing merged PRs The admission checker published a stale-base failure to every captured head, including heads that had just merged. Later sweeps inspect only open PRs, so those merged heads stayed red. Results also collapsed evaluator and publication failures into the same failed status as a real rejection. - Publish only after a fresh open-PR scan, and only for a target that is still open with unchanged head, base, body and approvals. Closed or superseded targets receive no write; their earlier verdict stays historical. - On an advanced base, re-evaluate against the current base inside the run. Unfinished work reports `deferred` and exits 2; the workflow runner refetches main and reruns. Open PRs stay fail-closed throughout. - Report `kind` per result (compliant, noncompliant, evaluator-error, deferred, closed, publication-failed) and print a summary. A shared head is an eligibility refusal, not an evaluator failure. - Replace the transient pending write with the final verdict so an interrupted run cannot leave pending residue on a merging head. - Add regressions for merge during evaluation, base advance, late publication, close/reopen, shared heads and mixed batches. Agent-assisted-by: Claude Code / claude-opus-5-5 [effort=xhigh] (author) Co-Authored-By: Claude Opus 5.5 Signed-off-by: GUOGUO --- .github/workflows/knowledge-model-policy.yml | 2 +- docs/knowledge-model-policy.md | 42 +++- tests/test_knowledge_model_policy.py | 93 ++++++++- tools/check_knowledge_model_policy.py | 191 +++++++++++++------ tools/run_knowledge_model_policy.sh | 18 ++ 5 files changed, 284 insertions(+), 62 deletions(-) create mode 100644 tools/run_knowledge_model_policy.sh diff --git a/.github/workflows/knowledge-model-policy.yml b/.github/workflows/knowledge-model-policy.yml index 10781313c..ad65f3586 100644 --- a/.github/workflows/knowledge-model-policy.yml +++ b/.github/workflows/knowledge-model-policy.yml @@ -34,4 +34,4 @@ jobs: - name: Evaluate declarations with trusted main code env: GITHUB_TOKEN: ${{ github.token }} - run: python tools/check_knowledge_model_policy.py --policy-sha "$(git rev-parse HEAD)" + run: bash tools/run_knowledge_model_policy.sh diff --git a/docs/knowledge-model-policy.md b/docs/knowledge-model-policy.md index 867a2a7b1..c6e5e56bc 100644 --- a/docs/knowledge-model-policy.md +++ b/docs/knowledge-model-policy.md @@ -127,11 +127,43 @@ open PRs is ineligible, even if one PR declares compliant authorship or receives PR-specific exception. The checker reads all open repository PRs, across target branches and API pages, and requires a unique head both before evaluation and before publishing its result. Close the extra PR or use a distinct commit before -retrying. An exception cannot bypass head uniqueness. Each refresh reads the full -open-PR collection once before evaluation and once before publication, with API -pagination in both scans. Results remain pending until the second scan revalidates -head ownership and PR metadata; applicable approvals are reread before publication. -The collection request count scales with its page count, not page count times PRs. +retrying. An exception cannot bypass head uniqueness. A closed peer no longer +shares the head: the remaining open PR is evaluated on its own metadata and +receives its own verdict. Each evaluation round reads the full open-PR collection +once before evaluation and once before publication, with API pagination in both +scans. Results are published only from that fresh scan, and only for a target that +is still open against the policy branch with unchanged head, base and body; +applicable approvals are reread before publication. The collection request count +scales with its page count, not page count times PRs. + +## Lifecycle and reruns + +A merged or closed PR that is no longer part of the sweep receives no new status; +its earlier verdict stays historical. Publication never targets a captured head +for work whose base advanced, whose PR changed, or whose approvals were edited +during evaluation. When the trusted base advances mid-sweep, the checker +re-evaluates open targets against the current base for a bounded number of rounds +inside the same run. If work is still moving, the run publishes no verdict for it, +reports the target as `deferred`, and exits `2`; trusted +`tools/run_knowledge_model_policy.sh` then refetches `main` and runs the checker +again from the current checkout. After its own attempt budget, the workflow run +fails. Open PRs remain fail-closed in every case: stale work is never published as +compliant, and an old green is never retained in place of a valid evaluation. + +Each result carries `kind` alongside the status `state`: + +| `kind` | Meaning | +| --- | --- | +| `compliant` | Declaration and approvals satisfy the trusted policy | +| `noncompliant` | Genuine rejection: declaration, approval or head eligibility (including a shared head) | +| `evaluator-error` | GitHub API, incomplete evidence or unsupported lifecycle input; never a rejection | +| `deferred` | Work could not be completed in this run; rerun required | +| `closed` | Target left the sweep; no status was written | +| `publication-failed` | Status write failed; the verdict was not recorded | + +The final JSON summary counts each kind. Exit codes: `0` all published results +compliant, `1` at least one failing kind, `2` unfinished work (rerun). A rerun +never overrides a recorded rejection. The workflow handles PR open/reopen/close/commits/body-or-base edits on any target branch (a PR against another base can share a main PR's head), approval-comment diff --git a/tests/test_knowledge_model_policy.py b/tests/test_knowledge_model_policy.py index 5146a7bcc..ed10f5029 100644 --- a/tests/test_knowledge_model_policy.py +++ b/tests/test_knowledge_model_policy.py @@ -11,6 +11,9 @@ from tools.check_knowledge_model_policy import ( APPROVAL, DECLARATION, + EXIT_COMPLIANT, + EXIT_NONCOMPLIANT, + EXIT_RERUN, GitHub, approved, assemble_file_pages, @@ -19,12 +22,14 @@ check_pr, check_prs, declaration, + exit_code, file_page_numbers, json_block, matching, model_errors, refresh_event, snapshot, + summarize, ) ROOT = Path(__file__).resolve().parents[1] @@ -305,6 +310,11 @@ def test_workflow_never_executes_pr_code_and_refreshes_metadata_and_base(): steps = value["jobs"]["declarations"]["steps"] assert steps[0]["with"] == {"ref": "main", "persist-credentials": False} assert all("pip install" not in step.get("run", "") for step in steps) + assert steps[-1]["run"] == "bash tools/run_knowledge_model_policy.sh" + runner = (ROOT / "tools/run_knowledge_model_policy.sh").read_text(encoding="utf-8") + # The runner refetches trusted main and reruns only for the stale-base exit code. + assert "git fetch --quiet origin main" in runner + assert "--policy-sha" in runner and 'status" -ne 2' in runner assert "pull_request.head" not in json.dumps(value) assert "knowledge-model-policy" not in ( ROOT / ".github/workflows/ci-ready.yml" @@ -315,7 +325,7 @@ def test_workflow_never_executes_pr_code_and_refreshes_metadata_and_base(): def api_fixture(): # 本机 HTTP fixture 使用真实 GitHub adapter，记录实际 POST 的共享 status。 state = {"prs": {123: pr()}, "comments": {}, "statuses": [], "requests": [], - "before_request": None, "errors": []} + "before_request": None, "errors": [], "base": BASE} class Handler(BaseHTTPRequestHandler): def log_message(self, *args): @@ -347,7 +357,7 @@ def do_GET(self): if len(values) > page * 100 else None) self.respond(values[(page - 1) * 100:page * 100], links) elif route == "git/ref/heads/main": - self.respond({"object": {"sha": BASE}}) + self.respond({"object": {"sha": state["base"]}}) elif route.startswith("pulls/"): parts = route.split("/") number = int(parts[1]) @@ -522,3 +532,82 @@ def change_earlier_pr(route): assert [r["state"] for r in results] == ["failure", "success"] assert latest_status(state)["state"] == "failure" assert latest_status(state, "c" * 40)["state"] == "success" + + +def test_api_merge_during_evaluation_publishes_nothing_to_the_merged_head(api_fixture, policy): + api, state = api_fixture + + def merge(route): + if route == "pulls/123/files": + state["prs"][123]["state"] = "closed" + + state["before_request"] = merge + results = check_prs(api, policy, BASE, False, [123]) + assert [r["kind"] for r in results] == ["closed"] + assert state["statuses"] == [] + assert exit_code(results) == EXIT_COMPLIANT + + +def test_api_base_advance_defers_instead_of_failing_the_captured_head(api_fixture, policy): + api, state = api_fixture + state["base"] = "e" * 40 + results = check_prs(api, policy, BASE, False, [123]) + assert [r["kind"] for r in results] == ["deferred"] + assert results[0]["rerun"] is True + # Open work stays fail-closed, but the published status never claims a declaration verdict. + assert latest_status(state)["state"] == "failure" + assert latest_status(state)["description"] == "Evaluation deferred" + assert exit_code(results) == EXIT_RERUN + # A rerun against the advanced main publishes the real verdict. + assert check_pr(api, 123, policy, "e" * 40, False)["state"] == "success" + assert latest_status(state)["state"] == "success" + + +def test_api_closed_target_keeps_historical_status_and_reopen_reevaluates(api_fixture, policy): + api, state = api_fixture + assert check_pr(api, 123, policy, BASE, False)["state"] == "success" + state["prs"][123]["state"] = "closed" + result = check_pr(api, 123, policy, BASE, False) + assert result["kind"] == "closed" + assert len(state["statuses"]) == 1 # The earlier verdict is not rewritten. + state["prs"][123]["state"] = "open" + state["prs"][123]["body"] = None + assert check_pr(api, 123, policy, BASE, False)["state"] == "failure" + assert latest_status(state)["state"] == "failure" + + +def test_api_peer_sharing_a_merged_head_keeps_its_own_verdict(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = {**pr(), "number": 124} + results = check_prs(api, policy, BASE, False, [123, 124]) + assert [(r["pull_request"], r["kind"]) for r in results] == [(123, "noncompliant"), + (124, "noncompliant")] + assert "shared heads" in results[0]["errors"][0] + assert latest_status(state)["state"] == "failure" + # One peer closes: the merged PR is dropped from the sweep and the survivor keeps the head + # under a fresh evaluation instead of inheriting the shared-head refusal. + state["prs"][123]["state"] = "closed" + results = check_prs(api, policy, BASE, False, [123, 124]) + assert [(r["pull_request"], r["kind"]) for r in results] == [(123, "closed"), + (124, "compliant")] + assert latest_status(state)["state"] == "success" + + +def test_api_batch_separates_rejection_from_lifecycle_outcomes(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = separate_pr(124, "c" * 40) + state["prs"][124]["body"] = None + state["prs"][125] = separate_pr(125, "d" * 40) + state["prs"][125]["state"] = "closed" + results = check_prs(api, policy, BASE, False, [123, 124, 125]) + kinds = {r["pull_request"]: r["kind"] for r in results} + assert kinds == {123: "compliant", 124: "noncompliant", 125: "closed"} + assert latest_status(state)["state"] == "success" + assert latest_status(state, "c" * 40)["state"] == "failure" + assert all(s["sha"] != "d" * 40 for s in state["statuses"]) + summary = summarize(results) + assert summary == {"kind_counts": {"compliant": 1, "noncompliant": 1, "evaluator-error": 0, + "deferred": 0, "closed": 1, "publication-failed": 0}, + "results": 3, "failed": 1, "rerun": False} + assert exit_code(results) == EXIT_NONCOMPLIANT + assert exit_code([r for r in results if r["kind"] == "compliant"]) == EXIT_COMPLIANT diff --git a/tools/check_knowledge_model_policy.py b/tools/check_knowledge_model_policy.py index 8f269fb80..18ee0eb55 100644 --- a/tools/check_knowledge_model_policy.py +++ b/tools/check_knowledge_model_policy.py @@ -14,6 +14,32 @@ APPROVAL = "knowledge-policy-approval" SHA = re.compile(r"[0-9a-f]{40}") ROOT = Path(__file__).resolve().parents[1] +# Exit codes: published results are final, a rejection is final, but stale work must be rerun. +EXIT_COMPLIANT = 0 +EXIT_NONCOMPLIANT = 1 +EXIT_RERUN = 2 +# Bounded in-run re-evaluation before the run reports the work as unfinished. +ATTEMPTS = 3 +API_ERRORS = (ValueError, KeyError, TypeError, OSError) +# Outcome kinds, reported separately so evaluator noise is never read as a rejection. +KINDS = ("compliant", "noncompliant", "evaluator-error", "deferred", "closed", + "publication-failed") +FAILING_KINDS = {"noncompliant", "evaluator-error", "deferred", "publication-failed"} + + +def summarize(results): + counts = {kind: sum(result["kind"] == kind for result in results) for kind in KINDS} + return {"kind_counts": counts, "results": len(results), + "failed": sum(result["kind"] in FAILING_KINDS for result in results), + "rerun": any(result["rerun"] for result in results)} + + +def exit_code(results): + # Unfinished (rerun) work keeps open PRs fail-closed; a real rejection alone exits 1. + if any(result["rerun"] for result in results): + return EXIT_RERUN + return EXIT_NONCOMPLIANT if any(result["kind"] in FAILING_KINDS for result in results) \ + else EXIT_COMPLIANT def unique_object(pairs): @@ -207,8 +233,10 @@ def assess(pr, paths, policy, comments=(), contributors=()): except (ValueError, TypeError): errors.append("Missing, invalid or stale knowledge author declaration") applicable = bool(protected or controls) + # Pure metadata: same state, but a kind so a rejection is never confused with evaluator noise. return { "state": "failure" if errors else "success", + "kind": "noncompliant" if errors else "compliant", "description": ("Declaration or policy approval failed" if errors else "Declaration compliance only" if applicable else "Not applicable"), "errors": errors, @@ -277,12 +305,25 @@ def require_unique_head(prs, number, head): raise ValueError("Head must belong to exactly one open PR; shared heads are ineligible") +def deferred(reason): + # Not a verdict: the target must be re-evaluated before anything is published. + return {"state": "failure", "kind": "deferred", "description": "Evaluation deferred", + "errors": [reason], "retry": True} + + def evaluation_failure(error): message = (f"GitHub API HTTP {error.code}" if isinstance(error, HTTPError) else "GitHub API unavailable" if isinstance(error, OSError) else str(error) if isinstance(error, ValueError) else "Invalid GitHub API response") - return {"state": "failure", "description": "Policy evaluation failed", "errors": [message]} + return {"state": "failure", "kind": "evaluator-error", + "description": "Policy evaluation failed", "errors": [message], "retry": True} + + +def refused(reason): + # Deterministic refusal by admission metadata, not an evaluator failure; still fail-closed. + return {"state": "failure", "kind": "noncompliant", + "description": "Eligibility refused", "errors": [reason], "retry": False} def read_pr_result(api, pr, policy, policy_sha, open_prs): @@ -290,12 +331,14 @@ def read_pr_result(api, pr, policy, policy_sha, open_prs): head = pr["head"]["sha"] comments = [] try: - if pr["state"] != "open" or pr["base"]["ref"] != policy["base_branch"]: - raise ValueError("PR is not open against the supported policy branch") require_unique_head(open_prs, number, head) - base_route = f"git/ref/heads/{policy['base_branch']}" - if api.get(base_route)["object"]["sha"] != policy_sha: - raise ValueError("Trusted policy checkout is stale; rerun on the current base") + except ValueError as error: + return refused(str(error)), comments + base_route = f"git/ref/heads/{policy['base_branch']}" + if api.get(base_route)["object"]["sha"] != policy_sha: + # The trusted checkout moved: this verdict belongs to no publishable base. + return deferred("Trusted policy checkout is stale; rerun on the current base"), comments + try: pages = [api.get(f"pulls/{number}/files?per_page=100&page={page}") for page in file_page_numbers(pr["changed_files"])] files = assemble_file_pages(pages, pr["changed_files"]) @@ -322,64 +365,104 @@ def read_pr_result(api, pr, policy, policy_sha, open_prs): contributors.add(commit[field]["login"]) comments = api.pages(f"issues/{number}/comments") result = assess(pr, paths, policy, comments, contributors) - # 发布前重读元数据与批准，防止正文、head、base 或例外已变化。 - if (snapshot(api.get(f"pulls/{number}")) != snapshot(pr) - or api.get(base_route)["object"]["sha"] != policy_sha): - raise ValueError("PR or base changed during evaluation; rerun required") + # Anything that moved since the snapshot invalidates this verdict; the base ref must + # still match the trusted checkout, or the verdict belongs to no publishable base. + if snapshot(api.get(f"pulls/{number}")) != snapshot(pr): + return deferred("PR changed during evaluation; rerun required"), comments + if api.get(base_route)["object"]["sha"] != policy_sha: + return deferred("Base advanced during evaluation; rerun required"), comments if (controls or request is not None) and comments != api.pages(f"issues/{number}/comments"): - raise ValueError("Approval comments changed during evaluation; rerun required") - except (ValueError, KeyError, TypeError, OSError) as error: + # Re-read approvals that were read, not only approvals that changed the verdict. + return deferred("Approval comments changed during evaluation; rerun required"), comments + except API_ERRORS as error: result = evaluation_failure(error) return result, comments def admission_snapshot(pr): - # list PR API 不含 changed_files；head/base 相同才可复用已验证的文件列表。 + # list PR API 不含 changed_files；head/base/body/state 相同才可复用已验证的文件列表。 return (pr["head"]["sha"], pr["base"]["ref"], pr["base"]["sha"], pr.get("body"), pr["state"]) +def addressable(pr, policy): + return pr["state"] == "open" and pr["base"]["ref"] == policy["base_branch"] + + def check_prs(api, policy, policy_sha, dry_run, numbers=None): open_prs = api.pages("pulls?state=open") if numbers is None: - numbers = [pr["number"] for pr in open_prs if pr["base"]["ref"] == policy["base_branch"]] - pending = [] - for number in numbers: - pr = api.get(f"pulls/{number}") - head = pr["head"]["sha"] - if not SHA.fullmatch(head): - raise ValueError("Invalid PR head") - if not dry_run: - api.status(head, "pending", "Checking current declaration against trusted base policy") - result, comments = read_pr_result(api, pr, policy, policy_sha, open_prs) - pending.append((pr, result, comments)) - # 整轮只扫描两次 open PR；最终索引同时复核共享 head 与正文等元数据。 - try: - current = api.pages("pulls?state=open") - if api.get(f"git/ref/heads/{policy['base_branch']}")["object"]["sha"] != policy_sha: - raise ValueError("Trusted policy checkout is stale; rerun on the current base") - by_number = {pr["number"]: pr for pr in current} - refresh_error = None - except (ValueError, KeyError, TypeError, OSError) as error: - refresh_error = error - results = [] - for pr, result, comments in pending: - number, head = pr["number"], pr["head"]["sha"] - try: - if refresh_error is not None: - raise refresh_error - require_unique_head(current, number, head) - if admission_snapshot(by_number[number]) != admission_snapshot(pr): - raise ValueError("PR changed before batch publication; rerun required") + numbers = [pr["number"] for pr in open_prs if addressable(pr, policy)] + published = {} + retry = list(numbers) + # Results are published only from a fresh scan, and only for a target that still matches + # the evaluated metadata. Stale or moved work is re-evaluated instead of being written to + # a captured head; a closed target receives no publication at all. + for attempt in range(ATTEMPTS): + staged, retry_next = [], [] + for number in retry: + pr = api.get(f"pulls/{number}") + head = pr["head"]["sha"] + if not SHA.fullmatch(head): + raise ValueError("Invalid PR head") + if not addressable(pr, policy): + published[number] = {"head": head, "state": "success", "kind": "closed", + "description": "Not open against the policy branch", + "errors": [], "retry": False} + continue + # No pending write: a run that stops early must not leave a stale pending status + # on a head that merges or otherwise leaves the sweep. + result, comments = read_pr_result(api, pr, policy, policy_sha, open_prs) + if result.pop("retry", False) and attempt + 1 < ATTEMPTS: + retry_next.append(number) + else: + staged.append((pr, result, comments)) + open_prs = api.pages("pulls?state=open") + current = {pr["number"]: pr for pr in open_prs} + for pr, result, comments in staged: + number, head = pr["number"], pr["head"]["sha"] + live = current.get(number) + if live is None or not addressable(live, policy): + published[number] = {"head": head, "state": "success", "kind": "closed", + "description": "Closed before publication", "errors": [], + "retry": False} + continue + if admission_snapshot(live) != admission_snapshot(pr): + retry_next.append(number) + continue if comments and comments != api.pages(f"issues/{number}/comments"): - raise ValueError( - "Approval comments changed before batch publication; rerun required") - except (ValueError, KeyError, TypeError, OSError) as error: - result = evaluation_failure(error) - if not dry_run: - api.status(head, result["state"], result["description"]) - results.append({"pull_request": number, "head_sha": head, "policy_sha": policy_sha, - "dry_run": dry_run, **result}) + # Approvals read for this verdict must still be current at publication. + retry_next.append(number) + continue + try: + require_unique_head(open_prs, number, head) + except ValueError as error: + result = refused(str(error)) + # Only a fresh, fully revalidated verdict reaches a head; a shared head is refused + # above, so each head is written at most once per round. + if not dry_run: + try: + api.status(head, result["state"], result["description"]) + except API_ERRORS as error: + result = {**result, "kind": "publication-failed", + "errors": [*result["errors"], + evaluation_failure(error)["errors"][0]]} + published[number] = {"head": head, **result} + retry = sorted(set(retry_next) - set(published)) + if not retry: + break + # After the attempt budget, an unfinished target stays failed and asks for a fresh run. + for number in retry: + published.setdefault(number, {"head": "", "state": "failure", "kind": "deferred", + "description": "Evaluation incomplete", "errors": []}) + results = [] + for number in sorted(published): + item = published[number] + results.append({"pull_request": number, "head_sha": item["head"], + "policy_sha": policy_sha, "dry_run": dry_run, + "rerun": item["kind"] == "deferred", "state": item["state"], + "kind": item["kind"], "description": item["description"], + "errors": item["errors"]}) return results @@ -421,11 +504,11 @@ def main(argv=None): event = json.loads(Path(args.event_path).read_text(encoding="utf-8")) # 所有触发共用一个 writer；待运行事件被合并时仍覆盖全部 open PR。 numbers = None if refresh_event(event, os.environ["GITHUB_EVENT_NAME"]) else [] - failed = False - for result in check_prs(api, policy, args.policy_sha, args.dry_run, numbers): + results = check_prs(api, policy, args.policy_sha, args.dry_run, numbers) + for result in results: print(json.dumps(result, ensure_ascii=True)) - failed |= result["state"] == "failure" - return int(failed) + print(json.dumps(summarize(results), ensure_ascii=True)) + return exit_code(results) if __name__ == "__main__": diff --git a/tools/run_knowledge_model_policy.sh b/tools/run_knowledge_model_policy.sh new file mode 100644 index 000000000..344f72ab4 --- /dev/null +++ b/tools/run_knowledge_model_policy.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# Run the knowledge admission checker against the current trusted main. +# Exit 2 means the trusted base advanced during evaluation: refetch main and evaluate again. +set -uo pipefail + +attempts=3 +for attempt in $(seq 1 "$attempts"); do + git fetch --quiet origin main || exit 1 + git checkout --quiet origin/main || exit 1 + status=0 + python tools/check_knowledge_model_policy.py --policy-sha "$(git rev-parse HEAD)" "$@" || status=$? + if [ "$status" -ne 2 ]; then + exit "$status" + fi + echo "Trusted base advanced during evaluation; retrying against current main (${attempt}/${attempts})" >&2 +done +echo "Trusted base kept advancing; ending this run as failed" >&2 +exit 1 From ea3c20a973ea148c8736a2d90da9fd48433837d5 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Thu, 22 Oct 2026 14:58:12 +0800 Subject: [PATCH 2/3] fix: invalidate old admission green and single-retry the trusted base Review findings on the first revision: - An old success could survive an evaluator error or a failed lifecycle read. Publication now separates invalidating an old success from publishing a verdict: success is written only from a verified premise, and an unreadable open-PR collection turns a compliant result into an explicit `unverified` failure. A target that cannot be re-read keeps no new write because it may be merged; the report says the old status could not be invalidated instead of claiming fail-closed. - A staged shared-head refusal was kept after its premise changed. The publication pass now compares current head ownership and reassesses once when a peer closed, while real shared heads across bases and pages still fail. - The checker no longer retries its own stale base: it returns `deferred` and exits 2, and the outer runner is the single retry owner that refetches trusted main. - The runner is added to `policy_paths` and covered by a policy-change regression. - Comments in the touched files are in English. Co-Authored-By: Claude Opus 5.5 Signed-off-by: GUOGUO --- .github/knowledge-model-policy.json | 1 + docs/knowledge-model-policy.md | 44 +++--- tests/test_knowledge_model_policy.py | 129 +++++++++++++++- tools/check_knowledge_model_policy.py | 212 ++++++++++++++++---------- 4 files changed, 278 insertions(+), 108 deletions(-) diff --git a/.github/knowledge-model-policy.json b/.github/knowledge-model-policy.json index a641260a7..f32c779e0 100644 --- a/.github/knowledge-model-policy.json +++ b/.github/knowledge-model-policy.json @@ -18,6 +18,7 @@ ".github/knowledge-model-policy.json", ".github/workflows/knowledge-model-policy.yml", "tools/check_knowledge_model_policy.py", + "tools/run_knowledge_model_policy.sh", "docs/knowledge-model-policy.md" ], "models": { diff --git a/docs/knowledge-model-policy.md b/docs/knowledge-model-policy.md index c6e5e56bc..90d1bfcb5 100644 --- a/docs/knowledge-model-policy.md +++ b/docs/knowledge-model-policy.md @@ -129,26 +129,35 @@ branches and API pages, and requires a unique head both before evaluation and before publishing its result. Close the extra PR or use a distinct commit before retrying. An exception cannot bypass head uniqueness. A closed peer no longer shares the head: the remaining open PR is evaluated on its own metadata and -receives its own verdict. Each evaluation round reads the full open-PR collection -once before evaluation and once before publication, with API pagination in both -scans. Results are published only from that fresh scan, and only for a target that -is still open against the policy branch with unchanged head, base and body; -applicable approvals are reread before publication. The collection request count -scales with its page count, not page count times PRs. +receives its own verdict. The checker reads the full open-PR collection once +before evaluation and once before publication, with API pagination in both scans. +Results are published only from that fresh scan, and only for a target that is +still open against the policy branch with an unchanged head; applicable approvals +are reread before publication. The collection request count scales with its page +count, not page count times PRs. ## Lifecycle and reruns A merged or closed PR that is no longer part of the sweep receives no new status; its earlier verdict stays historical. Publication never targets a captured head -for work whose base advanced, whose PR changed, or whose approvals were edited -during evaluation. When the trusted base advances mid-sweep, the checker -re-evaluates open targets against the current base for a bounded number of rounds -inside the same run. If work is still moving, the run publishes no verdict for it, -reports the target as `deferred`, and exits `2`; trusted -`tools/run_knowledge_model_policy.sh` then refetches `main` and runs the checker -again from the current checkout. After its own attempt budget, the workflow run -fails. Open PRs remain fail-closed in every case: stale work is never published as -compliant, and an old green is never retained in place of a valid evaluation. +for work whose base advanced or whose head changed. One evaluation pass is +followed by a publication pass that re-reads the target. If the head, base, body +or approvals moved, or if head ownership changed because a peer closed, the checker +reassesses once against the current metadata and the current open-PR collection +before writing. A green is published only from a verified premise; when the +open-PR collection cannot be read, an otherwise compliant target is recorded as +`unverified` with a failed status instead of keeping its old success. If the target +itself cannot be re-read, no status is written to the captured head because it may +already be merged; the run reports that the old status could not be invalidated, +which is a real limitation, not a fail-closed result. + +When the trusted base advances mid-sweep, the checker returns a retryable result +promptly instead of re-evaluating against its own stale checkout. It exits `2`, +and trusted `tools/run_knowledge_model_policy.sh` refetches `main` and runs the +checker again from the current code and policy. The runner is the single retry +owner with a bounded budget; after exhaustion the workflow run fails. Open PRs +remain fail-closed in every case: stale work is never published as compliant, and +an old green is never retained in place of a valid evaluation. Each result carries `kind` alongside the status `state`: @@ -156,13 +165,14 @@ Each result carries `kind` alongside the status `state`: | --- | --- | | `compliant` | Declaration and approvals satisfy the trusted policy | | `noncompliant` | Genuine rejection: declaration, approval or head eligibility (including a shared head) | -| `evaluator-error` | GitHub API, incomplete evidence or unsupported lifecycle input; never a rejection | +| `evaluator-error` | GitHub API or incomplete evidence; never a rejection | +| `unverified` | Compliance could not be published without unverified lifecycle or ownership metadata | | `deferred` | Work could not be completed in this run; rerun required | | `closed` | Target left the sweep; no status was written | | `publication-failed` | Status write failed; the verdict was not recorded | The final JSON summary counts each kind. Exit codes: `0` all published results -compliant, `1` at least one failing kind, `2` unfinished work (rerun). A rerun +compliant, `1` at least one failing kind, `2` unfinished work to rerun. A rerun never overrides a recorded rejection. The workflow handles PR open/reopen/close/commits/body-or-base edits on any target diff --git a/tests/test_knowledge_model_policy.py b/tests/test_knowledge_model_policy.py index ed10f5029..da4ebeea5 100644 --- a/tests/test_knowledge_model_policy.py +++ b/tests/test_knowledge_model_policy.py @@ -261,7 +261,7 @@ def test_policy_relaxation_uses_base_policy_and_requires_separate_approval(polic result = assess(item, {KNOWLEDGE, CONTROL}, policy) assert len(result["errors"]) == 2 assert result["state"] == "failure" - # PR 内容中声称的 policy 不是 assess 的可信输入。 + # A policy claimed in the PR body is not a trusted input to assess. item["body"] += '\nProposed policy: {"protected_paths": [], "models": {}}' assert assess(item, {KNOWLEDGE, CONTROL}, policy)["state"] == "failure" result = assess(pr(), {CONTROL}, policy, @@ -294,7 +294,7 @@ def test_workflow_never_executes_pr_code_and_refreshes_metadata_and_base(): value = yaml.safe_load( (ROOT / ".github/workflows/knowledge-model-policy.yml").read_text(encoding="utf-8") ) - events = value[True] # PyYAML 的 YAML 1.1 将 on 解析为布尔键。 + events = value[True] # PyYAML's YAML 1.1 parses `on` as a boolean key. assert set(events) == {"pull_request_target", "issue_comment", "push", "workflow_dispatch"} assert "edited" in events["pull_request_target"]["types"] assert "closed" in events["pull_request_target"]["types"] @@ -323,9 +323,10 @@ def test_workflow_never_executes_pr_code_and_refreshes_metadata_and_base(): @pytest.fixture def api_fixture(): - # 本机 HTTP fixture 使用真实 GitHub adapter，记录实际 POST 的共享 status。 + # A local HTTP fixture drives the real GitHub adapter and records the shared status POSTs. state = {"prs": {123: pr()}, "comments": {}, "statuses": [], "requests": [], - "before_request": None, "errors": [], "base": BASE} + "before_request": None, "errors": [], "base": BASE, "fail": lambda route: False, + "pulls_scans": 0} class Handler(BaseHTTPRequestHandler): def log_message(self, *args): @@ -345,9 +346,13 @@ def do_GET(self): parsed = urlsplit(self.path) route = parsed.path.removeprefix("/repos/example/repository/") state["requests"].append(("GET", route)) + if state["fail"](route): + self.send_error(503) + return if state["before_request"]: state["before_request"](route) if route == "pulls": + state["pulls_scans"] += 1 values = [p for p in state["prs"].values() if p["state"] == "open"] query = parse_qs(parsed.query) if "base" in query: @@ -424,7 +429,7 @@ def test_api_exception_for_one_pr_cannot_green_another(api_fixture, policy, orde value = declared([attribution("unknown", "unknown")]) value["exception"] = {"paths": [KNOWLEDGE], "reason": "Bounded exception"} state["prs"] = {123: pr(value), 124: {**pr(value), "number": 124}} - # 两处都有同一个 comment，但其中的批准明确只覆盖 PR 123。 + # Both PRs carry the same comment, but that approval covers PR 123 only. state["comments"] = {123: [approval({KNOWLEDGE})], 124: [approval({KNOWLEDGE})]} assert all(r["state"] == "failure" for r in check_prs(api, policy, BASE, False, order)) assert latest_status(state)["state"] == "failure" @@ -507,7 +512,7 @@ def test_api_batch_bounds_open_list_requests_across_pages(api_fixture, policy): results = check_prs(api, policy, BASE, False) assert {r["pull_request"] for r in results} == set(range(120, 126)) assert all(r["state"] == "success" for r in results) - assert state["requests"].count(("GET", "pulls")) == 4 # 两次扫描，每次两页。 + assert state["requests"].count(("GET", "pulls")) == 4 # Two scans, two pages each. @pytest.mark.parametrize("change", ["body", "approval"]) @@ -607,7 +612,117 @@ def test_api_batch_separates_rejection_from_lifecycle_outcomes(api_fixture, poli assert all(s["sha"] != "d" * 40 for s in state["statuses"]) summary = summarize(results) assert summary == {"kind_counts": {"compliant": 1, "noncompliant": 1, "evaluator-error": 0, - "deferred": 0, "closed": 1, "publication-failed": 0}, + "unverified": 0, "deferred": 0, "closed": 1, + "publication-failed": 0, "invalidation-failed": 0}, "results": 3, "failed": 1, "rerun": False} assert exit_code(results) == EXIT_NONCOMPLIANT assert exit_code([r for r in results if r["kind"] == "compliant"]) == EXIT_COMPLIANT + + +@pytest.mark.parametrize("mutation", ["body", "approval", "head", "metadata-error"]) +def test_api_old_green_cannot_survive_a_broken_publication_read(api_fixture, policy, mutation): + api, state = api_fixture + if mutation == "approval": + value = declared([attribution("unknown", "unknown")]) + value["exception"] = {"paths": [KNOWLEDGE], "reason": "Bounded exception"} + state["prs"][123] = pr(value) + state["comments"][123] = [approval({KNOWLEDGE})] + assert check_pr(api, 123, policy, BASE, False)["state"] == "success" + assert latest_status(state)["state"] == "success" + # The declaration becomes invalid, and the publication-time collection read fails while + # individual reads and status POST still work. + state["pulls_scans"] = 0 + if mutation == "body": + state["prs"][123]["body"] = None + elif mutation == "approval": + state["comments"][123] = [] + if mutation == "head": + # The head moves after the evaluation pass, before publication re-reads it. + def advance(route): + if route == "pulls" and state["pulls_scans"] == 1: + state["prs"][123]["head"] = {"sha": "d" * 40} + state["before_request"] = advance + state["fail"] = lambda route: False + else: + state["fail"] = lambda route: route == "pulls" and state["pulls_scans"] >= 1 + results = check_prs(api, policy, BASE, False, [123]) + head = state["prs"][123]["head"]["sha"] + if mutation == "head": + # The old head keeps its historical green; the new head has no verdict until a rerun. + assert results[0]["kind"] == "deferred" + assert latest_status(state)["state"] == "success" + assert all(s["sha"] != head for s in state["statuses"]) + assert exit_code(results) == EXIT_RERUN + return + # The invariant under test: the effective status of the current head is not the old green. + assert results[0]["state"] == "failure" + assert latest_status(state, head)["state"] == "failure" + if mutation == "metadata-error": + assert results[0]["kind"] == "unverified" + assert exit_code(results) == EXIT_RERUN + else: + assert results[0]["kind"] == "noncompliant" + assert exit_code(results) == EXIT_NONCOMPLIANT + + +def test_api_old_green_cannot_survive_metadata_mutation_and_exhausted_retries( + api_fixture, policy): + api, state = api_fixture + assert check_pr(api, 123, policy, BASE, False)["state"] == "success" + + def mutate_premise(route): + if route == "pulls": + state["prs"][123]["body"] = None + + state["before_request"] = mutate_premise + results = check_prs(api, policy, BASE, False, [123]) + # The first successful evaluation is superseded; the reassessment against current metadata + # records the real rejection instead of leaving the old green in place. + assert [r["kind"] for r in results] == ["noncompliant"] + assert latest_status(state)["state"] == "failure" + assert exit_code(results) == EXIT_NONCOMPLIANT + + +def test_api_peer_closing_during_the_same_sweep_reassesses_the_survivor(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = {**pr(), "number": 124} + + def close_peer(route): + # The peer leaves after it was seen sharing the head in the evaluation pass and before + # the publication-pass collection read completes. + if route == "pulls" and state["read"]: + state["prs"][123]["state"] = "closed" + + state["read"] = [] + original = api.get + + def tracking_get(path): + if path == "pulls/123": + state["read"].append(path) + return original(path) + + api.get = tracking_get + state["before_request"] = close_peer + results = check_prs(api, policy, BASE, False, [123, 124]) + kinds = {r["pull_request"]: r["kind"] for r in results} + # The staged shared-head refusal is invalidated: the surviving PR is assessed on its own. + assert kinds == {123: "closed", 124: "compliant"} + assert latest_status(state)["state"] == "success" + + +def test_api_real_shared_head_still_blocks_green_across_bases(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = {**pr(), "number": 124, "base": {"ref": "release", "sha": BASE}} + results = check_prs(api, policy, BASE, False, [123]) + assert [r["kind"] for r in results] == ["noncompliant"] + assert all(s["state"] != "success" for s in state["statuses"]) + + +def test_policy_runner_is_a_protected_control_surface(policy): + runner = "tools/run_knowledge_model_policy.sh" + assert runner in json.loads((ROOT / CONTROL).read_text(encoding="utf-8"))["policy_paths"] + item = pr() + assert assess(item, {runner}, policy)["state"] == "failure" + approved_item = assess(item, {runner}, policy, + [approval({runner}, kind="policy-change")], {"contributor"}) + assert approved_item["state"] == "success" diff --git a/tools/check_knowledge_model_policy.py b/tools/check_knowledge_model_policy.py index 18ee0eb55..55555faaf 100644 --- a/tools/check_knowledge_model_policy.py +++ b/tools/check_knowledge_model_policy.py @@ -14,17 +14,18 @@ APPROVAL = "knowledge-policy-approval" SHA = re.compile(r"[0-9a-f]{40}") ROOT = Path(__file__).resolve().parents[1] -# Exit codes: published results are final, a rejection is final, but stale work must be rerun. +# Exit codes: 0 all published verdicts comply, 1 a recorded failure, 2 unfinished work to rerun. EXIT_COMPLIANT = 0 EXIT_NONCOMPLIANT = 1 EXIT_RERUN = 2 -# Bounded in-run re-evaluation before the run reports the work as unfinished. -ATTEMPTS = 3 API_ERRORS = (ValueError, KeyError, TypeError, OSError) # Outcome kinds, reported separately so evaluator noise is never read as a rejection. -KINDS = ("compliant", "noncompliant", "evaluator-error", "deferred", "closed", - "publication-failed") -FAILING_KINDS = {"noncompliant", "evaluator-error", "deferred", "publication-failed"} +KINDS = ("compliant", "noncompliant", "evaluator-error", "unverified", "deferred", "closed", + "publication-failed", "invalidation-failed") +FAILING_KINDS = {"noncompliant", "evaluator-error", "unverified", "deferred", + "publication-failed", "invalidation-failed"} +# Only the outer runner retries; these kinds ask it for a fresh trusted checkout. +RERUN_KINDS = {"unverified", "deferred", "publication-failed", "invalidation-failed"} def summarize(results): @@ -35,7 +36,7 @@ def summarize(results): def exit_code(results): - # Unfinished (rerun) work keeps open PRs fail-closed; a real rejection alone exits 1. + # Unfinished work keeps open PRs fail-closed and asks for the single retry owner. if any(result["rerun"] for result in results): return EXIT_RERUN return EXIT_NONCOMPLIANT if any(result["kind"] in FAILING_KINDS for result in results) \ @@ -173,7 +174,7 @@ def approved(comments, policy, contributors, number, head, kind, paths, reason=N if login in owners and re.search(rf"^```{APPROVAL}[ \t]*\r?$", comment.get("body") or "", re.M): latest[login] = comment - # 只信任维护者账号的最新批准声明；提交者自己的文字不能提供授权。 + # Trust only the owner account's latest declaration; the submitter's own text grants nothing. for login, comment in latest.items(): if login in {name.casefold() for name in contributors}: continue @@ -204,7 +205,7 @@ def approved(comments, policy, contributors, number, head, kind, paths, reason=N if matches: return True except (ValueError, TypeError): - # 无效批准不能回退到同一账号的历史批准。 + # An invalid approval cannot fall back to that account's earlier approvals. continue return False @@ -297,18 +298,10 @@ def snapshot(pr): pr.get("body"), pr["changed_files"], pr["state"]) -def require_unique_head(prs, number, head): - # Commit status 由 SHA/context 共享；其他 base 的 open PR 也不能复用它。 - peers = {pr["number"] for pr in prs - if pr["head"]["sha"] == head} - if peers != {number}: - raise ValueError("Head must belong to exactly one open PR; shared heads are ineligible") - - def deferred(reason): # Not a verdict: the target must be re-evaluated before anything is published. return {"state": "failure", "kind": "deferred", "description": "Evaluation deferred", - "errors": [reason], "retry": True} + "errors": [reason]} def evaluation_failure(error): @@ -317,13 +310,30 @@ def evaluation_failure(error): else str(error) if isinstance(error, ValueError) else "Invalid GitHub API response") return {"state": "failure", "kind": "evaluator-error", - "description": "Policy evaluation failed", "errors": [message], "retry": True} + "description": "Policy evaluation failed", "errors": [message]} + + +def head_owner(prs, number, head): + # Commit status is per SHA/context, so ownership must be unique across all open PRs. + peers = {pr["number"] for pr in prs if pr["head"]["sha"] == head} + return None if peers == {number} else \ + "Head must belong to exactly one open PR; shared heads are ineligible" + + +def require_unique_head(prs, number, head): + reason = head_owner(prs, number, head) + if reason is not None: + raise ValueError(reason) + + +def closed_result(description): + return {"state": "success", "kind": "closed", "description": description, "errors": []} def refused(reason): # Deterministic refusal by admission metadata, not an evaluator failure; still fail-closed. return {"state": "failure", "kind": "noncompliant", - "description": "Eligibility refused", "errors": [reason], "retry": False} + "description": "Eligibility refused", "errors": [reason]} def read_pr_result(api, pr, policy, policy_sha, open_prs): @@ -380,7 +390,8 @@ def read_pr_result(api, pr, policy, policy_sha, open_prs): def admission_snapshot(pr): - # list PR API 不含 changed_files；head/base/body/state 相同才可复用已验证的文件列表。 + # The list PR API omits changed_files; reuse the verified file list only for identical + # head, base, body and state. return (pr["head"]["sha"], pr["base"]["ref"], pr["base"]["sha"], pr.get("body"), pr["state"]) @@ -389,78 +400,111 @@ def addressable(pr, policy): return pr["state"] == "open" and pr["base"]["ref"] == policy["base_branch"] +def lifecycle_result(result, scan_error): + # Invalidate an old success when the current declaration was not validly assessed. The + # check still publishes a verdict; only an unknowable lifecycle is reported without a write. + if result["state"] != "success": + return {**result, "rerun": result["kind"] in RERUN_KINDS} + if scan_error is not None: + return {"state": "failure", "kind": "unverified", "rerun": True, + "description": "Lifecycle metadata unavailable", + "errors": [evaluation_failure(scan_error)["errors"][0]]} + return {**result, "rerun": False} + + def check_prs(api, policy, policy_sha, dry_run, numbers=None): - open_prs = api.pages("pulls?state=open") + scan_error = None + try: + open_prs = api.pages("pulls?state=open") + except API_ERRORS as error: + open_prs, scan_error = [], error if numbers is None: + if scan_error is not None: + # Without a listing there is no target set; the runner retries with a fresh base. + return [{"pull_request": 0, "head_sha": "", "policy_sha": policy_sha, + "dry_run": dry_run, "rerun": True, "state": "failure", + "kind": "unverified", "description": "Lifecycle metadata unavailable", + "errors": [evaluation_failure(scan_error)["errors"][0]]}] numbers = [pr["number"] for pr in open_prs if addressable(pr, policy)] - published = {} - retry = list(numbers) - # Results are published only from a fresh scan, and only for a target that still matches - # the evaluated metadata. Stale or moved work is re-evaluated instead of being written to - # a captured head; a closed target receives no publication at all. - for attempt in range(ATTEMPTS): - staged, retry_next = [], [] - for number in retry: - pr = api.get(f"pulls/{number}") - head = pr["head"]["sha"] - if not SHA.fullmatch(head): - raise ValueError("Invalid PR head") - if not addressable(pr, policy): - published[number] = {"head": head, "state": "success", "kind": "closed", - "description": "Not open against the policy branch", - "errors": [], "retry": False} - continue - # No pending write: a run that stops early must not leave a stale pending status - # on a head that merges or otherwise leaves the sweep. - result, comments = read_pr_result(api, pr, policy, policy_sha, open_prs) - if result.pop("retry", False) and attempt + 1 < ATTEMPTS: - retry_next.append(number) - else: - staged.append((pr, result, comments)) + # One evaluation pass. Stale work is returned as deferred so the outer runner, which can + # refetch trusted main, owns retries; this checker never repairs its own loaded policy. + records = {} + for number in numbers: + pr = api.get(f"pulls/{number}") + head = pr["head"]["sha"] + if not SHA.fullmatch(head): + raise ValueError("Invalid PR head") + if not addressable(pr, policy): + records[number] = {"head": head, "snapshot": None, "comments": [], + "result": closed_result("Not open against the policy branch")} + continue + result, comments = read_pr_result(api, pr, policy, policy_sha, open_prs) + records[number] = {"head": head, "snapshot": admission_snapshot(pr), + "comments": comments, "result": result, + "ownership": head_owner(open_prs, number, head) + if scan_error is None else None} + # One publication pass. Ownership and metadata are read fresh here; a target that left the + # sweep is dropped, and a changed premise is reassessed once before any write. + scan_error = None + try: open_prs = api.pages("pulls?state=open") - current = {pr["number"]: pr for pr in open_prs} - for pr, result, comments in staged: - number, head = pr["number"], pr["head"]["sha"] - live = current.get(number) - if live is None or not addressable(live, policy): - published[number] = {"head": head, "state": "success", "kind": "closed", - "description": "Closed before publication", "errors": [], - "retry": False} - continue - if admission_snapshot(live) != admission_snapshot(pr): - retry_next.append(number) - continue - if comments and comments != api.pages(f"issues/{number}/comments"): - # Approvals read for this verdict must still be current at publication. - retry_next.append(number) - continue + except API_ERRORS as error: + open_prs, scan_error = [], error + published = {} + for number, record in records.items(): + head = record["head"] + try: + live = api.get(f"pulls/{number}") + except API_ERRORS as error: + # The target may be closed, so no failure is written to `head`; the report says the + # old status could not be invalidated, which is not the same as fail-closed. + published[number] = {"state": "failure", "kind": "unverified", "rerun": True, + "head": head, "description": "Lifecycle check failed", + "errors": [evaluation_failure(error)["errors"][0]]} + continue + if not addressable(live, policy): + published[number] = {**closed_result("Closed before publication"), "head": head, + "rerun": False} + continue + if live["head"]["sha"] != head: + # The old head carries no current claim; the new head has no status yet. + published[number] = {"state": "failure", "kind": "deferred", "rerun": True, + "head": head, "description": "Head advanced before publication", + "errors": ["Head advanced before publication; rerun required"]} + continue + result = record["result"] + premise = admission_snapshot(live) != record["snapshot"] + if record["comments"] and record["comments"] != api.pages(f"issues/{number}/comments"): + premise = True + ownership = None if scan_error is not None else head_owner(open_prs, number, head) + if premise or ownership != record["ownership"]: + # Reassess against current metadata and the current open-PR collection: a closed + # peer can end head sharing, and a moved body or approval changes the verdict. + result, _ = read_pr_result(api, live, policy, policy_sha, + open_prs if open_prs else [live]) + ownership = (None if scan_error is not None + else head_owner(open_prs, number, head)) + if result["state"] == "success" and ownership is not None: + result = {"state": "failure", "kind": "unverified", + "description": "Head ownership unverified", "errors": [ownership]} + else: + result = lifecycle_result(result, scan_error) + result["rerun"] = result["kind"] in RERUN_KINDS + if not dry_run: try: - require_unique_head(open_prs, number, head) - except ValueError as error: - result = refused(str(error)) - # Only a fresh, fully revalidated verdict reaches a head; a shared head is refused - # above, so each head is written at most once per round. - if not dry_run: - try: - api.status(head, result["state"], result["description"]) - except API_ERRORS as error: - result = {**result, "kind": "publication-failed", - "errors": [*result["errors"], - evaluation_failure(error)["errors"][0]]} - published[number] = {"head": head, **result} - retry = sorted(set(retry_next) - set(published)) - if not retry: - break - # After the attempt budget, an unfinished target stays failed and asks for a fresh run. - for number in retry: - published.setdefault(number, {"head": "", "state": "failure", "kind": "deferred", - "description": "Evaluation incomplete", "errors": []}) + api.status(head, result["state"], result["description"]) + except API_ERRORS as error: + result = {**result, "kind": "publication-failed", "state": "failure", + "errors": [*result["errors"], + evaluation_failure(error)["errors"][0]]} + published[number] = {**result, "head": head, + "rerun": result["kind"] in RERUN_KINDS} results = [] for number in sorted(published): item = published[number] results.append({"pull_request": number, "head_sha": item["head"], "policy_sha": policy_sha, "dry_run": dry_run, - "rerun": item["kind"] == "deferred", "state": item["state"], + "rerun": item["rerun"], "state": item["state"], "kind": item["kind"], "description": item["description"], "errors": item["errors"]}) return results @@ -502,7 +546,7 @@ def main(argv=None): numbers = [args.pull_request] else: event = json.loads(Path(args.event_path).read_text(encoding="utf-8")) - # 所有触发共用一个 writer；待运行事件被合并时仍覆盖全部 open PR。 + # Every trigger shares one writer, so a superseded queued event still covers all open PRs. numbers = None if refresh_event(event, os.environ["GITHUB_EVENT_NAME"]) else [] results = check_prs(api, policy, args.policy_sha, args.dry_run, numbers) for result in results: From 30c31d5ab9fdab425dd14fbd13661b8cfa515214 Mon Sep 17 00:00:00 2001 From: GUOGUO Date: Thu, 22 Oct 2026 16:54:04 +0800 Subject: [PATCH 3/3] feat: separate per-PR admission from sweep execution Revision request on the previous head: - Drop the policy-change approval gate. policy_paths now classifies and reports control-path changes only; a policy-only change neither fails for want of an approval nor is excused by one. Exception approvals, revocation, scope and head binding are unchanged, and a historical policy-change comment can never grant an exception. - Give every required read an outcome instead of letting an exception escape: base, target, approval and file reads now return an explicit retryable result. The runner stays the single bounded retry owner. - Preserve lifecycle observations from reassessment. A close or head change seen while re-evaluating is reported as `closed` or `advanced` and blocks publication to the captured head instead of becoming a failure write. - Separate each PR's admission verdict from the sweep's execution: one PR's rejection keeps that PR red without failing the run, the summary lists the rejected PRs, and evaluator, lifecycle or publication errors still fail the run explicitly. Co-Authored-By: Claude Opus 5.5 Signed-off-by: GUOGUO --- docs/knowledge-model-policy.md | 21 ++- tests/test_knowledge_model_policy.py | 205 +++++++++++++++++++++++--- tools/check_knowledge_model_policy.py | 100 ++++++++++--- tools/run_knowledge_model_policy.sh | 7 +- 4 files changed, 286 insertions(+), 47 deletions(-) diff --git a/docs/knowledge-model-policy.md b/docs/knowledge-model-policy.md index 90d1bfcb5..5791b2cd4 100644 --- a/docs/knowledge-model-policy.md +++ b/docs/knowledge-model-policy.md @@ -108,10 +108,14 @@ Unlinked Git identities cannot be inferred by CI. PR number, head, exact scope require renewed approval. Exceptions cannot excuse malformed/missing attribution; explicitly unknown settings may be considered by the approval owner. -Changes to the config, checker, metadata workflow or this document need an -independent owner approval of kind `policy-change`, listing exactly the changed -policy-control paths and a specific reason. This is separate from author -eligibility. One comment may contain both kinds in `approvals`. Checks use the +`policy_paths` classifies changes to this mechanism — the policy config, this +checker, the metadata workflow, the runner and this document — and reports them in +each result. Classification is not a gate: a policy-only change does not fail for +lack of a separate approval, and passing it is not a statement about content +quality. Nothing in this mechanism grants an exception: an approval of kind +`policy-change`, including historical comments, cannot excuse protected knowledge +changes or substitute for knowledge author admission. One comment may still carry +both kinds in `approvals`; only the `exception` kind is evaluated. Checks use the target branch's policy, never the PR's proposed replacement. Each authorized account's latest comment containing the approval fence replaces its earlier declarations. Use `decision: "revoke"` to revoke its grants. Comment @@ -171,9 +175,12 @@ Each result carries `kind` alongside the status `state`: | `closed` | Target left the sweep; no status was written | | `publication-failed` | Status write failed; the verdict was not recorded | -The final JSON summary counts each kind. Exit codes: `0` all published results -compliant, `1` at least one failing kind, `2` unfinished work to rerun. A rerun -never overrides a recorded rejection. +The final JSON summary counts each kind, lists the `noncompliant_pull_requests`, and +states the sweep `execution` as `ok`, `failed` or `unfinished`. Exit codes: `0` +the sweep completed (any number of individual rejections are their own red +statuses), `1` an execution error, `2` unfinished work to rerun. A rerun never +overrides a recorded rejection. A rejection belongs to its own PR: one PR's +rejection neither greens nor fails another PR's status. The workflow handles PR open/reopen/close/commits/body-or-base edits on any target branch (a PR against another base can share a main PR's head), approval-comment diff --git a/tests/test_knowledge_model_policy.py b/tests/test_knowledge_model_policy.py index da4ebeea5..3debe1cc7 100644 --- a/tests/test_knowledge_model_policy.py +++ b/tests/test_knowledge_model_policy.py @@ -12,7 +12,6 @@ APPROVAL, DECLARATION, EXIT_COMPLIANT, - EXIT_NONCOMPLIANT, EXIT_RERUN, GitHub, approved, @@ -256,18 +255,32 @@ def test_newest_approval_revoke_edit_or_invalid_block_does_not_reuse_history(pol assert not approved([], policy, set(), 123, HEAD, "exception", {KNOWLEDGE}) -def test_policy_relaxation_uses_base_policy_and_requires_separate_approval(policy): +def test_policy_paths_are_reported_not_gated_by_a_separate_approval(policy): item = pr(declared([attribution("unapproved-model")])) result = assess(item, {KNOWLEDGE, CONTROL}, policy) - assert len(result["errors"]) == 2 + # The knowledge part still fails; the policy part adds no gate of its own. + assert len(result["errors"]) == 1 assert result["state"] == "failure" + assert result["policy_paths"] == [CONTROL] # A policy claimed in the PR body is not a trusted input to assess. item["body"] += '\nProposed policy: {"protected_paths": [], "models": {}}' assert assess(item, {KNOWLEDGE, CONTROL}, policy)["state"] == "failure" - result = assess(pr(), {CONTROL}, policy, - [approval({CONTROL}, kind="policy-change")], {"contributor"}) - assert result["state"] == "success" - assert assess(pr(), {CONTROL}, policy)["state"] == "failure" + # A policy-only change neither needs a policy-change approval nor is excused from it. + assert assess(pr(), {CONTROL}, policy)["state"] == "success" + assert assess(pr(), {CONTROL}, policy, + [approval({CONTROL}, kind="policy-change")], + {"contributor"})["state"] == "success" + assert assess(pr(), {CONTROL}, policy)["policy_paths"] == [CONTROL] + + +def test_policy_change_comment_never_grants_an_exception(policy): + value = declared([attribution("unknown", "unknown")]) + value["exception"] = {"paths": [KNOWLEDGE], "reason": "Bounded exception"} + item = pr(value) + stale = approval({KNOWLEDGE}, kind="policy-change", reason="Bounded exception") + assert assess(item, {KNOWLEDGE}, policy, [stale], {"contributor"})["state"] == "failure" + assert assess(item, {KNOWLEDGE}, policy, [approval({KNOWLEDGE})], + {"contributor"})["state"] == "success" def test_snapshot_refreshes_on_body_head_and_base_changes(): @@ -613,9 +626,12 @@ def test_api_batch_separates_rejection_from_lifecycle_outcomes(api_fixture, poli summary = summarize(results) assert summary == {"kind_counts": {"compliant": 1, "noncompliant": 1, "evaluator-error": 0, "unverified": 0, "deferred": 0, "closed": 1, - "publication-failed": 0, "invalidation-failed": 0}, - "results": 3, "failed": 1, "rerun": False} - assert exit_code(results) == EXIT_NONCOMPLIANT + "advanced": 0, "publication-failed": 0, + "invalidation-failed": 0}, + "results": 3, "failed": 1, "noncompliant_pull_requests": [124], + "execution": "ok", "rerun": False} + # A per-PR rejection keeps that PR red; it does not fail the sweep. + assert exit_code(results) == EXIT_COMPLIANT assert exit_code([r for r in results if r["kind"] == "compliant"]) == EXIT_COMPLIANT @@ -661,8 +677,9 @@ def advance(route): assert results[0]["kind"] == "unverified" assert exit_code(results) == EXIT_RERUN else: + # A superseded declaration is this PR's own rejection, not a sweep failure. assert results[0]["kind"] == "noncompliant" - assert exit_code(results) == EXIT_NONCOMPLIANT + assert exit_code(results) == EXIT_COMPLIANT def test_api_old_green_cannot_survive_metadata_mutation_and_exhausted_retries( @@ -680,7 +697,7 @@ def mutate_premise(route): # records the real rejection instead of leaving the old green in place. assert [r["kind"] for r in results] == ["noncompliant"] assert latest_status(state)["state"] == "failure" - assert exit_code(results) == EXIT_NONCOMPLIANT + assert exit_code(results) == EXIT_COMPLIANT def test_api_peer_closing_during_the_same_sweep_reassesses_the_survivor(api_fixture, policy): @@ -716,13 +733,167 @@ def test_api_real_shared_head_still_blocks_green_across_bases(api_fixture, polic results = check_prs(api, policy, BASE, False, [123]) assert [r["kind"] for r in results] == ["noncompliant"] assert all(s["state"] != "success" for s in state["statuses"]) + # The rejected PR is red; the sweep itself completed. + assert exit_code(results) == EXIT_COMPLIANT + + +def test_api_compliant_and_noncompliant_prs_are_judged_separately(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = separate_pr(124, "c" * 40) + state["prs"][124]["body"] = None + results = check_prs(api, policy, BASE, False, [123, 124]) + kinds = {r["pull_request"]: r["kind"] for r in results} + # A stays green, B stays red, and B's rejection does not turn A's check into an error. + assert kinds == {123: "compliant", 124: "noncompliant"} + assert latest_status(state)["state"] == "success" + assert latest_status(state, "c" * 40)["state"] == "failure" + summary = summarize(results) + assert summary["noncompliant_pull_requests"] == [124] + assert summary["execution"] == "ok" + assert exit_code(results) == EXIT_COMPLIANT -def test_policy_runner_is_a_protected_control_surface(policy): +def test_api_sweep_errors_still_fail_the_run(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = separate_pr(124, "c" * 40) + state["fail"] = lambda route: route == "pulls/124/files" + results = check_prs(api, policy, BASE, False, [123, 124]) + kinds = {r["pull_request"]: r["kind"] for r in results} + assert kinds == {123: "compliant", 124: "unverified"} + summary = summarize(results) + # An evaluator error is never reported as a per-PR verdict or as a clean sweep; it is + # either unfinished (retryable) or failed, never "ok". + assert summary["execution"] in {"failed", "unfinished"} + assert summary["noncompliant_pull_requests"] == [] + assert exit_code(results) != EXIT_COMPLIANT + + +def test_policy_runner_is_reported_as_a_control_path(policy): runner = "tools/run_knowledge_model_policy.sh" assert runner in json.loads((ROOT / CONTROL).read_text(encoding="utf-8"))["policy_paths"] item = pr() - assert assess(item, {runner}, policy)["state"] == "failure" - approved_item = assess(item, {runner}, policy, - [approval({runner}, kind="policy-change")], {"contributor"}) - assert approved_item["state"] == "success" + result = assess(item, {runner}, policy) + # Reporting only: the runner is classified as a control path and does not become a gate. + assert result["state"] == "success" + assert result["policy_paths"] == [runner] + assert result["description"] == "Declaration compliance only" + + +def test_api_failed_approval_read_on_publication_does_not_escape(api_fixture, policy): + api, state = api_fixture + value = declared([attribution("unknown", "unknown")]) + value["exception"] = {"paths": [KNOWLEDGE], "reason": "Bounded exception"} + state["prs"][123] = pr(value) + state["comments"][123] = [approval({KNOWLEDGE})] + assert check_pr(api, 123, policy, BASE, False)["state"] == "success" + # The approval read required by the publication premise now fails. No exception escapes, + # nothing is published from stale approvals, and the result asks for the runner retry. + reads = {"count": 0} + + def fail_second_read(path): + if path == "issues/123/comments": + reads["count"] += 1 + return reads["count"] >= 2 + return False + + state["fail"] = fail_second_read + results = check_prs(api, policy, BASE, False, [123]) + assert [r["kind"] for r in results] == ["unverified"] + assert results[0]["rerun"] is True + assert exit_code(results) == EXIT_RERUN + + +@pytest.mark.parametrize("route", ["pulls/123", "git/ref/heads/main"]) +def test_api_failed_initial_reads_do_not_escape(api_fixture, policy, route): + api, state = api_fixture + state["fail"] = lambda path: path == route + result = check_pr(api, 123, policy, BASE, False) + assert result["state"] == "failure" + assert result["kind"] in {"unverified", "evaluator-error"} + assert result["rerun"] is True + if route == "pulls/123": + # The target could not be read, so nothing is written to a head that may be merged. + assert state["statuses"] == [] + else: + # The target was confirmed open at an unchanged head, so the old success is not kept: + # an explicit evidence-unavailable failure is published. + assert latest_status(state)["state"] == "failure" + + +def test_api_survivor_closing_during_reassessment_publishes_nothing(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = {**pr(), "number": 124} + + def close_peer_and_survivor(route): + # A closes after the shared head was staged; B then closes while it is re-evaluated. + if route == "pulls" and state["read"]: + state["prs"][123]["state"] = "closed" + if route == "pulls/124/files": + state["prs"][124]["state"] = "closed" + + state["read"] = [] + original = api.get + + def tracking_get(path): + if path == "pulls/123": + state["read"].append(path) + return original(path) + + api.get = tracking_get + state["before_request"] = close_peer_and_survivor + results = check_prs(api, policy, BASE, False, [123, 124]) + kinds = {r["pull_request"]: r["kind"] for r in results} + assert kinds == {123: "closed", 124: "closed"} + # Nothing is published to the old shared head, including no failure. + assert state["statuses"] == [] + + +def test_api_head_change_during_reassessment_publishes_nothing(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = {**pr(), "number": 124} + + def advance_survivor(route): + if route == "pulls" and state["read"]: + state["prs"][123]["state"] = "closed" + if route == "pulls/124/files": + state["prs"][124]["head"] = {"sha": "d" * 40} + + state["read"] = [] + original = api.get + + def tracking_get(path): + if path == "pulls/123": + state["read"].append(path) + return original(path) + + api.get = tracking_get + state["before_request"] = advance_survivor + results = check_prs(api, policy, BASE, False, [123, 124]) + kinds = {r["pull_request"]: r["kind"] for r in results} + assert kinds == {123: "closed", 124: "advanced"} + assert all(s["sha"] != "d" * 40 for s in state["statuses"]) + + +def test_rerun_is_requested_without_a_second_inner_retry(api_fixture, policy): + api, state = api_fixture + state["prs"][124] = {**pr(), "number": 124} + state["prs"][124]["head"] = {"sha": "c" * 40} + before = state["pulls_scans"] + results = check_prs(api, policy, BASE, False) + # A single sweep: one collection read per pass, regardless of result kinds. + assert state["pulls_scans"] - before == 2 + assert {r["pull_request"] for r in results} == {123, 124} + assert all(r["rerun"] is False for r in results) + + +def test_api_historical_policy_change_comment_is_inert(api_fixture, policy): + api, state = api_fixture + value = declared([attribution("unknown", "unknown")]) + value["exception"] = {"paths": [KNOWLEDGE], "reason": "Bounded exception"} + state["prs"][123] = pr(value) + state["comments"][123] = [approval({KNOWLEDGE}, kind="policy-change", + reason="Bounded exception")] + result = check_pr(api, 123, policy, BASE, False) + assert result["state"] == "failure" + assert result["kind"] == "noncompliant" + assert latest_status(state)["state"] == "failure" diff --git a/tools/check_knowledge_model_policy.py b/tools/check_knowledge_model_policy.py index 55555faaf..6312d3ef1 100644 --- a/tools/check_knowledge_model_policy.py +++ b/tools/check_knowledge_model_policy.py @@ -21,25 +21,39 @@ API_ERRORS = (ValueError, KeyError, TypeError, OSError) # Outcome kinds, reported separately so evaluator noise is never read as a rejection. KINDS = ("compliant", "noncompliant", "evaluator-error", "unverified", "deferred", "closed", - "publication-failed", "invalidation-failed") -FAILING_KINDS = {"noncompliant", "evaluator-error", "unverified", "deferred", - "publication-failed", "invalidation-failed"} + "advanced", "publication-failed", "invalidation-failed") +# Per-PR verdict: a rejection is this PR's red status, not a failure of the sweep. +REJECTION_KINDS = {"noncompliant"} +# Sweep execution problems, reported as errors and never as a per-PR verdict. +ERROR_KINDS = {"evaluator-error", "unverified", "deferred", "advanced", "publication-failed", + "invalidation-failed"} +FAILING_KINDS = REJECTION_KINDS | ERROR_KINDS # Only the outer runner retries; these kinds ask it for a fresh trusted checkout. -RERUN_KINDS = {"unverified", "deferred", "publication-failed", "invalidation-failed"} +RERUN_KINDS = {"evaluator-error", "unverified", "deferred", "advanced", "publication-failed", + "invalidation-failed"} +# A lifecycle kind forbids publication: the captured head is no longer the current target. +NO_PUBLISH = {"closed", "advanced"} def summarize(results): counts = {kind: sum(result["kind"] == kind for result in results) for kind in KINDS} + rejected = sorted(result["pull_request"] for result in results + if result["kind"] in REJECTION_KINDS) + rerun = any(result["rerun"] for result in results) + error = not rerun and any(result["kind"] in ERROR_KINDS for result in results) return {"kind_counts": counts, "results": len(results), "failed": sum(result["kind"] in FAILING_KINDS for result in results), - "rerun": any(result["rerun"] for result in results)} + "noncompliant_pull_requests": rejected, + "execution": "unfinished" if rerun else "failed" if error else "ok", + "rerun": rerun} def exit_code(results): - # Unfinished work keeps open PRs fail-closed and asks for the single retry owner. + # Separate the sweep's execution from each PR's verdict: one PR's rejection does not fail + # the run, while evaluation, lifecycle and publication errors always do. if any(result["rerun"] for result in results): return EXIT_RERUN - return EXIT_NONCOMPLIANT if any(result["kind"] in FAILING_KINDS for result in results) \ + return EXIT_NONCOMPLIANT if any(result["kind"] in ERROR_KINDS for result in results) \ else EXIT_COMPLIANT @@ -214,9 +228,8 @@ def assess(pr, paths, policy, comments=(), contributors=()): protected = matching(paths, policy["protected_paths"]) controls = matching(paths, policy["policy_paths"]) errors = [] - if controls and not approved(comments, policy, contributors, pr["number"], pr["head"]["sha"], - "policy-change", controls): - errors.append("Policy changes require explicit independent owner approval") + # policy_paths is classification and reporting only. No approval kind gates it, and an old + # policy-change comment can neither excuse this PR nor grant an exception. if protected: try: value = declaration(pr.get("body"), pr["head"]["sha"]) @@ -330,22 +343,42 @@ def closed_result(description): return {"state": "success", "kind": "closed", "description": description, "errors": []} +def advanced_result(head): + # The evaluated head is stale; the new head has no verdict until the runner re-evaluates. + return {"state": "failure", "kind": "advanced", + "description": f"Head advanced from {head[:12]}; rerun required", + "errors": ["Head advanced before publication; rerun required"]} + + def refused(reason): # Deterministic refusal by admission metadata, not an evaluator failure; still fail-closed. return {"state": "failure", "kind": "noncompliant", "description": "Eligibility refused", "errors": [reason]} +def unverified(reason): + # A required read failed. Not a verdict, and the old status must not survive as if current. + return {"state": "failure", "kind": "unverified", "description": "Evidence unavailable", + "errors": [reason]} + + def read_pr_result(api, pr, policy, policy_sha, open_prs): number = pr["number"] head = pr["head"]["sha"] comments = [] + if not addressable(pr, policy): + # A target that left the sweep publishes nothing; its earlier verdict stays historical. + return closed_result("Not open against the policy branch"), comments try: require_unique_head(open_prs, number, head) except ValueError as error: return refused(str(error)), comments base_route = f"git/ref/heads/{policy['base_branch']}" - if api.get(base_route)["object"]["sha"] != policy_sha: + try: + base_sha = api.get(base_route)["object"]["sha"] + except API_ERRORS as error: + return unverified(evaluation_failure(error)["errors"][0]), comments + if base_sha != policy_sha: # The trusted checkout moved: this verdict belongs to no publishable base. return deferred("Trusted policy checkout is stale; rerun on the current base"), comments try: @@ -353,7 +386,6 @@ def read_pr_result(api, pr, policy, policy_sha, open_prs): for page in file_page_numbers(pr["changed_files"])] files = assemble_file_pages(pages, pr["changed_files"]) paths = changed_paths(files, pr["changed_files"]) - controls = matching(paths, policy["policy_paths"]) protected = matching(paths, policy["protected_paths"]) request = None if protected: @@ -363,7 +395,8 @@ def read_pr_result(api, pr, policy, policy_sha, open_prs): pass comments, commits = [], [] contributors = {pr["user"]["login"]} - if controls or request is not None: + if request is not None: + # Exception approvals bind to commit attribution; nothing else needs it. if pr["commits"] > 250: raise ValueError("Commit attribution exceeds the GitHub PR commits API limit") commits = api.pages(f"pulls/{number}/commits") @@ -375,13 +408,17 @@ def read_pr_result(api, pr, policy, policy_sha, open_prs): contributors.add(commit[field]["login"]) comments = api.pages(f"issues/{number}/comments") result = assess(pr, paths, policy, comments, contributors) - # Anything that moved since the snapshot invalidates this verdict; the base ref must - # still match the trusted checkout, or the verdict belongs to no publishable base. - if snapshot(api.get(f"pulls/{number}")) != snapshot(pr): + # Lifecycle observed here is final for this verdict: it must not become a failure write. + live = api.get(f"pulls/{number}") + if not addressable(live, policy): + return closed_result("Closed during evaluation"), comments + if live["head"]["sha"] != head: + return advanced_result(head), comments + if snapshot(live) != snapshot(pr): return deferred("PR changed during evaluation; rerun required"), comments if api.get(base_route)["object"]["sha"] != policy_sha: return deferred("Base advanced during evaluation; rerun required"), comments - if (controls or request is not None) and comments != api.pages(f"issues/{number}/comments"): + if comments and comments != api.pages(f"issues/{number}/comments"): # Re-read approvals that were read, not only approvals that changed the verdict. return deferred("Approval comments changed during evaluation; rerun required"), comments except API_ERRORS as error: @@ -430,12 +467,20 @@ def check_prs(api, policy, policy_sha, dry_run, numbers=None): # refetch trusted main, owns retries; this checker never repairs its own loaded policy. records = {} for number in numbers: - pr = api.get(f"pulls/{number}") + try: + pr = api.get(f"pulls/{number}") + except API_ERRORS as error: + # The target cannot be read, so no status is written to a head that may be merged. + records[number] = {"head": "", "snapshot": None, "comments": [], + "ownership": None, + "result": unverified(evaluation_failure(error)["errors"][0])} + continue head = pr["head"]["sha"] if not SHA.fullmatch(head): raise ValueError("Invalid PR head") if not addressable(pr, policy): records[number] = {"head": head, "snapshot": None, "comments": [], + "ownership": None, "result": closed_result("Not open against the policy branch")} continue result, comments = read_pr_result(api, pr, policy, policy_sha, open_prs) @@ -474,8 +519,15 @@ def check_prs(api, policy, policy_sha, dry_run, numbers=None): continue result = record["result"] premise = admission_snapshot(live) != record["snapshot"] - if record["comments"] and record["comments"] != api.pages(f"issues/{number}/comments"): - premise = True + if record["comments"]: + try: + premise = premise or record["comments"] != api.pages( + f"issues/{number}/comments") + except API_ERRORS as error: + # A required read failed; no verdict may be published from stale approvals. + published[number] = {**unverified(evaluation_failure(error)["errors"][0]), + "head": head, "rerun": True} + continue ownership = None if scan_error is not None else head_owner(open_prs, number, head) if premise or ownership != record["ownership"]: # Reassess against current metadata and the current open-PR collection: a closed @@ -484,11 +536,19 @@ def check_prs(api, policy, policy_sha, dry_run, numbers=None): open_prs if open_prs else [live]) ownership = (None if scan_error is not None else head_owner(open_prs, number, head)) + if result["kind"] in NO_PUBLISH: + # The reassessment observed that this head is no longer the target. + published[number] = {**result, "head": head, + "rerun": result["kind"] in RERUN_KINDS} + continue if result["state"] == "success" and ownership is not None: result = {"state": "failure", "kind": "unverified", "description": "Head ownership unverified", "errors": [ownership]} else: result = lifecycle_result(result, scan_error) + if result["kind"] == "evaluator-error": + # After publication revalidation, a failed read is unavailable evidence. + result = unverified(result["errors"][0]) result["rerun"] = result["kind"] in RERUN_KINDS if not dry_run: try: diff --git a/tools/run_knowledge_model_policy.sh b/tools/run_knowledge_model_policy.sh index 344f72ab4..c8927d340 100644 --- a/tools/run_knowledge_model_policy.sh +++ b/tools/run_knowledge_model_policy.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash # Run the knowledge admission checker against the current trusted main. -# Exit 2 means the trusted base advanced during evaluation: refetch main and evaluate again. +# Exit 2 means the work could not be completed (stale base, unreadable evidence); this runner +# is the single retry owner, so it refetches main and runs the checker again. set -uo pipefail attempts=3 @@ -12,7 +13,7 @@ for attempt in $(seq 1 "$attempts"); do if [ "$status" -ne 2 ]; then exit "$status" fi - echo "Trusted base advanced during evaluation; retrying against current main (${attempt}/${attempts})" >&2 + echo "Admission work incomplete; retrying against current main (${attempt}/${attempts})" >&2 done -echo "Trusted base kept advancing; ending this run as failed" >&2 +echo "Admission work still incomplete after ${attempts} attempts" >&2 exit 1