forked from Rockachopa/Timmy-time-dashboard
feat: code quality audit + autoresearch integration + infra hardening (#150)
This commit is contained in:
committed by
GitHub
parent
fd0ede0d51
commit
ae3bb1cc21
@@ -27,12 +27,9 @@ import pytest
|
||||
|
||||
from integrations.paperclip.bridge import PaperclipBridge
|
||||
from integrations.paperclip.client import PaperclipClient
|
||||
from integrations.paperclip.models import (
|
||||
PaperclipIssue,
|
||||
)
|
||||
from integrations.paperclip.models import PaperclipIssue
|
||||
from integrations.paperclip.task_runner import TaskRunner
|
||||
|
||||
|
||||
# ── Constants ─────────────────────────────────────────────────────────────────
|
||||
|
||||
TIMMY_AGENT_ID = "agent-timmy"
|
||||
@@ -53,9 +50,7 @@ class StubOrchestrator:
|
||||
def __init__(self) -> None:
|
||||
self.calls: list[dict] = []
|
||||
|
||||
async def execute_task(
|
||||
self, task_id: str, description: str, context: dict
|
||||
) -> dict:
|
||||
async def execute_task(self, task_id: str, description: str, context: dict) -> dict:
|
||||
call_record = {
|
||||
"task_id": task_id,
|
||||
"description": description,
|
||||
@@ -121,8 +116,9 @@ def bridge(mock_client):
|
||||
@pytest.fixture
|
||||
def settings_patch():
|
||||
"""Patch settings for all task runner tests."""
|
||||
with patch("integrations.paperclip.task_runner.settings") as ts, \
|
||||
patch("integrations.paperclip.bridge.settings") as bs:
|
||||
with patch("integrations.paperclip.task_runner.settings") as ts, patch(
|
||||
"integrations.paperclip.bridge.settings"
|
||||
) as bs:
|
||||
for s in (ts, bs):
|
||||
s.paperclip_enabled = True
|
||||
s.paperclip_agent_id = TIMMY_AGENT_ID
|
||||
@@ -179,7 +175,11 @@ class TestOrchestratorWiring:
|
||||
"""Verify the orchestrator parameter actually connects to the pipe."""
|
||||
|
||||
async def test_orchestrator_execute_task_is_called(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""When orchestrator is wired, process_task calls execute_task."""
|
||||
issue = _make_issue()
|
||||
@@ -193,7 +193,11 @@ class TestOrchestratorWiring:
|
||||
assert call["context"]["title"] == "Muse about task automation"
|
||||
|
||||
async def test_orchestrator_receives_full_context(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""Context dict passed to execute_task includes all issue metadata."""
|
||||
issue = _make_issue(
|
||||
@@ -213,7 +217,11 @@ class TestOrchestratorWiring:
|
||||
assert ctx["labels"] == ["automation", "meta"]
|
||||
|
||||
async def test_orchestrator_dict_result_unwrapped(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""When execute_task returns a dict, the 'result' key is extracted."""
|
||||
issue = _make_issue()
|
||||
@@ -226,7 +234,10 @@ class TestOrchestratorWiring:
|
||||
assert "issue-1" in result
|
||||
|
||||
async def test_orchestrator_string_result_passthrough(
|
||||
self, mock_client, bridge, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
settings_patch,
|
||||
):
|
||||
"""When execute_task returns a plain string, it passes through."""
|
||||
|
||||
@@ -240,7 +251,11 @@ class TestOrchestratorWiring:
|
||||
assert result == "Plain string result for issue-1"
|
||||
|
||||
async def test_process_fn_overrides_orchestrator(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""Explicit process_fn takes priority over orchestrator."""
|
||||
|
||||
@@ -248,7 +263,9 @@ class TestOrchestratorWiring:
|
||||
return "override wins"
|
||||
|
||||
runner = TaskRunner(
|
||||
bridge=bridge, orchestrator=stub_orchestrator, process_fn=override,
|
||||
bridge=bridge,
|
||||
orchestrator=stub_orchestrator,
|
||||
process_fn=override,
|
||||
)
|
||||
result = await runner.process_task(_make_issue())
|
||||
|
||||
@@ -314,7 +331,11 @@ class TestProcessTask:
|
||||
"""Verify checkout + orchestrator invocation + result flow."""
|
||||
|
||||
async def test_checkout_before_orchestrator(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""Issue must be checked out before orchestrator runs."""
|
||||
issue = _make_issue()
|
||||
@@ -323,9 +344,7 @@ class TestProcessTask:
|
||||
original_execute = stub_orchestrator.execute_task
|
||||
|
||||
async def tracking_execute(task_id, desc, ctx):
|
||||
checkout_happened["before_execute"] = (
|
||||
mock_client.checkout_issue.await_count > 0
|
||||
)
|
||||
checkout_happened["before_execute"] = mock_client.checkout_issue.await_count > 0
|
||||
return await original_execute(task_id, desc, ctx)
|
||||
|
||||
stub_orchestrator.execute_task = tracking_execute
|
||||
@@ -336,7 +355,11 @@ class TestProcessTask:
|
||||
assert checkout_happened["before_execute"], "checkout must happen before execute_task"
|
||||
|
||||
async def test_orchestrator_output_flows_to_result(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""The string returned by process_task comes from the orchestrator."""
|
||||
issue = _make_issue(id="flow-1", title="Flow verification", priority="high")
|
||||
@@ -350,7 +373,10 @@ class TestProcessTask:
|
||||
assert "high" in result
|
||||
|
||||
async def test_default_fallback_without_orchestrator(
|
||||
self, mock_client, bridge, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
settings_patch,
|
||||
):
|
||||
"""Without orchestrator or process_fn, a default message is returned."""
|
||||
issue = _make_issue(title="Fallback test")
|
||||
@@ -368,7 +394,11 @@ class TestCompleteTask:
|
||||
"""Verify orchestrator output flows into the completion comment."""
|
||||
|
||||
async def test_orchestrator_output_in_comment(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""The comment posted to Paperclip contains the orchestrator's output."""
|
||||
issue = _make_issue(id="cmt-1", title="Comment pipe test")
|
||||
@@ -386,7 +416,10 @@ class TestCompleteTask:
|
||||
assert "Comment pipe test" in comment_content
|
||||
|
||||
async def test_marks_issue_done(
|
||||
self, mock_client, bridge, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
settings_patch,
|
||||
):
|
||||
issue = _make_issue()
|
||||
mock_client.update_issue.return_value = _make_done()
|
||||
@@ -399,7 +432,10 @@ class TestCompleteTask:
|
||||
assert update_req.status == "done"
|
||||
|
||||
async def test_returns_false_on_close_failure(
|
||||
self, mock_client, bridge, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
settings_patch,
|
||||
):
|
||||
mock_client.update_issue.return_value = None
|
||||
runner = TaskRunner(bridge=bridge)
|
||||
@@ -415,7 +451,11 @@ class TestCreateFollowUp:
|
||||
"""Verify orchestrator output flows into the follow-up description."""
|
||||
|
||||
async def test_follow_up_contains_orchestrator_output(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""The follow-up description includes the orchestrator's result text."""
|
||||
issue = _make_issue(id="fu-1", title="Follow-up pipe test")
|
||||
@@ -431,7 +471,10 @@ class TestCreateFollowUp:
|
||||
assert "fu-1" in create_req.description
|
||||
|
||||
async def test_follow_up_assigned_to_self(
|
||||
self, mock_client, bridge, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
settings_patch,
|
||||
):
|
||||
mock_client.create_issue.return_value = _make_follow_up()
|
||||
runner = TaskRunner(bridge=bridge)
|
||||
@@ -441,7 +484,10 @@ class TestCreateFollowUp:
|
||||
assert req.assignee_id == TIMMY_AGENT_ID
|
||||
|
||||
async def test_follow_up_preserves_priority(
|
||||
self, mock_client, bridge, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
settings_patch,
|
||||
):
|
||||
mock_client.create_issue.return_value = _make_follow_up()
|
||||
runner = TaskRunner(bridge=bridge)
|
||||
@@ -475,7 +521,11 @@ class TestGreenPathWithOrchestrator:
|
||||
"""
|
||||
|
||||
async def test_full_cycle_orchestrator_output_everywhere(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""Orchestrator result appears in comment, follow-up, and summary."""
|
||||
original = _make_issue(
|
||||
@@ -527,7 +577,11 @@ class TestGreenPathWithOrchestrator:
|
||||
assert mock_client.create_issue.await_count == 1
|
||||
|
||||
async def test_no_tasks_returns_none(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
mock_client.list_issues.return_value = []
|
||||
runner = TaskRunner(bridge=bridge, orchestrator=stub_orchestrator)
|
||||
@@ -535,7 +589,11 @@ class TestGreenPathWithOrchestrator:
|
||||
assert len(stub_orchestrator.calls) == 0
|
||||
|
||||
async def test_close_failure_still_creates_follow_up(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
mock_client.list_issues.return_value = [_make_issue()]
|
||||
mock_client.update_issue.return_value = None # close fails
|
||||
@@ -558,7 +616,11 @@ class TestExternalTaskInjection:
|
||||
"""External system creates a task → Timmy's orchestrator processes it."""
|
||||
|
||||
async def test_external_task_flows_through_orchestrator(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
external = _make_issue(
|
||||
id="ext-1",
|
||||
@@ -581,7 +643,11 @@ class TestExternalTaskInjection:
|
||||
assert "Review quarterly metrics" in summary["result"]
|
||||
|
||||
async def test_skips_tasks_for_other_agents(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
other = _make_issue(id="other-1", assignee_id="agent-codex")
|
||||
mine = _make_issue(id="mine-1", title="My task")
|
||||
@@ -605,17 +671,26 @@ class TestRecursiveChain:
|
||||
"""Multi-cycle chains where each follow-up becomes the next task."""
|
||||
|
||||
async def test_two_cycle_chain(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
task_a = _make_issue(id="A", title="Initial musing")
|
||||
fu_b = PaperclipIssue(
|
||||
id="B", title="Follow-up: Initial musing",
|
||||
description="Continue", status="open",
|
||||
assignee_id=TIMMY_AGENT_ID, priority="normal",
|
||||
id="B",
|
||||
title="Follow-up: Initial musing",
|
||||
description="Continue",
|
||||
status="open",
|
||||
assignee_id=TIMMY_AGENT_ID,
|
||||
priority="normal",
|
||||
)
|
||||
fu_c = PaperclipIssue(
|
||||
id="C", title="Follow-up: Follow-up",
|
||||
status="open", assignee_id=TIMMY_AGENT_ID,
|
||||
id="C",
|
||||
title="Follow-up: Follow-up",
|
||||
status="open",
|
||||
assignee_id=TIMMY_AGENT_ID,
|
||||
)
|
||||
|
||||
# Cycle 1
|
||||
@@ -643,14 +718,20 @@ class TestRecursiveChain:
|
||||
assert stub_orchestrator.calls[1]["task_id"] == "B"
|
||||
|
||||
async def test_three_cycle_chain_all_through_orchestrator(
|
||||
self, mock_client, bridge, stub_orchestrator, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
stub_orchestrator,
|
||||
settings_patch,
|
||||
):
|
||||
"""Three cycles — every task goes through the orchestrator pipe."""
|
||||
tasks = [_make_issue(id=f"c-{i}", title=f"Chain {i}") for i in range(3)]
|
||||
follow_ups = [
|
||||
PaperclipIssue(
|
||||
id=f"c-{i + 1}", title=f"Follow-up: Chain {i}",
|
||||
status="open", assignee_id=TIMMY_AGENT_ID,
|
||||
id=f"c-{i + 1}",
|
||||
title=f"Follow-up: Chain {i}",
|
||||
status="open",
|
||||
assignee_id=TIMMY_AGENT_ID,
|
||||
)
|
||||
for i in range(3)
|
||||
]
|
||||
@@ -676,7 +757,6 @@ class TestRecursiveChain:
|
||||
|
||||
|
||||
class TestLifecycle:
|
||||
|
||||
async def test_stop_halts_loop(self, mock_client, bridge, settings_patch):
|
||||
runner = TaskRunner(bridge=bridge)
|
||||
runner._running = True
|
||||
@@ -684,7 +764,10 @@ class TestLifecycle:
|
||||
assert runner._running is False
|
||||
|
||||
async def test_start_disabled_when_interval_zero(
|
||||
self, mock_client, bridge, settings_patch,
|
||||
self,
|
||||
mock_client,
|
||||
bridge,
|
||||
settings_patch,
|
||||
):
|
||||
settings_patch.paperclip_poll_interval = 0
|
||||
runner = TaskRunner(bridge=bridge)
|
||||
@@ -701,6 +784,7 @@ def _ollama_reachable() -> tuple[bool, list[str]]:
|
||||
"""Return (reachable, model_names)."""
|
||||
try:
|
||||
import httpx
|
||||
|
||||
resp = httpx.get("http://localhost:11434/api/tags", timeout=3)
|
||||
resp.raise_for_status()
|
||||
names = [m["name"] for m in resp.json().get("models", [])]
|
||||
@@ -726,9 +810,7 @@ class LiveOllamaOrchestrator:
|
||||
self.model_name = model_name
|
||||
self.calls: list[dict] = []
|
||||
|
||||
async def execute_task(
|
||||
self, task_id: str, description: str, context: dict
|
||||
) -> str:
|
||||
async def execute_task(self, task_id: str, description: str, context: dict) -> str:
|
||||
import httpx as hx
|
||||
|
||||
self.calls.append({"task_id": task_id, "description": description})
|
||||
@@ -814,13 +896,18 @@ class TestLiveOllamaGreenPath:
|
||||
|
||||
task_a = _make_issue(id="live-A", title="Initial reflection")
|
||||
fu_b = PaperclipIssue(
|
||||
id="live-B", title="Follow-up: Initial reflection",
|
||||
description="Continue reflecting", status="open",
|
||||
assignee_id=TIMMY_AGENT_ID, priority="normal",
|
||||
id="live-B",
|
||||
title="Follow-up: Initial reflection",
|
||||
description="Continue reflecting",
|
||||
status="open",
|
||||
assignee_id=TIMMY_AGENT_ID,
|
||||
priority="normal",
|
||||
)
|
||||
fu_c = PaperclipIssue(
|
||||
id="live-C", title="Follow-up: Follow-up",
|
||||
status="open", assignee_id=TIMMY_AGENT_ID,
|
||||
id="live-C",
|
||||
title="Follow-up: Follow-up",
|
||||
status="open",
|
||||
assignee_id=TIMMY_AGENT_ID,
|
||||
)
|
||||
|
||||
live_orch = LiveOllamaOrchestrator(chosen)
|
||||
|
||||
Reference in New Issue
Block a user