fix(allocator): pre-rank exclusions and candidates_json transport (#776)
Expose exclude_issue_numbers on gitea_allocate_next_work, remove excluded numbers before ranking, normalize decoded-list and JSON-string candidates_json fail-closed, and return candidate-set fingerprints for dry-run/apply CAS. Same-owner leases on excluded issues surface a structured resume/release blocker. Closes #776 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Claude Opus 4.8
@@ -0,0 +1,349 @@
|
||||
"""Allocator pre-rank exclusions and candidates_json transport (#776).
|
||||
|
||||
Covers:
|
||||
* #617 excluded before ranking (never leased when exclude_issue_numbers=[617]);
|
||||
* excluded top candidate selects the next safe candidate;
|
||||
* all candidates excluded → WAIT, no lease;
|
||||
* decoded-list and JSON-string candidates_json;
|
||||
* malformed / type-invalid fail-closed cases;
|
||||
* dry-run/apply fingerprint match and drift rejection;
|
||||
* foreign lease and same-owner lease on excluded issue;
|
||||
* skipped-accounting reason parity (excluded_by_controller);
|
||||
* public MCP entry-point coverage for exclude_issue_numbers.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import gitea_mcp_server as srv
|
||||
from allocator_service import (
|
||||
OUTCOME_ASSIGNED,
|
||||
OUTCOME_BLOCKED_EXCLUDED_OWN_LEASE,
|
||||
OUTCOME_CANDIDATE_SET_DRIFT,
|
||||
OUTCOME_NO_SAFE,
|
||||
OUTCOME_PREVIEW,
|
||||
OUTCOME_WAIT,
|
||||
SKIP_CLAIMED_BY_OTHER_SESSION,
|
||||
SKIP_EXCLUDED_BY_CONTROLLER,
|
||||
WorkCandidate,
|
||||
allocate_next_work,
|
||||
candidate_from_dict,
|
||||
candidate_set_fingerprint,
|
||||
normalize_candidates_payload,
|
||||
normalize_exclude_issue_numbers,
|
||||
)
|
||||
from control_plane_db import ControlPlaneDB
|
||||
|
||||
REMOTE = "prgs"
|
||||
ORG = "Scaled-Tech-Consulting"
|
||||
REPO = "Gitea-Tools"
|
||||
MINE = "ctl-mine-776"
|
||||
THEIRS = "ctl-theirs-776"
|
||||
|
||||
|
||||
def _issue(number: int, **kwargs) -> WorkCandidate:
|
||||
base = dict(
|
||||
kind="issue",
|
||||
number=number,
|
||||
state="open",
|
||||
labels=("status:ready", "type:bug"),
|
||||
title=f"issue {number}",
|
||||
priority=20,
|
||||
)
|
||||
base.update(kwargs)
|
||||
return WorkCandidate(**base)
|
||||
|
||||
|
||||
def _claim(number: int, *, session_id: str, instance: str | None, kind: str = "issue"):
|
||||
return {
|
||||
"lease_id": f"lease-{number}",
|
||||
"session_id": session_id,
|
||||
"controller_instance_id": instance,
|
||||
"role": "author",
|
||||
"profile": "prgs-author",
|
||||
"expires_at": "2026-07-21T12:00:00Z",
|
||||
"work_kind": kind,
|
||||
"work_number": number,
|
||||
}
|
||||
|
||||
|
||||
def _cand_dict(number: int, **kwargs) -> dict:
|
||||
d = {
|
||||
"kind": "issue",
|
||||
"number": number,
|
||||
"state": "open",
|
||||
"labels": ["status:ready", "type:bug"],
|
||||
"title": f"issue {number}",
|
||||
"priority": 20,
|
||||
}
|
||||
d.update(kwargs)
|
||||
return d
|
||||
|
||||
|
||||
class AllocatorExcludeServiceTest(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self._tmp.cleanup)
|
||||
self.db = ControlPlaneDB(os.path.join(self._tmp.name, "cp.sqlite3"))
|
||||
|
||||
def _alloc(self, candidates, **kwargs):
|
||||
defaults = dict(
|
||||
session_id="sess-776",
|
||||
role="author",
|
||||
remote=REMOTE,
|
||||
org=ORG,
|
||||
repo=REPO,
|
||||
profile_name="prgs-author",
|
||||
controller_instance_id=MINE,
|
||||
claims={},
|
||||
apply=False,
|
||||
)
|
||||
defaults.update(kwargs)
|
||||
return allocate_next_work(self.db, candidates=candidates, **defaults)
|
||||
|
||||
def test_exclude_617_before_ranking_never_selects(self) -> None:
|
||||
"""AC2/AC8: highest-ranked #617 is removed before ranking."""
|
||||
cands = [_issue(617), _issue(700), _issue(701)]
|
||||
res = self._alloc(cands, exclude_issue_numbers=[617])
|
||||
self.assertEqual(res["outcome"], OUTCOME_PREVIEW)
|
||||
self.assertEqual(res["selected"]["number"], 700)
|
||||
skipped = {s["number"]: s for s in res["skipped"]}
|
||||
self.assertIn(617, skipped)
|
||||
self.assertEqual(
|
||||
skipped[617]["reason_code"], SKIP_EXCLUDED_BY_CONTROLLER
|
||||
)
|
||||
self.assertIn(SKIP_EXCLUDED_BY_CONTROLLER, skipped[617]["reason"])
|
||||
|
||||
def test_excluded_top_selects_next_safe(self) -> None:
|
||||
"""AC2: excluding the oldest ready issue promotes the next number."""
|
||||
cands = [_issue(600), _issue(601), _issue(602)]
|
||||
res = self._alloc(cands, exclude_issue_numbers=[600])
|
||||
self.assertEqual(res["selected"]["number"], 601)
|
||||
|
||||
def test_all_candidates_excluded_wait_no_lease(self) -> None:
|
||||
"""AC7: every candidate excluded → WAIT, no assignment."""
|
||||
cands = [_issue(617), _issue(700)]
|
||||
res = self._alloc(cands, exclude_issue_numbers=[617, 700], apply=True)
|
||||
self.assertEqual(res["outcome"], OUTCOME_WAIT)
|
||||
self.assertIsNone(res["selected"])
|
||||
self.assertIsNone(res["assignment"])
|
||||
self.assertEqual(len(res["controller_excluded"]), 2)
|
||||
|
||||
def test_omit_exclude_retains_existing_behavior(self) -> None:
|
||||
"""AC1/AC9: omit exclusions → #617 still wins when oldest ready."""
|
||||
cands = [_issue(617), _issue(700)]
|
||||
res = self._alloc(cands)
|
||||
self.assertEqual(res["selected"]["number"], 617)
|
||||
self.assertEqual(res.get("exclude_issue_numbers"), [])
|
||||
|
||||
def test_foreign_lease_still_skipped(self) -> None:
|
||||
"""AC6/AC9: foreign claims keep SKIP_CLAIMED_BY_OTHER_SESSION."""
|
||||
cands = [_issue(617), _issue(700)]
|
||||
claims = {
|
||||
("issue", 700): _claim(700, session_id="other", instance=THEIRS),
|
||||
}
|
||||
res = self._alloc(
|
||||
cands, exclude_issue_numbers=[617], claims=claims
|
||||
)
|
||||
# 617 excluded, 700 foreign → wait, no selection
|
||||
self.assertEqual(res["outcome"], OUTCOME_WAIT)
|
||||
self.assertIsNone(res["selected"])
|
||||
codes = {s["reason_code"] for s in res["skipped"]}
|
||||
self.assertIn(SKIP_EXCLUDED_BY_CONTROLLER, codes)
|
||||
self.assertIn(SKIP_CLAIMED_BY_OTHER_SESSION, codes)
|
||||
|
||||
def test_same_owner_lease_on_excluded_blocks_resume_release(self) -> None:
|
||||
"""AC5: excluded + live same-owner lease → structured blocker."""
|
||||
cands = [_issue(617), _issue(700)]
|
||||
claims = {
|
||||
("issue", 617): _claim(617, session_id="sess-776", instance=MINE),
|
||||
}
|
||||
res = self._alloc(
|
||||
cands, exclude_issue_numbers=[617], claims=claims, apply=True
|
||||
)
|
||||
self.assertEqual(res["outcome"], OUTCOME_BLOCKED_EXCLUDED_OWN_LEASE)
|
||||
self.assertIsNone(res["assignment"])
|
||||
self.assertEqual(res["blocked_lease"]["number"], 617)
|
||||
self.assertIn("resume", res["blocked_lease"]["safe_next_action"])
|
||||
|
||||
def test_dry_run_apply_fingerprint_match(self) -> None:
|
||||
"""AC4: dry-run and apply share the same fingerprint."""
|
||||
cands = [_issue(617), _issue(700)]
|
||||
dry = self._alloc(cands, exclude_issue_numbers=[617], apply=False)
|
||||
apply_res = self._alloc(
|
||||
cands,
|
||||
exclude_issue_numbers=[617],
|
||||
apply=True,
|
||||
expected_candidate_set_fingerprint=dry["candidate_set_fingerprint"],
|
||||
)
|
||||
self.assertEqual(
|
||||
dry["candidate_set_fingerprint"],
|
||||
apply_res["candidate_set_fingerprint"],
|
||||
)
|
||||
self.assertEqual(apply_res["outcome"], OUTCOME_ASSIGNED)
|
||||
self.assertEqual(apply_res["selected"]["number"], 700)
|
||||
|
||||
def test_apply_rejects_fingerprint_drift(self) -> None:
|
||||
"""AC4: material candidate-set drift fails closed on apply."""
|
||||
cands = [_issue(617), _issue(700)]
|
||||
res = self._alloc(
|
||||
cands,
|
||||
exclude_issue_numbers=[617],
|
||||
apply=True,
|
||||
expected_candidate_set_fingerprint="0" * 64,
|
||||
)
|
||||
self.assertFalse(res["success"])
|
||||
self.assertEqual(res["outcome"], OUTCOME_CANDIDATE_SET_DRIFT)
|
||||
self.assertIsNone(res["assignment"])
|
||||
|
||||
def test_fingerprint_stable_helper(self) -> None:
|
||||
cands = [_issue(700), _issue(617)]
|
||||
a = candidate_set_fingerprint(cands, exclude_issue_numbers=[617])
|
||||
b = candidate_set_fingerprint(
|
||||
list(reversed(cands)), exclude_issue_numbers=[617]
|
||||
)
|
||||
self.assertEqual(a, b)
|
||||
|
||||
def test_normalize_exclude_rejects_bool(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
normalize_exclude_issue_numbers([True])
|
||||
|
||||
def test_normalize_exclude_rejects_scalar(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
normalize_exclude_issue_numbers(617)
|
||||
|
||||
|
||||
class CandidatesJsonNormalizeTest(unittest.TestCase):
|
||||
def test_decoded_list(self) -> None:
|
||||
"""AC3: already-decoded list from MCP transport."""
|
||||
cands = normalize_candidates_payload([_cand_dict(617), _cand_dict(700)])
|
||||
self.assertEqual([c.number for c in cands], [617, 700])
|
||||
|
||||
def test_json_string(self) -> None:
|
||||
"""AC3: backward-compatible JSON string."""
|
||||
raw = json.dumps([_cand_dict(617)])
|
||||
cands = normalize_candidates_payload(raw)
|
||||
self.assertEqual(cands[0].number, 617)
|
||||
|
||||
def test_malformed_json_fail_closed(self) -> None:
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
normalize_candidates_payload("{not json")
|
||||
self.assertIn("malformed", str(ctx.exception).lower())
|
||||
|
||||
def test_scalar_fail_closed(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
normalize_candidates_payload(42)
|
||||
|
||||
def test_bool_number_fail_closed(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
normalize_candidates_payload([_cand_dict(True)]) # type: ignore[arg-type]
|
||||
|
||||
def test_invalid_record_fail_closed(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
normalize_candidates_payload(["not-a-dict"])
|
||||
|
||||
def test_object_not_list_fail_closed(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
normalize_candidates_payload(json.dumps({"number": 1}))
|
||||
|
||||
def test_candidate_from_dict_rejects_bool_number(self) -> None:
|
||||
with self.assertRaises(ValueError):
|
||||
candidate_from_dict({"kind": "issue", "number": True})
|
||||
|
||||
|
||||
class AllocateNextWorkMcpExcludeTest(unittest.TestCase):
|
||||
"""Public MCP entry-point coverage (#776 AC8)."""
|
||||
|
||||
def setUp(self) -> None:
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.db = ControlPlaneDB(os.path.join(self._tmp.name, "cp.sqlite3"))
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self._tmp.cleanup()
|
||||
|
||||
def _call(self, **kwargs):
|
||||
with patch("gitea_mcp_server._profile_operation_gate", return_value=None), patch(
|
||||
"gitea_mcp_server._resolve", return_value=("h", ORG, REPO)
|
||||
), patch(
|
||||
"gitea_mcp_server.get_profile",
|
||||
return_value={"profile_name": "prgs-author", "role": "author"},
|
||||
), patch(
|
||||
"gitea_mcp_server._authenticated_username", return_value="jcwalker3"
|
||||
), patch(
|
||||
"gitea_mcp_server._control_plane_db_or_error", return_value=(self.db, [])
|
||||
), patch(
|
||||
"gitea_mcp_server.sentry_observability.monitor_checkin", return_value=None
|
||||
):
|
||||
return srv.gitea_allocate_next_work(
|
||||
remote="prgs", org=ORG, repo=REPO, role="author", **kwargs
|
||||
)
|
||||
|
||||
def test_mcp_exclude_617_decoded_list_never_selects(self) -> None:
|
||||
"""AC8: public tool with decoded list + exclude_issue_numbers=[617]."""
|
||||
candidates = [_cand_dict(617), _cand_dict(700)]
|
||||
res = self._call(
|
||||
candidates_json=candidates,
|
||||
exclude_issue_numbers=[617],
|
||||
apply=False,
|
||||
)
|
||||
self.assertTrue(res.get("success"), res)
|
||||
self.assertEqual(res["selected"]["number"], 700)
|
||||
skipped = {s["number"]: s for s in res["skipped"]}
|
||||
self.assertEqual(
|
||||
skipped[617]["reason_code"], SKIP_EXCLUDED_BY_CONTROLLER
|
||||
)
|
||||
self.assertNotEqual(res["selected"]["number"], 617)
|
||||
|
||||
def test_mcp_exclude_617_json_string_apply(self) -> None:
|
||||
"""AC8: JSON-string transport + apply never leases #617."""
|
||||
raw = json.dumps([_cand_dict(617), _cand_dict(700)])
|
||||
res = self._call(
|
||||
candidates_json=raw,
|
||||
exclude_issue_numbers=[617],
|
||||
apply=True,
|
||||
)
|
||||
self.assertEqual(res["outcome"], OUTCOME_ASSIGNED)
|
||||
self.assertEqual(res["assignment"]["work_number"], 700)
|
||||
self.assertNotEqual(res["selected"]["number"], 617)
|
||||
|
||||
def test_mcp_malformed_candidates_json_fail_closed(self) -> None:
|
||||
res = self._call(candidates_json="{bad", apply=False)
|
||||
self.assertFalse(res["success"])
|
||||
self.assertIsNone(res["assignment"])
|
||||
self.assertTrue(any("fail closed" in r for r in res["reasons"]))
|
||||
|
||||
def test_mcp_bool_number_fail_closed(self) -> None:
|
||||
res = self._call(
|
||||
candidates_json=[{"kind": "issue", "number": True, "priority": 20}],
|
||||
apply=False,
|
||||
)
|
||||
self.assertFalse(res["success"])
|
||||
self.assertIsNone(res["assignment"])
|
||||
|
||||
def test_mcp_fingerprint_dry_run_apply_parity(self) -> None:
|
||||
candidates = [_cand_dict(617), _cand_dict(700)]
|
||||
dry = self._call(
|
||||
candidates_json=candidates,
|
||||
exclude_issue_numbers=[617],
|
||||
apply=False,
|
||||
)
|
||||
apply_res = self._call(
|
||||
candidates_json=candidates,
|
||||
exclude_issue_numbers=[617],
|
||||
apply=True,
|
||||
expected_candidate_set_fingerprint=dry["candidate_set_fingerprint"],
|
||||
)
|
||||
self.assertEqual(
|
||||
dry["candidate_set_fingerprint"],
|
||||
apply_res["candidate_set_fingerprint"],
|
||||
)
|
||||
self.assertEqual(apply_res["selected"]["number"], 700)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user