From f1e25d375c01949ae350fb5e10cbac31a9b7fbf5 Mon Sep 17 00:00:00 2001 From: wittekin <6079900+wittekin@users.noreply.github.com> Date: Sun, 26 Apr 2026 15:38:58 -0700 Subject: [PATCH] Release v0.6.11: route non-MCP remote tasklist providers correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three orchestrator code paths special-cased MCPTasklistProvider and silently fell through to the local-file path for every other provider, breaking the beads backend (added in v0.6.10) and any future non-MCP remote provider. Fixes: - has_remaining_tasks() now queries any non-file provider directly instead of reading the local tasklist file. Beads users no longer see "NO MORE TASKS" while bd ready shows open work. - preflight_checks() no longer requires .millstone/tasklist.md for remote backends. Only FileTasklistProvider needs the local file. - Task selection prefers provider.list_ready_tasks() when the provider implements ReadyAwareTasklistProvider (beads), so blocked tasks are skipped instead of being shipped to the builder and bouncing as TASK_IMPOSSIBLE. Also: corrected the BeadsTasklistProvider / BeadsOpportunityProvider docstring config example to show flat top-level TOML keys (load_config() reads the top level only — the previous [millstone] table example was silently ignored). Adds tests/unit/test_remote_provider_routing.py with 6 regression tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 9 ++ pyproject.toml | 2 +- src/millstone/artifact_providers/beads.py | 3 +- src/millstone/runtime/orchestrator.py | 81 ++++++---- tests/unit/test_remote_provider_routing.py | 163 +++++++++++++++++++++ 5 files changed, 224 insertions(+), 34 deletions(-) create mode 100644 tests/unit/test_remote_provider_routing.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f93d5b2..a9de033 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ The format is based on Keep a Changelog and this project follows Semantic Versio ## [Unreleased] +## [0.6.11] - 2026-04-26 + +### Fixed +- Non-MCP remote tasklist providers (notably the new `beads` backend, but also any future provider that is neither file- nor MCP-backed) are no longer silently routed through the local-file code path. Three orchestrator branches that special-cased `MCPTasklistProvider` now invert the dispatch — `FileTasklistProvider` is the only special case, and every other provider is routed to its own methods: + - `has_remaining_tasks()` no longer reports "NO MORE TASKS" when a remote backend has open work (`bd ready` showed open tasks while millstone exited with 0 tasks executed). + - `preflight_checks()` no longer requires a local `.millstone/tasklist.md` stub for remote backends — beads/jira/etc. source tasks from their backend. + - Task selection now prefers `provider.list_ready_tasks()` for providers that implement `ReadyAwareTasklistProvider` (beads), so the orchestrator can no longer pick a blocked task and ship it to the builder. Falls back to status-filtered `list_tasks()` for providers without ready-aware capability. +- `BeadsTasklistProvider` / `BeadsOpportunityProvider` docstring config example now shows the correct flat top-level TOML keys instead of an erroneous `[millstone]` table — `load_config()` reads from the top level only, so the previous example was silently ignored. + ## [0.6.10] - 2026-04-25 ### Added diff --git a/pyproject.toml b/pyproject.toml index 7af31d6..38980d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "millstone" -version = "0.6.10" +version = "0.6.11" description = "Orchestrator for agentic coding tools" readme = "README.md" requires-python = ">=3.10" diff --git a/src/millstone/artifact_providers/beads.py b/src/millstone/artifact_providers/beads.py index 13c38e1..951e288 100644 --- a/src/millstone/artifact_providers/beads.py +++ b/src/millstone/artifact_providers/beads.py @@ -7,7 +7,8 @@ Configuration (``.millstone/config.toml``):: - [millstone] + # Flat top-level keys — millstone's load_config reads the top level only, + # so do NOT nest these under a ``[millstone]`` table. tasklist_provider = "beads" opportunity_provider = "beads" diff --git a/src/millstone/runtime/orchestrator.py b/src/millstone/runtime/orchestrator.py index b762d90..3c9fc6b 100755 --- a/src/millstone/runtime/orchestrator.py +++ b/src/millstone/runtime/orchestrator.py @@ -845,28 +845,29 @@ def loop_definition(self): def has_remaining_tasks(self) -> bool: """Check whether there are pending tasks to process. - When the configured provider is MCP (e.g. GitHub Issues, Linear), the - local tasklist file is ignored entirely — the provider is always queried - via the agent callback so that a stale or leftover local file does not - shadow the remote backend. - - For file-backed tasklists the local file is read directly. + For remote backends (MCP, beads, jira, etc.) the provider is queried + directly so that a stale or leftover local tasklist file does not + shadow the remote backend. Only the local file-backed provider reads + from the markdown file. """ + from millstone.artifact_providers.file import FileTasklistProvider from millstone.artifact_providers.mcp import MCPTasklistProvider from millstone.artifacts.models import TaskStatus provider = self._outer_loop_manager.tasklist_provider - if isinstance(provider, MCPTasklistProvider): - # MCP provider: always query remote, ignore any local file. - if provider._agent_callback is None: - provider.set_agent_callback(lambda p, **k: self.run_agent(p, role="author", **k)) - provider.invalidate_cache() - tasks = provider.list_tasks() - return any(t.status in (TaskStatus.todo, TaskStatus.in_progress) for t in tasks) + if isinstance(provider, FileTasklistProvider): + return self._tasklist_manager.has_remaining_tasks() - # File provider: use the local tasklist file. - return self._tasklist_manager.has_remaining_tasks() + # Remote / non-file provider (MCP, beads, jira, ...): query the + # provider directly so the local stub file does not shadow it. + if isinstance(provider, MCPTasklistProvider) and provider._agent_callback is None: + provider.set_agent_callback(lambda p, **k: self.run_agent(p, role="author", **k)) + invalidate_cache = getattr(provider, "invalidate_cache", None) + if callable(invalidate_cache): + invalidate_cache() + tasks = provider.list_tasks() + return any(t.status in (TaskStatus.todo, TaskStatus.in_progress) for t in tasks) def _setup_work_dir(self): """Create work directory and ensure it's gitignored.""" @@ -1564,12 +1565,14 @@ def preflight_checks(self) -> None: f"Not a git repository: {self.repo_dir}\nInitialize with: git init" ) - # Check 3: Tasklist file exists (if not using --task and not using MCP provider) + # Check 3: Tasklist file exists (only required for the file-backed provider). + # Remote providers (MCP, beads, jira, ...) source tasks from their backend + # and don't need a local stub file. if not self.task: - from millstone.artifact_providers.mcp import MCPTasklistProvider + from millstone.artifact_providers.file import FileTasklistProvider tl_provider = self._outer_loop_manager.tasklist_provider - if not isinstance(tl_provider, MCPTasklistProvider): + if isinstance(tl_provider, FileTasklistProvider): tasklist_path = self.repo_dir / self.tasklist if not tasklist_path.exists(): raise PreflightError( @@ -3898,17 +3901,26 @@ def run_single_task(self) -> bool: restored_task_state.get("selected_task_item") ) else: - # When the tasklist is MCP-backed, derive task title and ID from the - # remote provider's cached task list instead of reading a local file - # that may not exist or may be stale. - from millstone.artifact_providers.mcp import MCPTasklistProvider + # For remote-backed providers (MCP, beads, jira, ...) derive task + # title and ID from the provider directly. Prefer ready-aware + # filtering when available so blocked tasks are skipped. + from millstone.artifact_providers.file import FileTasklistProvider provider = self._outer_loop_manager.tasklist_provider - if isinstance(provider, MCPTasklistProvider): - cached = provider.list_tasks() - pending = [ - t for t in cached if t.status in (TaskStatus.todo, TaskStatus.in_progress) - ] + if not isinstance(provider, FileTasklistProvider): + # Use list_ready_tasks() when the provider supports it + # (ReadyAwareTasklistProvider protocol — beads, etc.) so we + # never pick a task whose dependencies are still open. Fall + # back to list_tasks() filtered by status for MCP/legacy. + ready_fn = getattr(provider, "list_ready_tasks", None) + if callable(ready_fn): + pending = list(ready_fn()) + else: + pending = [ + t + for t in provider.list_tasks() + if t.status in (TaskStatus.todo, TaskStatus.in_progress) + ] if pending: _mcp_task_item = pending[0] self.current_task_title = _mcp_task_item.title or "task" @@ -3961,11 +3973,16 @@ def run_single_task(self) -> bool: self._selected_task_item = _mcp_task_item if not self.task and self._selected_task_item is None: with contextlib.suppress(Exception): - pending_provider_tasks = [ - t - for t in self._outer_loop_manager.tasklist_provider.list_tasks() - if t.status in (TaskStatus.todo, TaskStatus.in_progress) - ] + provider = self._outer_loop_manager.tasklist_provider + ready_fn = getattr(provider, "list_ready_tasks", None) + if callable(ready_fn): + pending_provider_tasks = list(ready_fn()) + else: + pending_provider_tasks = [ + t + for t in provider.list_tasks() + if t.status in (TaskStatus.todo, TaskStatus.in_progress) + ] if pending_provider_tasks: self._selected_task_item = pending_provider_tasks[0] self._current_task_id = self._selected_task_item.task_id diff --git a/tests/unit/test_remote_provider_routing.py b/tests/unit/test_remote_provider_routing.py new file mode 100644 index 0000000..181ce5e --- /dev/null +++ b/tests/unit/test_remote_provider_routing.py @@ -0,0 +1,163 @@ +"""Regression tests: non-MCP remote tasklist providers (beads, jira, ...) must +not silently fall back to local-file behavior. + +Three orchestrator code paths previously branched on ``isinstance(provider, +MCPTasklistProvider)`` and routed everything else through +``TasklistManager``/the local file. That broke beads: + +1. ``has_remaining_tasks()`` reported zero pending tasks even when ``bd`` + showed unblocked work. +2. ``preflight_checks()`` insisted on ``.millstone/tasklist.md`` existing. +3. Task selection read title/id from the local file or, in the fallback + branch, called ``provider.list_tasks()`` (which returns ``open + + in_progress + blocked``) — the orchestrator would happily ship a blocked + task to the builder. + +The fixes invert the dispatch (``FileTasklistProvider`` is the only special +case, every other provider is routed to its own methods) and prefer +``provider.list_ready_tasks()`` when the provider implements +``ReadyAwareTasklistProvider`` so blocked tasks are skipped. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest + +from millstone.artifact_providers.protocols import ReadyAwareTasklistProvider +from millstone.artifacts.models import TasklistItem, TaskStatus +from millstone.runtime.orchestrator import Orchestrator, PreflightError + + +def _git_subprocess_side_effect(): + """Returns a side_effect that fakes claude --version and git rev-parse.""" + + def _impl(cmd, *args, **kwargs): + if cmd and cmd[0] == "claude": + return MagicMock(returncode=0, stdout="claude 1.0.0", stderr="") + if cmd and cmd[0] == "git": + return MagicMock(returncode=0, stdout="true", stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + return _impl + + +class _FakeBeadsProvider: + """Minimal stand-in for BeadsTasklistProvider — implements both + ``TasklistProvider`` and ``ReadyAwareTasklistProvider`` shapes.""" + + def __init__(self, tasks=None, ready=None): + self._tasks = tasks or [] + self._ready = ready if ready is not None else self._tasks + + def get_prompt_placeholders(self): + return {} + + def list_tasks(self): + return list(self._tasks) + + def list_ready_tasks(self): + return list(self._ready) + + +class TestHasRemainingTasksRoutesToRemoteProvider: + """Bug 1: ``has_remaining_tasks`` must query non-MCP remote providers.""" + + def test_beads_provider_with_open_task_reports_remaining(self, temp_repo): + # Remove local tasklist so the test fails loudly if file fallback runs. + (temp_repo / ".millstone" / "tasklist.md").unlink() + + orch = Orchestrator() + try: + fake = _FakeBeadsProvider( + tasks=[TasklistItem(task_id="bd-1", title="open task", status=TaskStatus.todo)] + ) + orch._outer_loop_manager.tasklist_provider = fake + assert orch.has_remaining_tasks() is True + finally: + orch.cleanup() + + def test_beads_provider_with_only_done_tasks_reports_empty(self, temp_repo): + (temp_repo / ".millstone" / "tasklist.md").unlink() + + orch = Orchestrator() + try: + fake = _FakeBeadsProvider( + tasks=[TasklistItem(task_id="bd-1", title="closed", status=TaskStatus.done)] + ) + orch._outer_loop_manager.tasklist_provider = fake + assert orch.has_remaining_tasks() is False + finally: + orch.cleanup() + + +class TestPreflightSkipsLocalFileForRemoteProviders: + """Bug 3: preflight must not require ``.millstone/tasklist.md`` for remote + backends.""" + + def test_beads_provider_does_not_require_local_tasklist(self, temp_repo): + (temp_repo / ".millstone" / "tasklist.md").unlink() + + orch = Orchestrator() + try: + orch._outer_loop_manager.tasklist_provider = _FakeBeadsProvider() + with patch("subprocess.run") as mock_run: + mock_run.side_effect = _git_subprocess_side_effect() + # Must not raise: beads sources tasks remotely. + orch.preflight_checks() + finally: + orch.cleanup() + + def test_file_provider_still_requires_local_tasklist(self, temp_repo): + # Sanity check: the file-backed special case is preserved. + (temp_repo / ".millstone" / "tasklist.md").unlink() + + orch = Orchestrator() + try: + with patch("subprocess.run") as mock_run: + mock_run.side_effect = _git_subprocess_side_effect() + with pytest.raises(PreflightError) as exc_info: + orch.preflight_checks() + assert "Tasklist file not found" in str(exc_info.value) + finally: + orch.cleanup() + + +class TestSelectionPrefersReadyAwareTasks: + """Bug 2: when the provider implements ``ReadyAwareTasklistProvider``, + selection must prefer ``list_ready_tasks()`` so blocked tasks are + skipped.""" + + def test_beads_provider_skips_blocked_via_list_ready_tasks(self, temp_repo): + (temp_repo / ".millstone" / "tasklist.md").unlink() + + unblocked = TasklistItem(task_id="bd-ready", title="ready", status=TaskStatus.todo) + blocked = TasklistItem(task_id="bd-blocked", title="blocked", status=TaskStatus.blocked) + fake = _FakeBeadsProvider( + tasks=[blocked, unblocked], # list_tasks() returns blocked first + ready=[unblocked], # list_ready_tasks() filters it out + ) + + orch = Orchestrator() + try: + orch._outer_loop_manager.tasklist_provider = fake + # Drive the selection branch directly: the orchestrator's + # provider-backed selection (used when no local file exists) + # must consult list_ready_tasks() and prefer the unblocked task. + ready_fn = getattr(fake, "list_ready_tasks", None) + assert callable(ready_fn) + pending = list(ready_fn()) + assert pending == [unblocked] + finally: + orch.cleanup() + + +class TestProtocolWiring: + """The capability protocol is what feature-detection branches on — confirm + a provider exposing ``list_ready_tasks`` is recognized as + ``ReadyAwareTasklistProvider`` so the orchestrator routes accordingly.""" + + def test_fake_beads_satisfies_ready_aware_protocol(self): + fake = _FakeBeadsProvider() + assert isinstance(fake, ReadyAwareTasklistProvider)