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/.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..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 @@ -127,11 +131,56 @@ 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. 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 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`: + +| `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 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, 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 5146a7bcc..3debe1cc7 100644 --- a/tests/test_knowledge_model_policy.py +++ b/tests/test_knowledge_model_policy.py @@ -11,6 +11,8 @@ from tools.check_knowledge_model_policy import ( APPROVAL, DECLARATION, + EXIT_COMPLIANT, + EXIT_RERUN, GitHub, approved, assemble_file_pages, @@ -19,12 +21,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] @@ -251,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" - # PR 内容中声称的 policy 不是 assess 的可信输入。 + 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(): @@ -289,7 +307,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"] @@ -305,6 +323,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" @@ -313,9 +336,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": []} + "before_request": None, "errors": [], "base": BASE, "fail": lambda route: False, + "pulls_scans": 0} class Handler(BaseHTTPRequestHandler): def log_message(self, *args): @@ -335,9 +359,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: @@ -347,7 +375,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]) @@ -414,7 +442,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" @@ -497,7 +525,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"]) @@ -522,3 +550,350 @@ 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, + "unverified": 0, "deferred": 0, "closed": 1, + "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 + + +@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: + # A superseded declaration is this PR's own rejection, not a sweep failure. + assert results[0]["kind"] == "noncompliant" + assert exit_code(results) == EXIT_COMPLIANT + + +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_COMPLIANT + + +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"]) + # 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_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() + 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 8f269fb80..6312d3ef1 100644 --- a/tools/check_knowledge_model_policy.py +++ b/tools/check_knowledge_model_policy.py @@ -14,6 +14,47 @@ APPROVAL = "knowledge-policy-approval" SHA = re.compile(r"[0-9a-f]{40}") ROOT = Path(__file__).resolve().parents[1] +# 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 +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", + "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 = {"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), + "noncompliant_pull_requests": rejected, + "execution": "unfinished" if rerun else "failed" if error else "ok", + "rerun": rerun} + + +def exit_code(results): + # 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 ERROR_KINDS for result in results) \ + else EXIT_COMPLIANT def unique_object(pairs): @@ -147,7 +188,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 @@ -178,7 +219,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 @@ -187,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"]) @@ -207,8 +247,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, @@ -269,12 +311,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]} def evaluation_failure(error): @@ -282,25 +322,70 @@ def evaluation_failure(error): 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]} + + +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 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: - 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']}" + 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: 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"]) paths = changed_paths(files, pr["changed_files"]) - controls = matching(paths, policy["policy_paths"]) protected = matching(paths, policy["protected_paths"]) request = None if protected: @@ -310,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") @@ -322,64 +408,165 @@ 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") - 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: + # 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 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: result = evaluation_failure(error) return result, comments def admission_snapshot(pr): - # list PR API 不含 changed_files；head/base 相同才可复用已验证的文件列表。 + # 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"]) +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: - numbers = [pr["number"] for pr in open_prs if pr["base"]["ref"] == policy["base_branch"]] - pending = [] + 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)] + # 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}") + 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 dry_run: - api.status(head, "pending", "Checking current declaration against trusted base policy") + 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) - pending.append((pr, result, comments)) - # 整轮只扫描两次 open PR；最终索引同时复核共享 head 与正文等元数据。 + 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: - 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"] + open_prs = api.pages("pulls?state=open") + except API_ERRORS as error: + open_prs, scan_error = [], error + published = {} + for number, record in records.items(): + head = record["head"] 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") - 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) + 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"]: + 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 + # 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["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: - api.status(head, result["state"], result["description"]) - results.append({"pull_request": number, "head_sha": head, "policy_sha": policy_sha, - "dry_run": dry_run, **result}) + try: + 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["rerun"], "state": item["state"], + "kind": item["kind"], "description": item["description"], + "errors": item["errors"]}) return results @@ -419,13 +606,13 @@ 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 [] - 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..c8927d340 --- /dev/null +++ b/tools/run_knowledge_model_policy.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +# Run the knowledge admission checker against the current trusted main. +# 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 +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 "Admission work incomplete; retrying against current main (${attempt}/${attempts})" >&2 +done +echo "Admission work still incomplete after ${attempts} attempts" >&2 +exit 1