diff --git a/.github/workflows/parity-governance.yml b/.github/workflows/parity-governance.yml index 2f7e82e65c..d68dc01cab 100644 --- a/.github/workflows/parity-governance.yml +++ b/.github/workflows/parity-governance.yml @@ -77,8 +77,8 @@ jobs: 2>&1 | tee "$RUNNER_TEMP/out.txt" ran=$(grep -oE '^Ran [0-9]+ test' "$RUNNER_TEMP/out.txt" | grep -oE '[0-9]+') echo "collected ${ran:-0} tests" - if [ "${ran:-0}" -lt 115 ]; then - echo "::error::expected at least 115 parity-governance tests, collected ${ran:-0} — discovery is broken, not the suite" + if [ "${ran:-0}" -lt 122 ]; then + echo "::error::expected at least 122 parity-governance tests, collected ${ran:-0} — discovery is broken, not the suite" exit 1 fi working-directory: Tools diff --git a/Tools/PARITY_GOVERNANCE.md b/Tools/PARITY_GOVERNANCE.md index a801856874..9e7be1b425 100644 --- a/Tools/PARITY_GOVERNANCE.md +++ b/Tools/PARITY_GOVERNANCE.md @@ -115,6 +115,36 @@ dispositions are unchanged between them and both regenerated snapshots exactly match that shared state. Any source debt introduced on the repair branch still fails closed, as does a hand-edited or partial snapshot. +Repair therefore covers a base that is stale and OTHERWISE UNCHANGED. It cannot +cover a base whose stored authority no longer reproduces at all, which is what +merged product files leave behind: the governed state genuinely differs, so the +equality repair insists on can never hold. That case fails with + +``` +base authority cannot be reproduced with the current derivation; migration +required (see --migrate-authority) +``` + +and its remedy is the migration variant: + +```sh +python3 Tools/parity_ledger.py --refresh-derived --migrate-authority --base origin/main +``` + +Migration re-bases the comparison onto a freshly derived base authority. It +waives exactly one thing, whether the base's STORED manifest can be reproduced, +and it is not a force switch either. New one-sided declarations are still +computed from the freshly derived base and current sets, so anything this branch +adds still needs its own issue-bound disposition and an undeclared one still +fails closed. A hand-edited current authority is refused outright. + +Be clear-eyed about what a post-merge refresh does: it ADOPTS the one-sided +declarations already on `main` into the authority, because the ratchet governs +debt relative to the base and merged debt is the base. That is the intended +maintenance action, and it is also why the daily schedule matters. It finds the +drift within a day rather than letting it accumulate until a contributor +inherits it. + For a new one-sided declaration, choose explicitly: The `add-unpaired-function` diagnostic defines "new" by absence of the diff --git a/Tools/parity_ledger.py b/Tools/parity_ledger.py index b9aa0ed5d2..dd920ab0c6 100644 --- a/Tools/parity_ledger.py +++ b/Tools/parity_ledger.py @@ -2626,6 +2626,12 @@ def main(argv: list[str] | None = None) -> int: help="with --refresh-derived, repair metadata drift already present in the exact base", ) parser.add_argument("--base", default="origin/main", help="exact git ref used to prove debt reductions") + parser.add_argument( + "--migrate-authority", + action="store_true", + help="re-base onto a freshly derived base authority when the base's stored one cannot be " + "reproduced; new debt still requires issue-bound dispositions", + ) args = parser.parse_args(argv) root = args.root.resolve() @@ -2637,6 +2643,12 @@ def main(argv: list[str] | None = None) -> int: if args.repair_stale_base and not args.refresh_derived: print("FAIL --repair-stale-base requires --refresh-derived") return 2 + if args.migrate_authority and not args.refresh_derived: + print("FAIL --migrate-authority requires --refresh-derived") + return 2 + if args.migrate_authority and args.repair_stale_base: + print("FAIL --repair-stale-base and --migrate-authority are different remedies; use one") + return 2 if args.refresh_derived: if args.bootstrap_map or args.write_baseline or args.no_baseline: print("FAIL --refresh-derived cannot be combined with bootstrap, baseline, or display modes") @@ -2664,6 +2676,8 @@ def main(argv: list[str] | None = None) -> int: ] if args.repair_stale_base: command.append("--repair-stale-base") + if args.migrate_authority: + command.append("--migrate-authority") completed = subprocess.run(command, cwd=root, text=True, capture_output=True) if completed.returncode: print("FAIL derived refresh rejected; snapshots restored") diff --git a/Tools/parity_ledger_baseline.json b/Tools/parity_ledger_baseline.json index 793b1c820a..5701d3dc31 100644 --- a/Tools/parity_ledger_baseline.json +++ b/Tools/parity_ledger_baseline.json @@ -16,10 +16,10 @@ {"rule": "duplicate-implementation", "scope": "android/ui", "reason": "Exact platform-local implementation is retained as visible parity debt pending a source-level consolidation decision.", "provenance": "Exact identities emitted by parity_ledger rule duplicate-implementation for current upstream scope android/ui; no wildcard matching.", "count": 4, "identities_sha256": "b9f579c3d30221ff72e40282c29baab20d57d95acf044e0f71d2338e37c52b1b"}, {"rule": "test-only-callsite", "scope": "Packages/OuraProtocol", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope Packages/OuraProtocol; no wildcard matching.", "count": 5, "identities_sha256": "ff6404cd4d27782334dd844223be3ec86bb3260bef7f7e29c658fb6ad6e7938e"}, {"rule": "test-only-callsite", "scope": "Packages/StrandAnalytics", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope Packages/StrandAnalytics; no wildcard matching.", "count": 44, "identities_sha256": "fff777acbc66e7dad92bd0a6c8b7644e5c7ab6ddc0a7106dfecc49319da6127b"}, - {"rule": "test-only-callsite", "scope": "Packages/StrandImport", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope Packages/StrandImport; no wildcard matching.", "count": 13, "identities_sha256": "2ee99bb3c9fe8ab244063600cd863f41344b26780bcb73b814c13cd26aea9413"}, + {"rule": "test-only-callsite", "scope": "Packages/StrandImport", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope Packages/StrandImport; no wildcard matching.", "count": 14, "identities_sha256": "1acdc71178e0a555f000c42980345e8c38495f9e3806a866f31b260865d3ac98"}, {"rule": "test-only-callsite", "scope": "Packages/WhoopProtocol", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope Packages/WhoopProtocol; no wildcard matching.", "count": 30, "identities_sha256": "a9dc115534e8710217db9cd39bcceaa1419741737bfd3b733007dc0215cd5af2"}, - {"rule": "test-only-callsite", "scope": "Packages/WhoopStore", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope Packages/WhoopStore; no wildcard matching.", "count": 51, "identities_sha256": "82cb0bf1f7db514ca69dc7a1394b0de01d7ae4d7d1f948754da52c5b29e01277"}, - {"rule": "test-only-callsite", "scope": "android/analytics", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope android/analytics; no wildcard matching.", "count": 66, "identities_sha256": "edae212c4a4da22b0f11432e83d6a9b8abc21d0649d16026b439686681add718"}, + {"rule": "test-only-callsite", "scope": "Packages/WhoopStore", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope Packages/WhoopStore; no wildcard matching.", "count": 37, "identities_sha256": "2a62c1ece1c541b662dd407f0668d9a02782bb4d21f9bb181518272ff12f0e40"}, + {"rule": "test-only-callsite", "scope": "android/analytics", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope android/analytics; no wildcard matching.", "count": 70, "identities_sha256": "ba5baa3af54f2a05759f3ddaa3fe61cfa8f99864852c7fd804a25ef2efdc3b80"}, {"rule": "test-only-callsite", "scope": "android/data", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope android/data; no wildcard matching.", "count": 14, "identities_sha256": "9bab871f5ddbc1d2634805bf525888125c3880d160ab09b3ee7f3d7af6c487cd"}, {"rule": "test-only-callsite", "scope": "android/ingest", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope android/ingest; no wildcard matching.", "count": 12, "identities_sha256": "d4f45d66087768eab133b918af6658b06ec1d91305e38fb045f6ae1629c94678"}, {"rule": "test-only-callsite", "scope": "android/oura", "reason": "Exact declaration is reached only from tests in the conservative lexical call graph; dynamic, callback and external entry points are intentionally not inferred.", "provenance": "Exact identities emitted by parity_ledger rule test-only-callsite for current upstream scope android/oura; no wildcard matching.", "count": 4, "identities_sha256": "9beaa9034af0a1021b9a9f43739971934f260147e0a9a5c462468548a7fd577d"}, diff --git a/Tools/parity_ratchet.py b/Tools/parity_ratchet.py index 7ba321bc00..c3bbc9416b 100644 --- a/Tools/parity_ratchet.py +++ b/Tools/parity_ratchet.py @@ -409,6 +409,7 @@ def compare_metadata( *, offline: bool, repair_stale_base: bool = False, + migrate_authority: bool = False, warnings: list[str] | None = None, ) -> list[str]: """Compare current governance metadata with the exact requested base.""" @@ -465,9 +466,31 @@ def compare_metadata( repair_mismatches.append("current authority is not exactly derived") if current_baseline != parity_ledger.build_compact_baseline(current_scan): repair_mismatches.append("current baseline is not exactly derived") - if not repair_stale_base: + if migrate_authority: + # The base's checked-in authority cannot be reproduced by the current derivation, + # so there is no exact basis to compare against and `--repair-stale-base` cannot + # help: repair exists for a base whose GOVERNED STATE matches, and here it does not. + # + # Migration re-bases the comparison onto a freshly derived base authority. It + # deliberately waives only the reproducibility of the base's stored manifest. It + # waives NO semantic debt: `required` below is computed from the freshly derived + # base and current sets, so every new one-sided declaration still needs its own + # issue-bound disposition, and an undeclared one still fails. + if current_map["authority"] != current_manifest: + errors.append( + f"{TWIN_MAP_PATH}: authority migration requires an exactly derived current " + "authority; refresh the snapshots rather than hand-editing them" + ) + else: + warnings.append( + f"{TWIN_MAP_PATH}: base authority at {base} is not reproducible with the " + "current derivation; migrated onto a freshly derived base. New debt still " + "requires issue-bound dispositions." + ) + elif not repair_stale_base: errors.append( - f"{TWIN_MAP_PATH}: base authority cannot be reproduced with the current derivation; migration required" + f"{TWIN_MAP_PATH}: base authority cannot be reproduced with the current derivation; " + "migration required (see --migrate-authority)" ) elif repair_mismatches: errors.append( @@ -654,7 +677,15 @@ def main(argv: list[str] | None = None) -> int: action="store_true", help="adopt exactly derived metadata only when governed state is unchanged from an already-stale base", ) + parser.add_argument( + "--migrate-authority", + action="store_true", + help="re-base onto a freshly derived base authority when the base's stored one cannot be " + "reproduced; new debt still requires issue-bound dispositions", + ) args = parser.parse_args(argv) + if args.repair_stale_base and args.migrate_authority: + parser.error("--repair-stale-base and --migrate-authority are different remedies; use one") root = args.root.resolve() try: base = resolve_base(root, args.base) @@ -665,6 +696,7 @@ def main(argv: list[str] | None = None) -> int: base, offline=args.offline, repair_stale_base=args.repair_stale_base, + migrate_authority=args.migrate_authority, warnings=warnings, )) for warning in warnings: diff --git a/Tools/parity_twin_map.json b/Tools/parity_twin_map.json index 07770c2da2..77a8f1dd64 100644 --- a/Tools/parity_twin_map.json +++ b/Tools/parity_twin_map.json @@ -18,17 +18,17 @@ ] }, "authority": { - "files": {"count": 490, "sha256": "554e510e220e3bf87187abc422178216f31c855311cb093434bda155e9283f96"}, - "functions": {"count": 4353, "sha256": "dc31ce701cf0ee2ec1992a21da290712ee63cfa0268078051fd71878dd2671bf"}, - "properties": {"count": 451, "sha256": "cb176d7203c8c42522f55d0db9acdcde97ed7cad5e7225cc6a41b18206495c8d"}, - "constants": {"count": 1893, "sha256": "9687e3e36f209bf54e0f78e8e0d0cbf4e21d148c47ed1e4cac3b0645c60b53ff"}, - "file_pairs": {"count": 61, "sha256": "ed77a9b1fadece7a30b27b37cb1826a2e95ac1b5e81419fa5364ae6dffc22bf3"}, - "function_pairs": {"count": 153, "sha256": "a4ed8f320c9da4e8838b605095f5f82d58abd98fae3ece0e30ce4db4898d5dc6"}, - "property_pairs": {"count": 147, "sha256": "781073f4b7af31db4400da247eb16910c2c8adbbb4ab5f9169b852566ed3dbc8"}, - "constant_pairs": {"count": 665, "sha256": "080c97b062e80cb7d1f526bfb50b367cbbf6b0cadf43e2c5ec3f2ad046fe09cb"}, - "unpaired_files": {"count": 386, "sha256": "a2edbca2181629394366a14ad4562bbaa82b2b9db0d729e43f3a8f1836f255cd"}, - "unpaired_functions": {"count": 4053, "sha256": "ee636450c185d53eb6e6754d41890a07f8d24412c07edeec859492f9cafe4c39"}, - "unpaired_properties": {"count": 157, "sha256": "bd4cab9d2ebec6da6ed6ff95599b0ab8cd3b4ca7a0c2c3e82493280940d8fda9"}, - "unpaired_constants": {"count": 563, "sha256": "0ada9824528f78eba95c65c56ce1b744d9d5bb876ebb2ca75f6d0b02874e89e8"} + "files": {"count": 495, "sha256": "b816e4e60a9af282c8b8421d04ce8b22e138f6576300a3099b99e2e15f6d7ce9"}, + "functions": {"count": 4404, "sha256": "34625acc52521af84e4930b35e437dcd68771a379587a1b0057991417af487b2"}, + "properties": {"count": 454, "sha256": "7ffacb0e1503998abd5ed71a75fcbcad8babecc980fd185934669cf6950af758"}, + "constants": {"count": 1926, "sha256": "3eda514a0c352dfd2c48d023d2ac812b08ea3b5f1ff0537d878382eb338368c0"}, + "file_pairs": {"count": 63, "sha256": "e244a0f865ce602228ab102fcb3f2235e38974c60f81473054c543e531ba1829"}, + "function_pairs": {"count": 161, "sha256": "f848d2b040b4c6a52fbe3f188116aaae9868e02bcfc52e1ffefdc608d85e996b"}, + "property_pairs": {"count": 148, "sha256": "6fa59982fca9e8e306562b9d027676457c9fa4cc6766c1d6171e86084f3b2c54"}, + "constant_pairs": {"count": 672, "sha256": "1142e37c14f174037e0358c845ea4ae2a12a1c2f2b348de4e2cb37ce310f1a00"}, + "unpaired_files": {"count": 387, "sha256": "3648c0f6f32f0fc95a8ee6b110610fb4a27df3155f4f28709a1a1004affec9b5"}, + "unpaired_functions": {"count": 4088, "sha256": "f5b83976efe33788a00e6eaa38e82639704ba894374d92ef0532426cc1ea6016"}, + "unpaired_properties": {"count": 158, "sha256": "68b38a5e8ede04d74a858544adbcc9a43bd0d49d7b318656823cf5696ceb99ad"}, + "unpaired_constants": {"count": 582, "sha256": "c660255caa35b36e4bd12e17680eb398754472a9f77c8de95b12c049668cf397"} } } diff --git a/Tools/tests/test_parity_governance_acceptance.py b/Tools/tests/test_parity_governance_acceptance.py index aa87af338c..f4ffff3e76 100644 --- a/Tools/tests/test_parity_governance_acceptance.py +++ b/Tools/tests/test_parity_governance_acceptance.py @@ -578,6 +578,144 @@ def test_stale_base_repair_rejects_nonexact_baseline(self) -> None: for error in errors ), errors) + def test_authority_migration_accepts_a_stale_base_when_new_debt_is_disposed(self) -> None: + """The remedy #2229 asked for: a base whose stored authority cannot be reproduced. + + `--repair-stale-base` cannot help here, because repair is for a base whose GOVERNED STATE + matches and this one carries genuinely new drift. Migration re-bases onto a freshly derived + base authority; the new declaration still has to be declared to pass. + """ + swift, base = self.stale_base_repair_fixture() + swift.write_text( + "enum Engine { static func alreadyOnMain() {}; static func addedOnBranch() {} }\n", + encoding="utf-8", + ) + refreshed = parity_ledger.build_compact_twin_map(self.root) + self.write("Tools/parity_twin_map.json", refreshed) + self.write( + "Tools/parity_ledger_baseline.json", + parity_ledger.build_compact_baseline(parity_ledger.scan(self.root, refreshed)), + ) + identity = next( + item for item in parity_ledger.semantic_authority(self.root)["unpaired_functions"] + if "addedOnBranch" in item + ) + self.write("Tools/parity_dispositions.json", { + "schema_version": 1, + "dispositions": [{ + "type": "platform_specific", + "kind": "add-unpaired-function", + "identity": identity, + "identity_sha256": parity_ledger._canonical_sha256(identity), + "platform": "swift", + "rationale": "Swift-only by design for this fixture.", + }], + }) + + warnings: list[str] = [] + errors = parity_ratchet.compare_metadata( + self.root, base, offline=True, migrate_authority=True, warnings=warnings, + ) + + self.assertEqual([], errors) + self.assertTrue(any("migrated onto a freshly derived base" in w for w in warnings), warnings) + + def test_authority_migration_still_rejects_undeclared_new_debt(self) -> None: + """Migration waives the base manifest's reproducibility and nothing else. + + This is the guard worth pinning: if migration ever started waiving semantic debt too, the + flag would become a way to launder undeclared one-sided declarations onto main. + """ + swift, base = self.stale_base_repair_fixture() + swift.write_text( + "enum Engine { static func alreadyOnMain() {}; static func addedOnBranch() {} }\n", + encoding="utf-8", + ) + refreshed = parity_ledger.build_compact_twin_map(self.root) + self.write("Tools/parity_twin_map.json", refreshed) + self.write( + "Tools/parity_ledger_baseline.json", + parity_ledger.build_compact_baseline(parity_ledger.scan(self.root, refreshed)), + ) + + errors = parity_ratchet.compare_metadata( + self.root, base, offline=True, migrate_authority=True, + ) + + self.assertTrue(any("addedOnBranch" in error for error in errors), errors) + self.assertTrue( + any("issue-bound authority change" in error for error in errors), errors + ) + + def test_authority_migration_rejects_a_hand_edited_current_authority(self) -> None: + _swift, base = self.stale_base_repair_fixture() + tampered = parity_ledger._load_json(self.root / "Tools/parity_twin_map.json", {}) + tampered["authority"]["unpaired_functions"]["count"] += 1 + self.write("Tools/parity_twin_map.json", tampered) + + errors = parity_ratchet.compare_metadata( + self.root, base, offline=True, migrate_authority=True, + ) + + self.assertTrue( + any("requires an exactly derived current authority" in error for error in errors), + errors, + ) + + def test_authority_migration_rejects_a_current_authority_equal_to_the_stale_base(self) -> None: + """The sharp case, found by mutating the guard rather than by reading it. + + The general protection against a non-exact current authority only ERRORS when it matches + neither the tree nor the base; when it matches the stale base exactly it merely warns + "debt decreased". Under migration that shape would otherwise sail through and adopt an + unrefreshed authority, which is the one thing migration must not do. + """ + _swift, base = self.stale_base_repair_fixture() + stale = parity_ratchet._read_base(self.root, base, "Tools/parity_twin_map.json") + self.write("Tools/parity_twin_map.json", stale) + + warnings: list[str] = [] + errors = parity_ratchet.compare_metadata( + self.root, base, offline=True, migrate_authority=True, warnings=warnings, + ) + + self.assertTrue( + any("requires an exactly derived current authority" in error for error in errors), + errors, + ) + self.assertFalse( + any("migrated onto a freshly derived base" in w for w in warnings), + "a stale current authority must not be reported as a completed migration", + ) + + def test_migrate_authority_flag_requires_guarded_refresh(self) -> None: + output = io.StringIO() + with mock.patch("sys.stdout", output): + code = parity_ledger.main([ + "--root", str(self.root), "--migrate-authority", + ]) + self.assertEqual(2, code) + self.assertIn("requires --refresh-derived", output.getvalue()) + + def test_migrate_authority_and_repair_are_mutually_exclusive(self) -> None: + output = io.StringIO() + with mock.patch("sys.stdout", output): + code = parity_ledger.main([ + "--root", str(self.root), "--refresh-derived", + "--repair-stale-base", "--migrate-authority", + ]) + self.assertEqual(2, code) + self.assertIn("different remedies", output.getvalue()) + + def test_ordinary_refusal_names_the_migration_flag(self) -> None: + """A dead end that does not name its exit is what made #2229 take a day.""" + _swift, base = self.stale_base_repair_fixture() + + errors = parity_ratchet.compare_metadata(self.root, base, offline=True) + + self.assertTrue(any("migration required" in error for error in errors), errors) + self.assertTrue(any("--migrate-authority" in error for error in errors), errors) + def test_repair_stale_base_flag_requires_guarded_refresh(self) -> None: output = io.StringIO() with mock.patch("sys.stdout", output):