From a60dea972aa71defb5e0d1366279326316f58426 Mon Sep 17 00:00:00 2001 From: Dong1017 <55723162+Dong1017@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:08:55 +0900 Subject: [PATCH] fix(skill-eval): retain authenticated results on unpaid events Use verified ledger bindings to locate existing same-plan evidence without additional execution. Update bounded live rollout documentation. Agent-assisted-by: OpenAI Codex Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com> --- docs/skill-eval-deployment-215.md | 8 +- docs/skill-eval-pr.md | 42 ++++++--- tests/test_skill_eval_retention.py | 141 +++++++++++++++++++++++++++++ tools/skill_eval_auto.py | 27 ++++++ tools/skill_eval_pr.py | 15 +-- 5 files changed, 212 insertions(+), 21 deletions(-) create mode 100644 tests/test_skill_eval_retention.py diff --git a/docs/skill-eval-deployment-215.md b/docs/skill-eval-deployment-215.md index fa11a538..09c07836 100644 --- a/docs/skill-eval-deployment-215.md +++ b/docs/skill-eval-deployment-215.md @@ -71,8 +71,10 @@ production guards. Deployment is not complete merely because this file exists. The trusted main controller, all blocking review fixes, authenticated Git-data non-force CAS, audited anchor readback and exact-head checks must pass before a label is added -or any paid request is launched. This local preparation performs none of those -external mutations, does not dispatch a run, and does not claim token/storage -permissions have been live-tested. Keep existing protection/settings unchanged. +or any paid request is launched. The historical local preparation performed none +of those external mutations and did not dispatch a run or claim live-tested +permissions. Subsequent bounded deployment and real-run verification are recorded +in [the acceptance progress](https://github.com/InferMatrixLab/InferRSI/issues/215#issuecomment-6471044747); +that record does not authorize additional paid runs. Keep existing protection/settings unchanged. Remove the label to revoke authorization before the next request; never delete or roll back a ledger to recover capacity. Retain all ambiguous reservations. diff --git a/docs/skill-eval-pr.md b/docs/skill-eval-pr.md index c0e24b08..a611fc7a 100644 --- a/docs/skill-eval-pr.md +++ b/docs/skill-eval-pr.md @@ -208,8 +208,13 @@ link is simulated; inspect the corresponding local `publication/` evidence. The workflow is already on main. The bounded #88/#101 addition/update acceptance established the existing real-agent execution and feedback path, including a valid neutral update; it did not establish whole-library efficacy or subject-PR -merge readiness. #215's advisory behavior still needs revision-scoped real-PR -demonstration and independent review. Offline controls are not that demonstration. +merge readiness. The deployed #215 path subsequently completed owner-authorized +revision-scoped real runs on [#159](https://github.com/InferMatrixLab/InferRSI/actions/runs/40700386355) +and [#160](https://github.com/InferMatrixLab/InferRSI/actions/runs/40700395804), with retained +publication. #159's candidate was inconclusive and the advisory reported a regression; +#160 was unchanged-correct. These bounded observations are not skill-efficacy claims. +[Acceptance progress](https://github.com/InferMatrixLab/InferRSI/issues/215#issuecomment-6471044747) +tracks the remaining #215 work; the issue is not declared closed here. Neither fixed outputs nor green CI authorize paid trials, merge or issue closure. ## Reading the advisory guidance @@ -261,9 +266,9 @@ answers. Standalone freeze fields are not all independently authenticated. The PR path's trusted reconstruction narrows that boundary; it does not create cryptographic provenance. Review guidance is advisory evidence interpretation, not execution attestation or skill-efficacy certification. Unrelated harness -lifecycle/Windows contention work remains outside this slice and under #179. +lifecycle/Windows contention work was tracked separately in the now-closed #179. -## Label-gated cumulative paid execution (deployment prerequisite) +## Label-gated cumulative paid execution The trusted controller extends the existing prepare/execute/pack/publish pipeline; it is not another runner. It uses only the existing Yibuapi Messages agent and @@ -274,10 +279,11 @@ subsequent requests through a live GitHub check, independently of whether a path-filtered notification starts another workflow. Already in-flight calls cannot be revoked. Closed events never start preparation or paid work. -**Not activated by this change.** An owner must separately review and deploy -`tools/skill_eval_auto_config.json` in trusted main. No working config, label, -credential or ledger genesis is created by this implementation. The config is -strictly shaped as: +**Deployment is present on trusted main.** Owner-reviewed configuration and +audited ledger genesis were deployed for the bounded #159/#160 rollout. A new +deployment still requires separate owner approval, credentials and ledger audit; +the code never creates these prerequisites implicitly. +`tools/skill_eval_auto_config.json` is strictly shaped as: - `schema_version: 1`, immutable `repository_id`, dedicated `label`, and explicit owner-authorized GitHub logins in `operators`; @@ -324,10 +330,22 @@ SHA contract. Neither event metadata alone nor a result can rebind an old plan. Legacy paid `workflow_dispatch execute` is explicitly blocked instead of bypassing the cumulative ledger. Existing credential-free injected-client tests still work. -Live acceptance remains pending until the owner-reviewed provider configuration, -explicit label/operator policy, current pricing evidence and audited historical -spend/genesis anchors exist, followed by an authorized real run and publication. -Offline controller tests do not establish provider charges or skill efficacy. +The bounded real-run verification above exercised planning, guarded execution and +retained publication. It does not authorize additional runs or establish general +skill efficacy. Offline controller tests do not establish provider charges. + +Unpaid events (including label removal and duplicates) look up an exact frozen-plan +fingerprint in the verified Git ledger and reuse its authenticated original source +run. They do not spend or mutate the ledger. The publisher still verifies source +workflow identity and the complete original plan before importing evidence; changed +plans never inherit old results. Missing/expired/invalid evidence remains explicit. +An unreadable configured ledger blocks publication instead of erasing evidence. + +The implementation version itself is part of the frozen plan. Deploying this +retention repair therefore makes the earlier real-run plans historical; their +source links remain receipts, not evidence for a newly generated plan. Same-plan +preservation is verified offline without new paid validation. Do not weaken the +version/exam binding merely to restore an older comment. ### Free planning remains available when execution is blocked diff --git a/tests/test_skill_eval_retention.py b/tests/test_skill_eval_retention.py new file mode 100644 index 00000000..f07c550f --- /dev/null +++ b/tests/test_skill_eval_retention.py @@ -0,0 +1,141 @@ +"""Unpaid notifications retain exact-plan evidence using authenticated provenance.""" +import copy +from types import SimpleNamespace +from unittest.mock import Mock + +import pytest + +from inferrsi.infra.artifacts import write_json +from tests import test_skill_eval_auto as auto_tests +from tests.fixtures.skill_eval_compare_support import FixedOutputClient +from tests.fixtures.skill_eval_pr_support import SimulatedGitHub, approval, setup +from tests.test_skill_eval_pr import dispatch_event +from tools import skill_eval_auto as auto +from tools import skill_eval_pr as pr +from tools.skill_eval_budget import Binding, BudgetError, Entry + + +@pytest.fixture +def config(): + return auto_tests.config.__wrapped__() + + +def prepared_evidence(tmp_path, monkeypatch, config): + repo, mapping, identity = setup(tmp_path, ids=("one",)) + output = tmp_path / "prepared" + plan = pr.prepare(repo, mapping, identity, output) + client = FixedOutputClient(tmp_path) + pr.execute(output, approval(plan), client=client) + data = pr.result_archive(output).read_bytes() + # The publisher fixture is synthetic, not a provider execution: attach the + # reviewed provider identity only after generating offline injected answers. + plan["provider_config"] = config["provider"] + plan["reservation"] = pr.provider_reservation(plan) + write_json(output / "pr-plan.json", plan) + api = SimulatedGitHub(plan, data) + api.run.update(event="pull_request_target", head_sha=plan["head"], + display_title="skill-eval plan or publish PR 7") + entries = [Entry(Binding(pr.fingerprint(plan), "10.1", 100), "claimed")] + store = SimpleNamespace(read_verified=lambda key: SimpleNamespace( + ledger=SimpleNamespace(entries=entries))) + monkeypatch.setattr(auto, "configuration", lambda **kw: {**config, "anchors": {"7": {}}}) + monkeypatch.setattr(auto, "ledger_store", lambda *a: (store, "offline-key")) + monkeypatch.setattr(auto, "report_control", lambda *a: {"state": "blocked"}) + monkeypatch.setattr(pr, "GitHub", lambda *a: api) + monkeypatch.setattr(auto, "controlled_execute", Mock(side_effect=AssertionError("No paid run"))) + monkeypatch.setattr(pr, "execute", Mock(side_effect=AssertionError("No model calls"))) + return plan, output, api, entries, store + + +@pytest.mark.parametrize("event", ["unlabeled", "duplicate", "unpaid"]) +def test_unpaid_event_reuses_original_run_and_does_not_erase_results( + tmp_path, monkeypatch, config, event): + plan, output, api, entries, store = prepared_evidence(tmp_path, monkeypatch, config) + before = copy.deepcopy(entries) + pr.publish_retained(api, output, 10, 10, 1) + assert '"unchanged_correct":1' in api.comments[0]["body"] + dispatch_event(monkeypatch, tmp_path, output, {}, "publish", run_id=11) + write_json(tmp_path / "event.json", {"action": event, "pull_request": {"number": 7}}) + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request_target") + monkeypatch.setenv("SKILL_EVAL_AUTO_SOURCE", "true" if event == "duplicate" else "false") + pr.main() + body = api.comments[0]["body"] + assert '"unchanged_correct":1' in body + assert "/actions/runs/10" in body and "skill-eval-writer:11" in body + assert "/actions/runs/11" not in api.reads + assert entries == before + auto.controlled_execute.assert_not_called() + pr.execute.assert_not_called() + + +@pytest.mark.parametrize("field", ["head", "base", "version", "exam"]) +def test_changed_frozen_plan_never_reuses_old_evidence(tmp_path, monkeypatch, config, field): + plan, output, api, entries, store = prepared_evidence(tmp_path, monkeypatch, config) + if field == "exam": + plan["comparisons"][0]["exam"]["grader"] = "f" * 64 + else: + plan[field] = "f" * 40 + assert auto.retained_source(api, plan) is None + write_json(output / "pr-plan.json", plan) + api.current.update({key: plan[key] for key in ("base", "head")}) + dispatch_event(monkeypatch, tmp_path, output, {}, "publish") + write_json(tmp_path / "event.json", {"pull_request": {"number": 7}}) + monkeypatch.setenv("SKILL_EVAL_AUTO_SOURCE", "false") + pr.main() + assert '"unchanged_correct":1' not in api.comments[0]["body"] + assert not any("/artifacts" in path for path in api.reads) + + +@pytest.mark.parametrize("damage", ["pending", "event", "head", "repository", "ledger", + "attempt", "ambiguous", "original_plan"]) +def test_untrusted_retained_source_cannot_replace_available_evidence( + tmp_path, monkeypatch, config, damage): + plan, output, api, entries, store = prepared_evidence(tmp_path, monkeypatch, config) + pr.publish_retained(api, output, 10, 10, 1) + before = copy.deepcopy(api.comments) + if damage == "pending": + api.run["status"] = "in_progress" + elif damage == "event": + api.run["event"] = "pull_request" + elif damage == "head": + api.run["head_sha"] = "f" * 40 + elif damage == "repository": + api.run["repository"]["id"] = 456 + elif damage == "ledger": + store.read_verified = Mock(side_effect=BudgetError("Unknown ledger")) + elif damage == "attempt": + entries[0] = Entry(Binding(pr.fingerprint(plan), "10.2", 100), "claimed") + elif damage == "ambiguous": + entries.append(entries[0]) + else: + # Forged matching ledger cannot override the original source artifact. + plan["version"] = "f" * 40 + entries[0] = Entry(Binding(pr.fingerprint(plan), "10.1", 100), "claimed") + write_json(output / "pr-plan.json", plan) + dispatch_event(monkeypatch, tmp_path, output, {}, "publish") + write_json(tmp_path / "event.json", {"pull_request": {"number": 7}}) + monkeypatch.setenv("SKILL_EVAL_AUTO_SOURCE", "false") + if damage == "original_plan": + pr.main() + assert "artifacts are invalid" in api.comments[0]["body"] + assert '"unchanged_correct":1' not in api.comments[0]["body"] + else: + with pytest.raises(ValueError): + pr.main() + assert api.comments == before + auto.controlled_execute.assert_not_called() + pr.execute.assert_not_called() + + +def test_unconfigured_pr_has_no_invented_retained_source(tmp_path, monkeypatch, config): + plan, output, api, entries, store = prepared_evidence(tmp_path, monkeypatch, config) + monkeypatch.setattr(auto, "configuration", lambda **kw: config) + assert auto.retained_source(api, plan) is None # No trusted anchor for PR7. + monkeypatch.setattr(auto, "configuration", Mock(side_effect=FileNotFoundError)) + assert auto.retained_source(api, plan) is None + + +def test_unclaimed_reservation_is_not_execution_provenance(tmp_path, monkeypatch, config): + plan, output, api, entries, store = prepared_evidence(tmp_path, monkeypatch, config) + entries[0] = Entry(entries[0].binding, "reserved") + assert auto.retained_source(api, plan) is None diff --git a/tools/skill_eval_auto.py b/tools/skill_eval_auto.py index a170443d..009ec7bc 100644 --- a/tools/skill_eval_auto.py +++ b/tools/skill_eval_auto.py @@ -213,6 +213,33 @@ def check(): # No refund: missing/partial usage and cancellations retain the full reservation. +def retained_source(api, plan): + """Find prior evidence via the verified ledger, never via mutable comment text. + + Expiry/label removal revokes spending, not read-only evidence publication. + An unavailable configured ledger is an error, not permission to erase results. + """ + if "provider_config" not in plan: + return None + try: + config = configuration(now=0) + except FileNotFoundError: + return None + if str(plan["pr"]) not in config["anchors"]: + return None + if api.request("GET", "")["id"] != config["repository_id"]: + raise ValueError("Repository identity changed") + store, key = ledger_store(api, config, plan["pr"]) + entries = store.read_verified(key).ledger.entries + matching = [entry for entry in entries if entry.binding.plan_digest == pr.fingerprint(plan) + and entry.phase in {"claimed", "finished"}] + if not matching: + return None + if len(matching) != 1 or not re.fullmatch(r"[1-9][0-9]*[.]1", matching[0].binding.run_id): + raise ValueError("Ambiguous or invalid retained execution identity") + return int(matching[0].binding.run_id.split(".")[0]) + + def report_control(api, plan): """Read-only publisher derivation; never trust an artifact's claimed budget state.""" result = {"state": "blocked", diff --git a/tools/skill_eval_pr.py b/tools/skill_eval_pr.py index f5623912..67b595e6 100644 --- a/tools/skill_eval_pr.py +++ b/tools/skill_eval_pr.py @@ -897,12 +897,15 @@ def main(): claim(plan, runs, run_id, int(os.environ["GITHUB_RUN_ATTEMPT"])) write_json(output / "approval.json", approval) return - if (inputs.get("action") in ("execute", "publish") - or os.environ.get("SKILL_EVAL_AUTO_SOURCE") == "true"): - if inputs.get("action") == "publish" and not inputs.get("source_run"): - raise ValueError("Publish-only retry requires the original execution run ID") - receipt = publish_retained(api, output, int(inputs.get("source_run") or run_id), - run_id, time.time()) + if inputs.get("action") == "publish" and not inputs.get("source_run"): + raise ValueError("Publish-only retry requires the original execution run ID") + from tools.skill_eval_auto import retained_source + source = (int(inputs["source_run"]) if inputs.get("action") == "publish" + else retained_source(api, plan)) + if source is None and os.environ.get("SKILL_EVAL_AUTO_SOURCE") == "true": + source = run_id + if source is not None: + receipt = publish_retained(api, output, source, run_id, time.time()) else: now = time.time() url = f"https://github.com/{repository}/actions/runs/{run_id}"