diff --git a/allocator_dependencies.py b/allocator_dependencies.py new file mode 100644 index 0000000..87d37e4 --- /dev/null +++ b/allocator_dependencies.py @@ -0,0 +1,150 @@ +"""Canonical dependency parsing and live-state resolution for the allocator (#758). + +The work allocator previously inferred dependency state from two lowercase +substrings (``"blocked on #"`` / ``"downstream of #"``). The repository's +canonical declaration form is a ``Depends:`` field inside the issue body's +linkage line, for example:: + + * Parent: #631 · Depends: #633, #634 · Related: #630, #434 + +That form matched neither substring, so dependency-blocked issues were emitted +as eligible candidates. This module replaces substring inference with: + +1. structured parsing of ``Depends:`` declarations into issue references, and +2. resolution of each reference against **live issue state**, never body text. + +Both halves fail closed: a reference whose state cannot be established makes +the owning candidate ineligible rather than assignable. + +No issue number is special-cased here (#758 AC4/AC14); the parser is driven +entirely by the declaration syntax. +""" + +from __future__ import annotations + +import re +from typing import Callable, Iterable + +# Live issue states, as reported by Gitea. +DEP_STATE_OPEN = "open" +DEP_STATE_CLOSED = "closed" + +# "Depends:" / "Depends on:" introduces the declaration. "Dependencies" does +# not match: after "depend" it continues with "e", not "s". +_DEPENDS_KEYWORD = re.compile(r"depends(?:\s+on)?\s*:?\s*", re.IGNORECASE) + +# Immediately after the keyword, consume only the contiguous run of issue +# references. Anchoring the run this way means the declaration ends naturally +# at the next separator ("·", newline) or sibling field ("Related:"), without +# needing to enumerate separators. +_DEP_RUN = re.compile( + r"\s*(#\d+(?:\s*(?:,|and|&)\s*#\d+)*)", + re.IGNORECASE, +) + +# Legacy marker retained so previously-recognized bodies keep working. +_LEGACY_BLOCKED = re.compile(r"blocked\s+on\s+#(\d+)", re.IGNORECASE) + +_ISSUE_REF = re.compile(r"#(\d+)") + + +def parse_dependency_refs(body: str | None) -> tuple[int, ...]: + """Extract declared dependency issue numbers from an issue *body*. + + Recognizes the canonical ``Depends: #N, #N`` field (including the + ``Depends on`` spelling) plus the legacy ``blocked on #N`` marker. + Returns references in first-seen order with duplicates removed. Malformed + or absent declarations yield an empty tuple rather than raising. + """ + if not body: + return () + + refs: list[int] = [] + + def _add(value: str) -> None: + number = int(value) + if number > 0 and number not in refs: + refs.append(number) + + for match in _DEPENDS_KEYWORD.finditer(body): + run = _DEP_RUN.match(body, match.end()) + if not run: + continue + for ref in _ISSUE_REF.findall(run.group(1)): + _add(ref) + + for ref in _LEGACY_BLOCKED.findall(body): + _add(ref) + + return tuple(refs) + + +def resolve_dependency_state( + refs: Iterable[int], + state_lookup: Callable[[int], str | None], + *, + subject: str = "candidate", +) -> dict: + """Resolve declared *refs* against live issue state. + + *state_lookup* maps an issue number to its live state string, or to + ``None`` when that evidence could not be obtained. A reference is: + + * **met** when live state is ``closed``; + * **unmet** when live state is any other live value (``open``, etc.); + * **unavailable** when state is ``None`` or the lookup raises. + + Unmet *and* unavailable both mark the candidate ineligible (#758 AC6/AC7): + allocation must never assume a dependency is satisfied. + """ + unmet: list[int] = [] + unavailable: list[int] = [] + met: list[int] = [] + + for ref in refs: + try: + number = int(ref) + except (TypeError, ValueError): + continue + try: + state = state_lookup(number) + except Exception: # noqa: BLE001 — unavailable evidence fails closed + state = None + normalized = (str(state).strip().lower() if state is not None else "") or None + if normalized is None: + unavailable.append(number) + elif normalized == DEP_STATE_CLOSED: + met.append(number) + else: + unmet.append(number) + + reason: str | None = None + if unmet and unavailable: + reason = ( + f"{subject} has unresolved dependencies " + f"{_fmt(unmet)} and unverifiable dependencies {_fmt(unavailable)} " + "(fail closed)" + ) + elif unmet: + reason = ( + f"{subject} depends on unresolved issue(s) {_fmt(unmet)}; " + "they are not closed" + ) + elif unavailable: + reason = ( + f"{subject} dependency evidence unavailable for {_fmt(unavailable)} " + "(fail closed)" + ) + + return { + "refs": tuple(int(x) for x in refs), + "met": tuple(met), + "unmet": tuple(unmet), + "unavailable": tuple(unavailable), + "dependency_unmet": bool(unmet or unavailable), + "reason": reason, + } + + +def _fmt(numbers: Iterable[int]) -> str: + return ", ".join(f"#{n}" for n in numbers) diff --git a/allocator_service.py b/allocator_service.py index 4a45564..bd35b73 100644 --- a/allocator_service.py +++ b/allocator_service.py @@ -41,6 +41,15 @@ OUTCOME_NO_SAFE = "no_safe_work" OUTCOME_ROLE_INELIGIBLE = "role_ineligible" OUTCOME_PREVIEW = "preview" # dry-run only (apply=false) +# Human-readable statement of how a winner is chosen (#758 AC10). Reported +# alongside allocator results so the flat status:ready tier and its +# oldest-number tie-break are explicit rather than incidental. +SELECTION_POLICY = ( + "rank complete inventory by (priority desc, PRs before issues, " + "number asc); status:ready issues share priority 20, so the oldest " + "eligible number wins ties; result limits never affect selection" +) + ROLE_AUTHOR = "author" ROLE_REVIEWER = "reviewer" ROLE_MERGER = "merger" @@ -242,7 +251,23 @@ def classify_skip( def sort_candidates(candidates: Sequence[WorkCandidate]) -> list[WorkCandidate]: - """Higher priority first; then lower number (older issues) for stability.""" + """Rank candidates deterministically (#758 AC10). + + Ordering key, in precedence order: + + 1. ``priority`` descending — the loader scores ``status:ready`` issues at + 20 and everything else at 1, so the ready queue ties at a single value + by design; + 2. PRs before issues — in-flight review work drains before new authoring; + 3. ``number`` ascending — oldest first, which is what actually breaks the + flat ``status:ready`` tie. + + Because the ready tier is intentionally flat, rule 3 decides most real + selections. That is only safe when ranking sees the *complete* candidate + inventory: truncating before this call silently redefines "oldest" as + "oldest among whatever survived the slice", which is the defect #758 + fixed. Callers must rank everything and bound reporting afterwards. + """ return sorted( candidates, key=lambda c: (-int(c.priority), c.kind != "pr", int(c.number)), diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index e5d2a6d..2fa0284 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -1765,6 +1765,7 @@ import review_workflow_load # noqa: E402 import mcp_session_state # noqa: E402 import stale_review_decision_lock # noqa: E402 import allocator_service # noqa: E402 +import allocator_dependencies # noqa: E402 import control_plane_db # noqa: E402 import lease_lifecycle # noqa: E402 import incident_bridge # noqa: E402 @@ -17794,16 +17795,24 @@ def _allocator_candidates_from_gitea( repo: str, include_issues: bool = True, include_prs: bool = True, - limit: int = 50, -) -> tuple[list[Any], list[str]]: - """Build allocator candidates from live Gitea open issues/PRs.""" +) -> tuple[list[Any], list[str], bool]: + """Build allocator candidates from live Gitea open issues/PRs. + + Returns ``(candidates, reasons, inventory_complete)``. The inventory is + assembled in full and is never truncated before ranking (#758 AC1/AC3): + any result/display bound belongs to the caller's reporting, not to + candidate construction. ``inventory_complete`` is False when a required + listing failed, so the caller can fail closed instead of ranking a + silently short candidate set. + """ reasons: list[str] = [] candidates: list[Any] = [] + inventory_complete = True try: h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) except Exception as exc: # noqa: BLE001 - return [], [f"failed to resolve Gitea target: {_redact(str(exc))}"] + return [], [f"failed to resolve Gitea target: {_redact(str(exc))}"], False if include_prs: try: @@ -17813,7 +17822,8 @@ def _allocator_candidates_from_gitea( except Exception as exc: # noqa: BLE001 reasons.append(f"failed to list open PRs: {_redact(str(exc))}") prs = [] - for pr in prs[: max(1, int(limit))]: + inventory_complete = False + for pr in prs: if not isinstance(pr, dict): continue number = pr.get("number") @@ -17885,7 +17895,39 @@ def _allocator_candidates_from_gitea( f"failed to list open issues: {_redact(str(exc2))}" ) issues = [] - for issue in issues[: max(1, int(limit))]: + inventory_complete = False + + # Live dependency evidence (#758 AC5). The complete open-issue listing + # already proves which references are still open; anything absent from + # it is confirmed by a targeted lookup rather than assumed closed, so a + # deleted or unreachable reference fails closed instead of passing. + open_issue_numbers = { + int(i["number"]) + for i in issues + if isinstance(i, dict) + and i.get("number") is not None + and i.get("pull_request") is None + } + dep_state_cache: dict[int, str | None] = {} + + def _issue_state(number: int) -> str | None: + if number in open_issue_numbers: + return allocator_dependencies.DEP_STATE_OPEN + if number in dep_state_cache: + return dep_state_cache[number] + state: str | None = None + try: + data = api_request( + "GET", f"{repo_api_url(h, o, r)}/issues/{number}", auth + ) + if isinstance(data, dict): + state = str(data.get("state") or "").strip().lower() or None + except Exception: # noqa: BLE001 — unavailable evidence fails closed + state = None + dep_state_cache[number] = state + return state + + for issue in issues: if not isinstance(issue, dict): continue # Pull requests also appear in /issues on Gitea — skip them. @@ -17903,21 +17945,15 @@ def _allocator_candidates_from_gitea( body = str(issue.get("body") or "") title = str(issue.get("title") or "") blocked = "status:blocked" in labels - # Explicit downstream dependency: #612 waits on #600 allocator. - dep_unmet = False - dep_reason = None - # Generic body markers for blocked-on unfinished deps. - # (Hard-coded #612→#600 block removed after #600 merged — #612.) - lower_body = body.lower() - if "blocked on #" in lower_body or "downstream of #" in lower_body: - # Only treat as unmet when body still marks a live open dependency - # pattern; callers may clear labels when deps complete. - dep_unmet = "blocked on #" in lower_body - dep_reason = ( - f"issue#{number} body marks an unmet dependency" - if dep_unmet - else None - ) + # #758: parse the canonical "Depends: #N, #N" declaration into + # structured references and resolve each against live issue state. + # Body substrings no longer decide dependency status. + dep_refs = allocator_dependencies.parse_dependency_refs(body) + dep_result = allocator_dependencies.resolve_dependency_state( + dep_refs, _issue_state, subject=f"issue#{number}" + ) + dep_unmet = dep_result["dependency_unmet"] + dep_reason = dep_result["reason"] try: candidates.append( allocator_service.WorkCandidate( @@ -17937,7 +17973,7 @@ def _allocator_candidates_from_gitea( f"skipped invalid issue candidate #{number}: {_redact(str(exc))}" ) - return candidates, reasons + return candidates, reasons, inventory_complete @mcp.tool() @@ -18253,15 +18289,32 @@ def gitea_allocate_next_work( "substrate": "control_plane_db", } else: - candidates, inv_reasons = _allocator_candidates_from_gitea( + candidates, inv_reasons, inventory_complete = _allocator_candidates_from_gitea( remote=remote, host=host, org=o, repo=r, include_issues=include_issues, include_prs=include_prs, - limit=limit, ) + # #758 AC3: ranking a silently short inventory could select the wrong + # candidate, so an incomplete listing is a fail-closed stop. + if not inventory_complete: + return { + "success": False, + "outcome": allocator_service.OUTCOME_NO_SAFE, + "apply": bool(apply), + "reasons": inv_reasons + + [ + "candidate inventory is incomplete; refusing to rank a " + "partial candidate set (fail closed, #758)" + ], + "inventory_complete": False, + "assignment": None, + "substrate": "control_plane_db", + "file_lock_only": False, + "comment_lease_only": False, + } sid = (session_id or "").strip() or ( f"{profile_name or 'session'}-{os.getpid()}-{uuid.uuid4().hex[:8]}" @@ -18284,6 +18337,24 @@ def gitea_allocate_next_work( result["inventory_source"] = ( "candidates_json" if candidates_json else "gitea_live" ) + result["inventory_complete"] = True + result["selection_policy"] = allocator_service.SELECTION_POLICY + # #758 AC2/AC10: `limit` bounds the reported skip list only — selection has + # already happened over the complete inventory above. Any truncation here is + # stated explicitly so a shortened report is never read as full coverage. + reported_limit = max(1, int(limit)) + skipped_all = result.get("skipped") or [] + result["skipped_total"] = len(skipped_all) + result["limit_applies_to"] = "reported_skip_list_only" + if len(skipped_all) > reported_limit: + result["skipped"] = skipped_all[:reported_limit] + result["skipped_report_truncated"] = True + result.setdefault("reasons", []).append( + f"skip list truncated for reporting: showing {reported_limit} of " + f"{len(skipped_all)} skipped candidates (selection unaffected)" + ) + else: + result["skipped_report_truncated"] = False # #606: watchdog check-ins for the recurring jobs this allocator run # performs — global stale-lease expiry, terminal-lock lookup, and the # allocator itself. Best-effort; a failed selection reports "error". diff --git a/tests/test_allocator_dependencies.py b/tests/test_allocator_dependencies.py new file mode 100644 index 0000000..205bc55 --- /dev/null +++ b/tests/test_allocator_dependencies.py @@ -0,0 +1,266 @@ +"""Dependency parsing/resolution and allocator completeness tests (#758). + +Covers the two defects behind #758: + +* Defect 1 — candidate truncation before ranking, which let a result-size + parameter change the winner. +* Defect 2 — dependency state inferred from body substrings, which emitted + canonical ``Depends:`` blocked issues as eligible. + +No production behavior is special-cased for any issue number (#758 AC14), so +these tests use synthetic issue numbers throughout. +""" + +from __future__ import annotations + +import os +import tempfile +import unittest + +import allocator_dependencies +from allocator_service import ( + OUTCOME_PREVIEW, + SELECTION_POLICY, + WorkCandidate, + allocate_next_work, + classify_skip, + sort_candidates, +) +from control_plane_db import ControlPlaneDB + +# The canonical linkage line this repository writes into issue bodies. +CANONICAL_BODY = ( + "## Dependencies and linkage\n\n" + "* Parent: #900 · Depends: #901, #902 · Related: #903, #904\n" +) + + +class ParseDependencyRefsTest(unittest.TestCase): + def test_parses_canonical_depends_field(self) -> None: + self.assertEqual( + allocator_dependencies.parse_dependency_refs(CANONICAL_BODY), + (901, 902), + ) + + def test_stops_at_sibling_field_and_ignores_related(self) -> None: + """"Related:" refs must never be treated as dependencies.""" + refs = allocator_dependencies.parse_dependency_refs(CANONICAL_BODY) + self.assertNotIn(903, refs) + self.assertNotIn(904, refs) + self.assertNotIn(900, refs) # Parent is not a dependency + + def test_single_reference(self) -> None: + body = "* Parent: #10 · Depends: #11 · Related: #12" + self.assertEqual(allocator_dependencies.parse_dependency_refs(body), (11,)) + + def test_depends_on_spelling_and_and_separator(self) -> None: + body = "Depends on #21 and #22\n" + self.assertEqual( + allocator_dependencies.parse_dependency_refs(body), (21, 22) + ) + + def test_newline_terminates_declaration(self) -> None: + body = "Depends: #31, #32\nRelated: #33\n" + self.assertEqual( + allocator_dependencies.parse_dependency_refs(body), (31, 32) + ) + + def test_legacy_blocked_on_marker_still_recognized(self) -> None: + body = "This work is blocked on #41 until that lands.\n" + self.assertEqual(allocator_dependencies.parse_dependency_refs(body), (41,)) + + def test_dependencies_heading_alone_is_not_a_declaration(self) -> None: + """"Dependencies and linkage" must not parse as "Depends".""" + body = "## Dependencies and linkage\n\n* Related: #51\n" + self.assertEqual(allocator_dependencies.parse_dependency_refs(body), ()) + + def test_deduplicates_and_preserves_order(self) -> None: + body = "Depends: #61, #62, #61\n" + self.assertEqual( + allocator_dependencies.parse_dependency_refs(body), (61, 62) + ) + + def test_malformed_and_empty_inputs(self) -> None: + for body in ("", None, "Depends:", "Depends: none", "Depends: TBD\n"): + self.assertEqual(allocator_dependencies.parse_dependency_refs(body), ()) + + +class ResolveDependencyStateTest(unittest.TestCase): + def test_open_dependency_is_unmet(self) -> None: + result = allocator_dependencies.resolve_dependency_state( + (901, 902), lambda n: "open", subject="issue#644" + ) + self.assertTrue(result["dependency_unmet"]) + self.assertEqual(result["unmet"], (901, 902)) + self.assertIn("#901", result["reason"]) + + def test_closed_dependencies_are_met(self) -> None: + result = allocator_dependencies.resolve_dependency_state( + (901, 902), lambda n: "closed" + ) + self.assertFalse(result["dependency_unmet"]) + self.assertEqual(result["met"], (901, 902)) + self.assertIsNone(result["reason"]) + + def test_mixed_open_and_closed_is_unmet(self) -> None: + states = {901: "closed", 902: "open"} + result = allocator_dependencies.resolve_dependency_state( + (901, 902), states.get + ) + self.assertTrue(result["dependency_unmet"]) + self.assertEqual(result["unmet"], (902,)) + self.assertEqual(result["met"], (901,)) + + def test_unavailable_evidence_fails_closed(self) -> None: + """AC7: unknown state must block, never pass.""" + result = allocator_dependencies.resolve_dependency_state( + (901,), lambda n: None + ) + self.assertTrue(result["dependency_unmet"]) + self.assertEqual(result["unavailable"], (901,)) + self.assertIn("fail closed", result["reason"]) + + def test_raising_lookup_fails_closed(self) -> None: + def boom(_n: int) -> str: + raise RuntimeError("lookup exploded") + + result = allocator_dependencies.resolve_dependency_state((901,), boom) + self.assertTrue(result["dependency_unmet"]) + self.assertEqual(result["unavailable"], (901,)) + + def test_no_refs_is_eligible(self) -> None: + result = allocator_dependencies.resolve_dependency_state((), lambda n: None) + self.assertFalse(result["dependency_unmet"]) + self.assertIsNone(result["reason"]) + + +class DependencyBlockedCandidateTest(unittest.TestCase): + """A dependency-blocked candidate must be skipped, not selected.""" + + def test_classify_skip_rejects_unmet_dependency(self) -> None: + candidate = WorkCandidate( + kind="issue", + number=644, + labels=("status:ready",), + priority=20, + dependency_unmet=True, + dependency_reason="issue#644 depends on unresolved issue(s) #633", + ) + reason = classify_skip(candidate, role="author", terminal_pr=None) + self.assertIsNotNone(reason) + self.assertIn("#633", reason) + + +class SelectionInvarianceTest(unittest.TestCase): + """AC1/AC2/AC11: ranking sees everything; result bounds cannot move the winner.""" + + 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() + + @staticmethod + def _ready_issue(number: int, **kw) -> WorkCandidate: + return WorkCandidate( + kind="issue", + number=number, + labels=("status:ready",), + priority=20, + title=f"issue {number}", + **kw, + ) + + def _preview(self, candidates): + return allocate_next_work( + self.db, + session_id="s-758", + role="author", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + candidates=candidates, + apply=False, + ) + + def test_more_than_fifty_candidates_lowest_number_wins(self) -> None: + """Winner is the oldest eligible issue across a >50 inventory.""" + candidates = [self._ready_issue(n) for n in range(600, 700)] # 100 items + result = self._preview(candidates) + self.assertEqual(result["outcome"], OUTCOME_PREVIEW) + self.assertEqual(result["selected"]["number"], 600) + + def test_selection_is_invariant_to_candidate_ordering(self) -> None: + """Ranking must not depend on the order the inventory arrived in.""" + forward = [self._ready_issue(n) for n in range(600, 700)] + reverse = list(reversed(forward)) + self.assertEqual( + self._preview(forward)["selected"]["number"], + self._preview(reverse)["selected"]["number"], + ) + + def test_truncating_inventory_changes_winner(self) -> None: + """Regression guard: this is exactly what pre-ranking slicing did. + + A 50-item slice of a 100-item inventory yields a different winner, so + any future reintroduction of pre-ranking truncation is detectable. + """ + full = [self._ready_issue(n) for n in range(600, 700)] + sliced = sorted(full, key=lambda c: -c.number)[:50] + self.assertNotEqual( + self._preview(full)["selected"]["number"], + self._preview(sliced)["selected"]["number"], + ) + + def test_blocked_first_candidate_falls_through_to_next(self) -> None: + """AC8: a blocked winner must not end the iteration.""" + blocked = self._ready_issue( + 600, + dependency_unmet=True, + dependency_reason="issue#600 depends on unresolved issue(s) #599", + ) + result = self._preview([blocked, self._ready_issue(601)]) + self.assertEqual(result["selected"]["number"], 601) + skipped = {s["number"] for s in result["skipped"]} + self.assertIn(600, skipped) + + def test_all_blocked_yields_no_safe_work(self) -> None: + candidates = [ + self._ready_issue( + n, dependency_unmet=True, dependency_reason=f"issue#{n} blocked" + ) + for n in range(600, 605) + ] + result = self._preview(candidates) + self.assertIsNone(result["selected"]) + self.assertEqual(len(result["skipped"]), 5) + + def test_dry_run_and_apply_select_identically(self) -> None: + """AC9: apply mode must not re-rank differently from preview.""" + candidates = [self._ready_issue(n) for n in range(600, 700)] + preview = self._preview(candidates) + applied = allocate_next_work( + self.db, + session_id="s-758-apply", + role="author", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + candidates=candidates, + apply=True, + ) + self.assertEqual( + preview["selected"]["number"], applied["selected"]["number"] + ) + + def test_sort_is_stable_and_documented(self) -> None: + ordered = sort_candidates( + [self._ready_issue(603), self._ready_issue(601), self._ready_issue(602)] + ) + self.assertEqual([c.number for c in ordered], [601, 602, 603]) + self.assertIn("never affect selection", SELECTION_POLICY) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_allocator_inventory_mcp.py b/tests/test_allocator_inventory_mcp.py new file mode 100644 index 0000000..91715b7 --- /dev/null +++ b/tests/test_allocator_inventory_mcp.py @@ -0,0 +1,227 @@ +"""MCP-level allocator inventory and dependency regressions (#758). + +Exercises ``_allocator_candidates_from_gitea`` and the ``gitea_allocate_next_work`` +tool end to end against a faked Gitea API, proving: + +* the complete open-issue inventory is ranked (no pre-ranking truncation); +* ``limit`` cannot change which candidate wins; +* canonical ``Depends:`` declarations are resolved from live issue state; +* unavailable dependency evidence fails closed; +* an incomplete listing fails closed instead of ranking a partial set. + +Issue numbers here are synthetic; no production number is special-cased. +""" + +from __future__ import annotations + +import os +import tempfile +import unittest +from unittest.mock import patch + +import gitea_mcp_server as srv +from control_plane_db import ControlPlaneDB + +FAKE_AUTH = "token REDACTED" +ORG = "Scaled-Tech-Consulting" +REPO = "Gitea-Tools" + + +def _issue(number: int, *, body: str = "", labels=("status:ready",)) -> dict: + return { + "number": number, + "title": f"issue {number}", + "body": body, + "labels": [{"name": name} for name in labels], + "state": "open", + } + + +def _depends_body(*refs: int) -> str: + joined = ", ".join(f"#{r}" for r in refs) + return f"## Dependencies and linkage\n\n* Parent: #999 · Depends: {joined}\n" + + +class _FakeGitea: + """Minimal stand-in for the two Gitea list endpoints plus issue lookups.""" + + def __init__(self, issues, *, closed=(), unavailable=(), fail_issue_list=False): + self.issues = list(issues) + self.closed = set(closed) + self.unavailable = set(unavailable) + self.fail_issue_list = fail_issue_list + self.lookups: list[int] = [] + + def api_get_all(self, url, _auth, **_kw): + if "/pulls" in url: + return [] + if self.fail_issue_list: + raise RuntimeError("issue listing failed") + return list(self.issues) + + def api_request(self, _method, url, _auth, **_kw): + number = int(url.rsplit("/", 1)[-1]) + self.lookups.append(number) + if number in self.unavailable: + raise RuntimeError("lookup failed") + if number in self.closed: + return {"number": number, "state": "closed"} + return {"number": number, "state": "open"} + + +class AllocatorInventoryTest(unittest.TestCase): + """Direct tests of the candidate loader.""" + + def _load(self, fake, **kwargs): + with patch("gitea_mcp_server._resolve", return_value=("h", ORG, REPO)), patch( + "gitea_mcp_server._auth", return_value=FAKE_AUTH + ), patch("gitea_mcp_server.api_get_all", side_effect=fake.api_get_all), patch( + "gitea_mcp_server.api_request", side_effect=fake.api_request + ): + return srv._allocator_candidates_from_gitea( + remote="prgs", host=None, org=ORG, repo=REPO, **kwargs + ) + + def test_full_inventory_above_fifty_is_ranked(self) -> None: + """AC1: all 73 open issues become candidates, not the first 50.""" + fake = _FakeGitea([_issue(n) for n in range(600, 673)]) + candidates, _reasons, complete = self._load(fake) + self.assertTrue(complete) + self.assertEqual(len(candidates), 73) + self.assertEqual(min(c.number for c in candidates), 600) + self.assertEqual(max(c.number for c in candidates), 672) + + def test_open_dependency_marks_candidate_unmet(self) -> None: + """AC4/AC5/AC6: canonical Depends on an open issue blocks the candidate.""" + fake = _FakeGitea( + [_issue(600, body=_depends_body(601, 602)), _issue(601), _issue(602)] + ) + candidates, _reasons, _complete = self._load(fake) + blocked = next(c for c in candidates if c.number == 600) + self.assertTrue(blocked.dependency_unmet) + self.assertIn("#601", blocked.dependency_reason) + + def test_closed_dependency_is_eligible(self) -> None: + """A dependency absent from the open list is confirmed closed, not assumed.""" + fake = _FakeGitea([_issue(600, body=_depends_body(500))], closed={500}) + candidates, _reasons, _complete = self._load(fake) + candidate = next(c for c in candidates if c.number == 600) + self.assertFalse(candidate.dependency_unmet) + self.assertIn(500, fake.lookups) # proved live, not inferred + + def test_unavailable_dependency_evidence_fails_closed(self) -> None: + """AC7: an unreachable dependency must block, never pass.""" + fake = _FakeGitea([_issue(600, body=_depends_body(500))], unavailable={500}) + candidates, _reasons, _complete = self._load(fake) + candidate = next(c for c in candidates if c.number == 600) + self.assertTrue(candidate.dependency_unmet) + self.assertIn("fail closed", candidate.dependency_reason) + + def test_dependency_state_lookups_are_cached(self) -> None: + """Repeated references resolve with a single live lookup.""" + fake = _FakeGitea( + [_issue(n, body=_depends_body(500)) for n in range(600, 610)], + closed={500}, + ) + self._load(fake) + self.assertEqual(fake.lookups.count(500), 1) + + def test_open_dependency_needs_no_lookup(self) -> None: + """The complete open listing already proves openness.""" + fake = _FakeGitea([_issue(600, body=_depends_body(601)), _issue(601)]) + self._load(fake) + self.assertNotIn(601, fake.lookups) + + def test_failed_issue_listing_reports_incomplete(self) -> None: + """AC3: a failed listing must not silently yield a short inventory.""" + fake = _FakeGitea([], fail_issue_list=True) + _candidates, reasons, complete = self._load(fake) + self.assertFalse(complete) + self.assertTrue(any("failed to list open issues" in r for r in reasons)) + + +class AllocateNextWorkToolTest(unittest.TestCase): + """End-to-end tests of the gitea_allocate_next_work MCP tool.""" + + 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 _allocate(self, fake, **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._auth", return_value=FAKE_AUTH), 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.api_get_all", side_effect=fake.api_get_all + ), patch( + "gitea_mcp_server.api_request", side_effect=fake.api_request + ), 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_limit_does_not_change_selection(self) -> None: + """AC2/AC11: the winner is identical at limit=1 and limit=300.""" + issues = [_issue(n) for n in range(600, 673)] # 73 candidates + low = self._allocate(_FakeGitea(issues), limit=1) + high = self._allocate(_FakeGitea(issues), limit=300) + self.assertEqual(low["selected"]["number"], high["selected"]["number"]) + self.assertEqual(low["selected"]["number"], 600) + self.assertEqual(low["candidate_count"], 73) + self.assertEqual(high["candidate_count"], 73) + + def test_dependency_blocked_winner_falls_through(self) -> None: + """AC8: a blocked highest-ranked issue yields the next eligible one.""" + issues = [ + _issue(600, body=_depends_body(601)), + _issue(601), + _issue(602), + ] + result = self._allocate(_FakeGitea(issues)) + # 600 is blocked by open 601; 601 itself is a valid candidate. + self.assertEqual(result["selected"]["number"], 601) + skipped = {s["number"] for s in result["skipped"]} + self.assertIn(600, skipped) + + def test_limit_truncates_only_the_reported_skip_list(self) -> None: + """A shortened report is labelled, never presented as full coverage.""" + issues = [_issue(n, body=_depends_body(999)) for n in range(600, 640)] + issues.append(_issue(999)) # open dependency blocks all of the above + issues.append(_issue(700)) # the one eligible candidate + result = self._allocate(_FakeGitea(issues), limit=5) + self.assertTrue(result["skipped_report_truncated"]) + self.assertEqual(len(result["skipped"]), 5) + self.assertGreater(result["skipped_total"], 5) + self.assertEqual(result["limit_applies_to"], "reported_skip_list_only") + + def test_incomplete_inventory_fails_closed(self) -> None: + """AC3: no selection is made from a partial candidate set.""" + result = self._allocate(_FakeGitea([], fail_issue_list=True)) + self.assertFalse(result["success"]) + self.assertFalse(result["inventory_complete"]) + self.assertIsNone(result["assignment"]) + self.assertTrue( + any("fail closed" in r for r in result["reasons"]), + result["reasons"], + ) + + def test_selection_policy_is_reported(self) -> None: + """AC10: tie-breaking is stated in the result, not left implicit.""" + result = self._allocate(_FakeGitea([_issue(600)])) + self.assertIn("selection_policy", result) + self.assertIn("number asc", result["selection_policy"]) + + +if __name__ == "__main__": + unittest.main()