fix: mutation budget counts server-side changes only (Closes #617)
Auto-mode classifier now distinguishes local validator rejection, capability-gate rejection, transport failure before API, and successful server-side mutation. Pre-API validator failures no longer consume server-side mutation budget; the final report separately accounts for local failed attempts, blocked API attempts, and successful server-side mutations. Recovered from preserved unpublished commit b46f0f9 via native MCP unpublished-claim recovery (#772) and author-worktree lock binding (#618), reconciled onto current master. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Claude Opus 4.8
@@ -0,0 +1,388 @@
|
||||
"""Tests for the #617 mutation-budget classifier.
|
||||
|
||||
Covers every acceptance criterion on issue #617:
|
||||
|
||||
* AC1 — the classifier distinguishes local validator rejection, capability-gate
|
||||
rejection, transport failure before API, and successful server-side mutation.
|
||||
* AC2 — pre-API validator failures do not consume server-side mutation budget.
|
||||
* AC3 — failed attempts are still logged in the local attempt ledger.
|
||||
* AC4 — the final report separately shows local failed attempts, blocked API
|
||||
attempts, and successful server-side mutations.
|
||||
* AC5 — the six named scenarios, including the #615 reproduction where two
|
||||
local validator rejections precede one successful comment.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
|
||||
from mutation_budget_classifier import (
|
||||
AMBIGUOUS_REQUIRES_READBACK,
|
||||
CAPABILITY_GATE_REJECTION,
|
||||
LOCAL_VALIDATOR_REJECTION,
|
||||
SERVER_SIDE_MUTATION,
|
||||
TRANSPORT_FAILURE_BEFORE_API,
|
||||
assess_final_report_mutation_accounting,
|
||||
classify_mutation_attempt,
|
||||
record_attempt,
|
||||
summarize_attempt_ledger,
|
||||
)
|
||||
|
||||
# The two pre-API rejections observed on the #615 comment flow.
|
||||
MISSING_LEDGER_BLOCK = {
|
||||
"success": False,
|
||||
"performed": False,
|
||||
"api_called": False,
|
||||
"reasons": ["missing [THREAD STATE LEDGER] block"],
|
||||
}
|
||||
|
||||
MISSING_CANONICAL_STATE = {
|
||||
"success": False,
|
||||
"performed": False,
|
||||
"api_called": False,
|
||||
"reasons": ["missing ## Canonical Issue State block"],
|
||||
}
|
||||
|
||||
# The corrected comment that actually landed as #615 comment 9137.
|
||||
SUCCESSFUL_COMMENT = {
|
||||
"success": True,
|
||||
"performed": True,
|
||||
"api_called": True,
|
||||
"comment_id": 9137,
|
||||
"issue_number": 615,
|
||||
}
|
||||
|
||||
|
||||
class TestAC1Classification(unittest.TestCase):
|
||||
"""AC1: the four outcome classes are distinguished."""
|
||||
|
||||
def test_local_validator_rejection_is_its_own_class(self):
|
||||
result = classify_mutation_attempt(MISSING_LEDGER_BLOCK)
|
||||
self.assertEqual(result["classification"], LOCAL_VALIDATOR_REJECTION)
|
||||
self.assertTrue(result["pre_api"])
|
||||
|
||||
def test_capability_gate_rejection_is_its_own_class(self):
|
||||
result = classify_mutation_attempt(
|
||||
{
|
||||
"success": False,
|
||||
"api_called": False,
|
||||
"permission_report": {"missing_permission": "gitea.issue.comment"},
|
||||
}
|
||||
)
|
||||
self.assertEqual(result["classification"], CAPABILITY_GATE_REJECTION)
|
||||
self.assertTrue(result["pre_api"])
|
||||
|
||||
def test_transport_failure_before_api_is_its_own_class(self):
|
||||
result = classify_mutation_attempt(
|
||||
{"success": False, "api_called": False, "transport_error": "EOF"}
|
||||
)
|
||||
self.assertEqual(result["classification"], TRANSPORT_FAILURE_BEFORE_API)
|
||||
self.assertTrue(result["pre_api"])
|
||||
|
||||
def test_successful_server_mutation_is_its_own_class(self):
|
||||
result = classify_mutation_attempt(SUCCESSFUL_COMMENT)
|
||||
self.assertEqual(result["classification"], SERVER_SIDE_MUTATION)
|
||||
self.assertFalse(result["pre_api"])
|
||||
|
||||
def test_each_class_is_distinct(self):
|
||||
classes = {
|
||||
classify_mutation_attempt(payload)["classification"]
|
||||
for payload in (
|
||||
MISSING_LEDGER_BLOCK,
|
||||
{"success": False, "api_called": False, "capability_blocked": True},
|
||||
{"success": False, "api_called": False, "transport_failed": True},
|
||||
SUCCESSFUL_COMMENT,
|
||||
)
|
||||
}
|
||||
self.assertEqual(len(classes), 4)
|
||||
|
||||
|
||||
class TestAC2BudgetAccounting(unittest.TestCase):
|
||||
"""AC2: pre-API failures never consume server-side mutation budget."""
|
||||
|
||||
def test_missing_thread_state_ledger_not_counted_as_mutation(self):
|
||||
result = classify_mutation_attempt(MISSING_LEDGER_BLOCK)
|
||||
self.assertFalse(result["budget_consumed"])
|
||||
self.assertIs(result["api_called"], False)
|
||||
|
||||
def test_missing_canonical_issue_state_not_counted_as_mutation(self):
|
||||
result = classify_mutation_attempt(MISSING_CANONICAL_STATE)
|
||||
self.assertFalse(result["budget_consumed"])
|
||||
self.assertIs(result["api_called"], False)
|
||||
|
||||
def test_transport_failure_before_api_not_counted_as_mutation(self):
|
||||
result = classify_mutation_attempt(
|
||||
{
|
||||
"success": False,
|
||||
"api_called": False,
|
||||
"transport_error": "connection reset",
|
||||
}
|
||||
)
|
||||
self.assertFalse(result["budget_consumed"])
|
||||
|
||||
def test_capability_gate_block_not_counted_as_mutation(self):
|
||||
result = classify_mutation_attempt(
|
||||
{
|
||||
"success": False,
|
||||
"api_called": False,
|
||||
"permission_report": {"missing_permission": "gitea.pr.merge"},
|
||||
}
|
||||
)
|
||||
self.assertFalse(result["budget_consumed"])
|
||||
|
||||
def test_successful_comment_with_comment_id_counts_as_one_mutation(self):
|
||||
result = classify_mutation_attempt(SUCCESSFUL_COMMENT)
|
||||
self.assertTrue(result["budget_consumed"])
|
||||
self.assertEqual(result["proof_fields"], ["comment_id"])
|
||||
|
||||
|
||||
class TestAC2FailsClosed(unittest.TestCase):
|
||||
"""AC2 must not become a loophole: ambiguity still fails closed."""
|
||||
|
||||
def test_api_reached_without_proof_is_ambiguous_and_consumes_budget(self):
|
||||
result = classify_mutation_attempt({"success": True, "api_called": True})
|
||||
self.assertEqual(result["classification"], AMBIGUOUS_REQUIRES_READBACK)
|
||||
self.assertTrue(result["budget_consumed"])
|
||||
self.assertTrue(result["requires_readback"])
|
||||
|
||||
def test_missing_api_called_signal_fails_closed(self):
|
||||
result = classify_mutation_attempt({"success": False})
|
||||
self.assertEqual(result["classification"], AMBIGUOUS_REQUIRES_READBACK)
|
||||
self.assertTrue(result["budget_consumed"])
|
||||
self.assertIsNone(result["api_called"])
|
||||
|
||||
def test_empty_and_none_results_fail_closed(self):
|
||||
for payload in ({}, None):
|
||||
result = classify_mutation_attempt(payload)
|
||||
self.assertEqual(result["classification"], AMBIGUOUS_REQUIRES_READBACK)
|
||||
self.assertTrue(result["budget_consumed"])
|
||||
|
||||
def test_success_with_proof_counts_even_when_api_called_absent(self):
|
||||
result = classify_mutation_attempt({"success": True, "comment_id": 13320})
|
||||
self.assertEqual(result["classification"], SERVER_SIDE_MUTATION)
|
||||
self.assertTrue(result["budget_consumed"])
|
||||
|
||||
def test_blank_proof_field_is_not_proof(self):
|
||||
result = classify_mutation_attempt(
|
||||
{"success": True, "api_called": True, "merge_commit_sha": " "}
|
||||
)
|
||||
self.assertEqual(result["classification"], AMBIGUOUS_REQUIRES_READBACK)
|
||||
|
||||
|
||||
class TestAC3AttemptLedger(unittest.TestCase):
|
||||
"""AC3: failed attempts are still logged locally."""
|
||||
|
||||
def test_failed_attempts_are_recorded(self):
|
||||
ledger: list[dict] = []
|
||||
record_attempt(ledger, MISSING_LEDGER_BLOCK, operation="create_issue_comment")
|
||||
record_attempt(ledger, MISSING_CANONICAL_STATE, operation="create_issue_comment")
|
||||
self.assertEqual(len(ledger), 2)
|
||||
self.assertTrue(
|
||||
all(e["classification"] == LOCAL_VALIDATOR_REJECTION for e in ledger)
|
||||
)
|
||||
|
||||
def test_recorded_entry_carries_operation_and_timestamp(self):
|
||||
ledger: list[dict] = []
|
||||
entry = record_attempt(
|
||||
ledger,
|
||||
SUCCESSFUL_COMMENT,
|
||||
operation="create_issue_comment",
|
||||
timestamp="2026-07-20T18:15:04+00:00",
|
||||
)
|
||||
self.assertEqual(entry["operation"], "create_issue_comment")
|
||||
self.assertEqual(entry["timestamp"], "2026-07-20T18:15:04+00:00")
|
||||
|
||||
def test_timestamp_is_generated_when_omitted(self):
|
||||
ledger: list[dict] = []
|
||||
entry = record_attempt(ledger, SUCCESSFUL_COMMENT)
|
||||
self.assertTrue(entry["timestamp"])
|
||||
|
||||
|
||||
class TestAC5CorrectedCommentAllowed(unittest.TestCase):
|
||||
"""AC5: the #615 reproduction — two local rejections then one success."""
|
||||
|
||||
def _replay_615_flow(self) -> list[dict]:
|
||||
ledger: list[dict] = []
|
||||
record_attempt(ledger, MISSING_LEDGER_BLOCK, operation="create_issue_comment")
|
||||
record_attempt(ledger, MISSING_CANONICAL_STATE, operation="create_issue_comment")
|
||||
record_attempt(ledger, SUCCESSFUL_COMMENT, operation="create_issue_comment")
|
||||
return ledger
|
||||
|
||||
def test_corrected_comment_after_two_rejections_is_allowed(self):
|
||||
summary = summarize_attempt_ledger(self._replay_615_flow())
|
||||
# The regression: budget must show ONE mutation, not three attempts.
|
||||
self.assertEqual(summary["successful_server_mutations"], 1)
|
||||
self.assertEqual(summary["budget_consumed"], 1)
|
||||
|
||||
def test_all_three_attempts_remain_visible(self):
|
||||
summary = summarize_attempt_ledger(self._replay_615_flow())
|
||||
self.assertEqual(summary["total_attempts"], 3)
|
||||
self.assertEqual(summary["local_failed_attempts"], 2)
|
||||
|
||||
def test_no_readback_required_for_clean_flow(self):
|
||||
summary = summarize_attempt_ledger(self._replay_615_flow())
|
||||
self.assertFalse(summary["requires_readback"])
|
||||
|
||||
|
||||
class TestAC4FinalReportAccounting(unittest.TestCase):
|
||||
"""AC4: the report must show the three categories, and match the ledger."""
|
||||
|
||||
def _mixed_ledger(self) -> list[dict]:
|
||||
ledger: list[dict] = []
|
||||
record_attempt(ledger, MISSING_LEDGER_BLOCK, operation="comment")
|
||||
record_attempt(ledger, MISSING_CANONICAL_STATE, operation="comment")
|
||||
record_attempt(
|
||||
ledger,
|
||||
{"success": False, "api_called": False, "transport_error": "EOF"},
|
||||
operation="comment",
|
||||
)
|
||||
record_attempt(
|
||||
ledger,
|
||||
{"success": False, "api_called": False, "capability_blocked": True},
|
||||
operation="merge",
|
||||
)
|
||||
record_attempt(ledger, SUCCESSFUL_COMMENT, operation="comment")
|
||||
return ledger
|
||||
|
||||
def test_summary_separates_the_three_categories(self):
|
||||
summary = summarize_attempt_ledger(self._mixed_ledger())
|
||||
self.assertEqual(summary["local_failed_attempts"], 2)
|
||||
self.assertEqual(summary["blocked_api_attempts"], 2)
|
||||
self.assertEqual(summary["successful_server_mutations"], 1)
|
||||
|
||||
def test_matching_report_is_valid(self):
|
||||
result = assess_final_report_mutation_accounting(
|
||||
{
|
||||
"local_failed_attempts": 2,
|
||||
"blocked_api_attempts": 2,
|
||||
"successful_server_mutations": 1,
|
||||
},
|
||||
self._mixed_ledger(),
|
||||
)
|
||||
self.assertTrue(result["valid"], result["reasons"])
|
||||
|
||||
def test_omitted_category_fails_closed(self):
|
||||
result = assess_final_report_mutation_accounting(
|
||||
{"local_failed_attempts": 2, "blocked_api_attempts": 2},
|
||||
self._mixed_ledger(),
|
||||
)
|
||||
self.assertFalse(result["valid"])
|
||||
self.assertTrue(
|
||||
any("successful_server_mutations" in r for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_inflated_mutation_count_fails_closed(self):
|
||||
# The #617 bug shape: claiming three mutations when only one landed.
|
||||
result = assess_final_report_mutation_accounting(
|
||||
{
|
||||
"local_failed_attempts": 2,
|
||||
"blocked_api_attempts": 2,
|
||||
"successful_server_mutations": 3,
|
||||
},
|
||||
self._mixed_ledger(),
|
||||
)
|
||||
self.assertFalse(result["valid"])
|
||||
self.assertTrue(
|
||||
any("successful_server_mutations=3" in r for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_ambiguous_attempt_requires_readback_proof(self):
|
||||
ledger: list[dict] = []
|
||||
record_attempt(ledger, {"success": True, "api_called": True}, operation="comment")
|
||||
report = {
|
||||
"local_failed_attempts": 0,
|
||||
"blocked_api_attempts": 0,
|
||||
"successful_server_mutations": 0,
|
||||
}
|
||||
blocked = assess_final_report_mutation_accounting(report, ledger)
|
||||
self.assertFalse(blocked["valid"])
|
||||
self.assertTrue(any("readback_verified" in r for r in blocked["reasons"]))
|
||||
|
||||
allowed = assess_final_report_mutation_accounting(
|
||||
{**report, "readback_verified": True}, ledger
|
||||
)
|
||||
self.assertTrue(allowed["valid"], allowed["reasons"])
|
||||
|
||||
def test_ledger_summary_is_returned_without_raw_entries(self):
|
||||
result = assess_final_report_mutation_accounting({}, self._mixed_ledger())
|
||||
self.assertNotIn("entries", result["ledger_summary"])
|
||||
self.assertEqual(result["ledger_summary"]["total_attempts"], 5)
|
||||
|
||||
|
||||
class TestEmptyLedger(unittest.TestCase):
|
||||
def test_empty_ledger_summarizes_to_zero(self):
|
||||
summary = summarize_attempt_ledger([])
|
||||
self.assertEqual(summary["total_attempts"], 0)
|
||||
self.assertEqual(summary["successful_server_mutations"], 0)
|
||||
self.assertFalse(summary["requires_readback"])
|
||||
|
||||
def test_none_ledger_is_tolerated(self):
|
||||
self.assertEqual(summarize_attempt_ledger(None)["total_attempts"], 0)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
|
||||
class TestValidatorIntegration(unittest.TestCase):
|
||||
"""The classifier is wired into the shared final-report validator (AC4)."""
|
||||
|
||||
def _ledger_two_rejections_one_success(self) -> list[dict]:
|
||||
ledger: list[dict] = []
|
||||
record_attempt(ledger, MISSING_LEDGER_BLOCK, operation="comment")
|
||||
record_attempt(ledger, MISSING_CANONICAL_STATE, operation="comment")
|
||||
record_attempt(ledger, SUCCESSFUL_COMMENT, operation="comment")
|
||||
return ledger
|
||||
|
||||
def test_rule_is_noop_without_a_ledger(self):
|
||||
from final_report_validator import assess_final_report_validator
|
||||
|
||||
result = assess_final_report_validator("some report", "review_pr")
|
||||
self.assertFalse(
|
||||
any(
|
||||
f["rule_id"] == "shared.mutation_budget_accounting"
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
def test_report_matching_ledger_produces_no_finding(self):
|
||||
from final_report_validator import assess_final_report_validator
|
||||
|
||||
report = (
|
||||
"Local failed attempts: 2\n"
|
||||
"Blocked API attempts: 0\n"
|
||||
"Successful server-side mutations: 1\n"
|
||||
)
|
||||
result = assess_final_report_validator(
|
||||
report,
|
||||
"review_pr",
|
||||
mutation_attempt_ledger=self._ledger_two_rejections_one_success(),
|
||||
)
|
||||
self.assertFalse(
|
||||
any(
|
||||
f["rule_id"] == "shared.mutation_budget_accounting"
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
def test_counting_rejections_as_mutations_is_blocked(self):
|
||||
from final_report_validator import assess_final_report_validator
|
||||
|
||||
# The #617 bug: three attempts reported as three server-side mutations.
|
||||
report = (
|
||||
"Local failed attempts: 0\n"
|
||||
"Blocked API attempts: 0\n"
|
||||
"Successful server-side mutations: 3\n"
|
||||
)
|
||||
result = assess_final_report_validator(
|
||||
report,
|
||||
"review_pr",
|
||||
mutation_attempt_ledger=self._ledger_two_rejections_one_success(),
|
||||
)
|
||||
findings = [
|
||||
f
|
||||
for f in result["findings"]
|
||||
if f["rule_id"] == "shared.mutation_budget_accounting"
|
||||
]
|
||||
self.assertTrue(findings)
|
||||
self.assertTrue(all(f["severity"] == "block" for f in findings))
|
||||
Reference in New Issue
Block a user