diff --git a/.claude/commands/approve-merge.md b/.claude/commands/approve-merge.md index 5aaf08e7..b815fc28 100644 --- a/.claude/commands/approve-merge.md +++ b/.claude/commands/approve-merge.md @@ -116,7 +116,7 @@ If merge queue is detected (or forced via config): - Write a merge queue marker file so the dashboard can track the PR: ```bash mkdir -p .claude/agent-control - python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': '', 'branch_name': ''}))" > .claude/agent-control/merge-queue.json + python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': , 'branch_name': ''}))" > .claude/agent-control/merge-queue.json ``` - Proceed to queue polling (step 3b) diff --git a/.claude/commands/integrate-pr.md b/.claude/commands/integrate-pr.md index a8e2f5e2..54175949 100644 --- a/.claude/commands/integrate-pr.md +++ b/.claude/commands/integrate-pr.md @@ -171,7 +171,7 @@ If merge queue is detected: - If enqueued, write a merge queue marker file so the dashboard can track the PR: ```bash mkdir -p .claude/agent-control - python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': '', 'branch_name': ''}))" > .claude/agent-control/merge-queue.json + python3 -c "import json; print(json.dumps({'pr_number': , 'repo': '', 'issue_number': , 'branch_name': ''}))" > .claude/agent-control/merge-queue.json ``` - Then poll merge queue status via GraphQL every `merge_queue_poll_interval` seconds (default 30) - On MERGED: proceed to Phase 6. If `delete_branch = true`, delete remote branch via GitHub API diff --git a/.claude/rules/architecture.md b/.claude/rules/architecture.md index e1eaa230..96deb685 100644 --- a/.claude/rules/architecture.md +++ b/.claude/rules/architecture.md @@ -14,7 +14,7 @@ SOVA has four main components: - `core/state.py` -- 19-state TaskStatus StrEnum with transition validation - `core/context.py` -- ExecutionContext dataclass threading state through steps - `core/output.py` -- OutputWriter for per-run DB-backed output persistence, read_lines, retention cleanup -- `core/steps/` -- 29 BaseStep implementations with execute/validate_output/can_skip. Four pipeline variants: +- `core/steps/` -- 31 BaseStep implementations with execute/validate_output/can_skip. Four pipeline variants: - **Developer pipeline** (16 steps): sync -> assess -> create_worktree -> capture_baseline -> develop -> simplify -> self_review -> commit -> validate -> push -> create_pr -> wait_for_external_reviews -> address_external_findings -> monitor_ci -> extract_memory -> handoff_to_reviewer - **Address-review pipeline** (10 steps): ensure_worktree -> rebase -> address_review -> rearrange_commits -> validate -> push -> monitor_ci -> resolve_external_reviews -> extract_memory -> handoff_to_user - **Researcher pipeline** (4 steps): fetch_task -> research -> spec -> extract_memory diff --git a/Makefile b/Makefile index a9d1f004..33e9d8d5 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ test-bash: lint-bash ## Validate bash scripts (shellcheck + --help) done test-py: ## Run pytest suite (excludes runtime/stress/chaos) - $(PYTEST) tests/ -v -m "not runtime and not stress and not chaos" + $(PYTEST) tests/ -v -m "not runtime and not stress and not chaos" -n auto test-runtime: ## Run runtime, stress, and chaos tests (manual) $(PYTEST) tests/ -v -m "runtime or stress or chaos" --timeout=120 diff --git a/pyproject.toml b/pyproject.toml index 9db0205e..254dfecb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,6 +55,7 @@ dev = [ "pytest-asyncio>=0.24", "pytest-cov>=6.0", "pytest-timeout>=2.3", + "pytest-xdist>=3.0", "ruff>=0.4", "respx>=0.22", ] diff --git a/sova/dashboard/static/app.js b/sova/dashboard/static/app.js index e541b537..193e67ff 100644 --- a/sova/dashboard/static/app.js +++ b/sova/dashboard/static/app.js @@ -895,7 +895,33 @@ if (document.getElementById('activity-dot')) { initGlobalBatch(); /* ============================================================ - 12. ROLE COLORS + 12. PRIORITY ICONS + ============================================================ */ + +// Keep in sync with priority_map in _components.html +var _PRIORITY_ICON_MAP = { + critical: 'blocker', + high: 'high', + medium: 'medium', + low: 'low' +}; + +function priorityIconUrl(priority) { + var p = (priority || '').toString().toLowerCase().trim(); + var iconName = _PRIORITY_ICON_MAP[p] || 'undefined'; + return '/static/priority/' + iconName + '.svg'; +} + +function _extractPriority(labels) { + for (var i = 0; i < (labels || []).length; i++) { + var l = labels[i]; + if (l.indexOf('priority:') === 0) return l.replace('priority: ', '').replace('priority:', '').trim(); + } + return ''; +} + +/* ============================================================ + 13. ROLE COLORS ============================================================ */ function _roleHex(key) { diff --git a/sova/dashboard/static/priority/blocker.svg b/sova/dashboard/static/priority/blocker.svg new file mode 100644 index 00000000..069d2c26 --- /dev/null +++ b/sova/dashboard/static/priority/blocker.svg @@ -0,0 +1,5 @@ + + + + + diff --git a/sova/dashboard/static/priority/critical.svg b/sova/dashboard/static/priority/critical.svg new file mode 100644 index 00000000..2f5b0ca2 --- /dev/null +++ b/sova/dashboard/static/priority/critical.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/static/priority/high.svg b/sova/dashboard/static/priority/high.svg new file mode 100644 index 00000000..d1ba8598 --- /dev/null +++ b/sova/dashboard/static/priority/high.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/static/priority/low.svg b/sova/dashboard/static/priority/low.svg new file mode 100644 index 00000000..1c2fd2ce --- /dev/null +++ b/sova/dashboard/static/priority/low.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/static/priority/medium.svg b/sova/dashboard/static/priority/medium.svg new file mode 100644 index 00000000..c4a9c3d5 --- /dev/null +++ b/sova/dashboard/static/priority/medium.svg @@ -0,0 +1,3 @@ + + + diff --git a/sova/dashboard/static/priority/undefined.svg b/sova/dashboard/static/priority/undefined.svg new file mode 100644 index 00000000..626a6bd9 --- /dev/null +++ b/sova/dashboard/static/priority/undefined.svg @@ -0,0 +1,4 @@ + + + + diff --git a/sova/dashboard/templates/_components.html b/sova/dashboard/templates/_components.html index 5e7e1e67..4a4a0e1e 100644 --- a/sova/dashboard/templates/_components.html +++ b/sova/dashboard/templates/_components.html @@ -18,3 +18,17 @@

+{%- endmacro -%} diff --git a/sova/dashboard/templates/agents.html b/sova/dashboard/templates/agents.html index 4fdd627a..9c971154 100644 --- a/sova/dashboard/templates/agents.html +++ b/sova/dashboard/templates/agents.html @@ -482,14 +482,6 @@

Launch Agent

renderTaskCards(); } -function _extractPriority(labels) { - for (var i = 0; i < (labels || []).length; i++) { - var l = labels[i]; - if (l.indexOf('priority:') === 0) return l.replace('priority: ', '').replace('priority:', '').trim(); - } - return ''; -} - var _priorityOrder = { critical: 0, high: 1, medium: 2, low: 3 }; function _renderHandoffBtn(ha, issueNumber) { @@ -552,8 +544,7 @@

Launch Agent

var issueKey = item.issue_number || ('pr:' + item.pr_number); var isRunning = item.state === 'agent_running'; var p = _extractPriority(item.labels); - var pDots = { critical: 'bg-accent-red', high: 'bg-accent-yellow', medium: 'bg-gray-500', low: 'bg-gray-600' }; - var pDot = p ? '' : ''; + var priorityIcon = p ? '' + escapeHtml(p) + ' priority' : ''; var handoffActions = item.handoff_actions || []; var isSpecReview = item.state === 'spec_review'; @@ -694,7 +685,7 @@

Launch Agent

: ''; var mainRow = '
' + - (needsAttention ? attentionDot : pDot) + + (needsAttention ? attentionDot : priorityIcon) + issueRef + '' + escapeHtml(item.title) + '' + metaStrip + @@ -1543,14 +1534,10 @@

Launch Agent

} var taskListHtml = tasks.map(function(t, i) { - var priorityColors = { - critical: 'text-accent-red', high: 'text-accent-yellow', - medium: 'text-accent', low: 'text-gray-400' - }; - var pColor = priorityColors[t.priority] || 'text-gray-400'; + var iconUrl = priorityIconUrl(t.priority); return ''; }).join(''); diff --git a/sova/dashboard/templates/queue.html b/sova/dashboard/templates/queue.html index e666dd7c..b60b00bd 100644 --- a/sova/dashboard/templates/queue.html +++ b/sova/dashboard/templates/queue.html @@ -284,6 +284,9 @@

Priority Queue

var typeBadge = issueTypeBadge(item.issue_type); + var priority = _extractPriority(item.labels || []); + var priorityIcon = '' + escapeHtml(priority || 'undefined') + ' priority'; + var displayLabels = (item.labels || []).filter(function(l) { return l.indexOf('agent:') !== 0 && l.indexOf('priority:') !== 0 && l.indexOf('role:') !== 0; }); @@ -314,6 +317,7 @@

Priority Queue

'' + + priorityIcon + '' + item.priority_label + '' + '
' + '
' + diff --git a/sova/dashboard/templates/supervisor.html b/sova/dashboard/templates/supervisor.html index 7bc7164d..b4874507 100644 --- a/sova/dashboard/templates/supervisor.html +++ b/sova/dashboard/templates/supervisor.html @@ -1,4 +1,5 @@ {% extends "base.html" %} +{% from "_components.html" import priority_icon %} {% block title %}Supervisor - SOVA{% endblock %} {% block content %}
@@ -36,14 +37,12 @@

Dependency Graph Agent Handoff | - Critical - High - Med - Low + {{ priority_icon('critical', '12') }} Critical + {{ priority_icon('high', '12') }} High + {{ priority_icon('medium', '12') }} Med + {{ priority_icon('low', '12') }} Low

- @@ -202,17 +201,6 @@

Activity Stream

let _selectedNodeId = null; let _fitTransform = null; let _savedTransform = null; // user's current pan/zoom, preserved across re-renders -let _lastGraphData = null; - -function _lsGet(key) { - try { return localStorage.getItem(key); } catch { return null; } -} - -function _lsSet(key, val) { - try { localStorage.setItem(key, val); } catch { /* noop */ } -} - -let _hideDone = _lsGet('sova-graph-hide-done') === '1'; function _computeGraphKey(data) { // agent_elapsed_seconds is intentionally excluded: it changes every poll while an agent runs @@ -242,43 +230,24 @@

Activity Stream

return null; } -// Priority configuration: colors and SVG arrow paths (8x8 viewport, Jira-style) +// Priority configuration: colors for backward compatibility const PRIORITY_CONFIG = { - critical: { color: '#f38ba8', arrow: 'M4 0 L7 4 L5 4 L5 6 L3 6 L3 4 L1 4 Z M3 7 L5 7 L5 8 L3 8 Z' }, // up + exclamation - high: { color: '#fab387', arrow: 'M4 0 L7 4 L5 4 L5 8 L3 8 L3 4 L1 4 Z' }, // up - medium: { color: '#f9e2af', arrow: 'M8 4 L4 1 L4 3 L0 3 L0 5 L4 5 L4 7 Z' }, // right - low: { color: '#6c7086', arrow: 'M4 8 L1 4 L3 4 L3 0 L5 0 L5 4 L7 4 Z' } // down + critical: { color: '#f38ba8' }, + high: { color: '#fab387' }, + medium: { color: '#f9e2af' }, + low: { color: '#6c7086' } }; function priorityColor(priority) { return PRIORITY_CONFIG[priority]?.color || null; } -function priorityArrowPath(priority) { - return PRIORITY_CONFIG[priority]?.arrow || null; -} - const _PRIORITY_ORDER = { critical: 0, high: 1, medium: 2, low: 3 }; function zoomIn() { if (_svgSel && _zoomBehavior) _svgSel.transition().duration(300).call(_zoomBehavior.scaleBy, 1.35); } function zoomOut() { if (_svgSel && _zoomBehavior) _svgSel.transition().duration(300).call(_zoomBehavior.scaleBy, 0.74); } function zoomFit() { if (_svgSel && _zoomBehavior && _fitTransform) _svgSel.transition().duration(350).call(_zoomBehavior.transform, _fitTransform); } -function toggleHideDone() { - _hideDone = !_hideDone; - _lsSet('sova-graph-hide-done', _hideDone ? '1' : '0'); - _updateHideDoneBtn(); - if (_lastGraphData) { - if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); - renderGraph(_lastGraphData); - } -} - -function _updateHideDoneBtn() { - const btn = document.getElementById('hide-done-btn'); - if (btn) btn.textContent = _hideDone ? 'Show Done' : 'Hide Done'; -} - // -- Toggle / enable / disable -- let _toggleBusy = false; @@ -525,7 +494,6 @@

Setup Required

if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); errorEl.classList.add('hidden'); - _lastGraphData = data; renderGraph(data); } catch (e) { loading.classList.add('hidden'); @@ -731,42 +699,22 @@

Setup Required

} }); - // Determine completed groups and visibility - const TOMBSTONE_H = 28; - - function isGroupCompleted(m) { - const nodes = msMap.get(m); - return !nodes.length || nodes.every(n => n.state === 'done' || n.state === 'closed'); - } - - const visibleMs = _hideDone - ? sortedMs.filter(m => !isGroupCompleted(m)) - : [...sortedMs]; - - const collapsedGroups = new Set(); - visibleMs.forEach(m => { - if (isGroupCompleted(m) && _lsGet(`sova-graph-expanded:${m}`) !== '1') { - collapsedGroups.add(m); - } - }); - // Arrange groups in a 2-column grid (even-index groups = left column, odd = right) - const col0MaxW = Math.max(...visibleMs.filter((_, i) => i % 2 === 0).map(m => groupLayouts.get(m).boxW), NW + GP * 2); + const col0MaxW = Math.max(...sortedMs.filter((_, i) => i % 2 === 0).map(m => groupLayouts.get(m).boxW), NW + GP * 2); const col0X = MARGIN; const col1X = col0X + col0MaxW + GGX; const groupPos = new Map(); let curY0 = MARGIN, curY1 = MARGIN; - visibleMs.forEach((m, i) => { - const h = collapsedGroups.has(m) ? TOMBSTONE_H : groupLayouts.get(m).boxH; - if (i % 2 === 0) { groupPos.set(m, { x: col0X, y: curY0 }); curY0 += h + GGY; } - else { groupPos.set(m, { x: col1X, y: curY1 }); curY1 += h + GGY; } + sortedMs.forEach((m, i) => { + const { boxH } = groupLayouts.get(m); + if (i % 2 === 0) { groupPos.set(m, { x: col0X, y: curY0 }); curY0 += boxH + GGY; } + else { groupPos.set(m, { x: col1X, y: curY1 }); curY1 += boxH + GGY; } }); - // Build global nodePos from group origin + local position (skip collapsed groups) + // Build global nodePos from group origin + local position const nodePos = {}; - visibleMs.forEach(m => { - if (collapsedGroups.has(m)) return; + sortedMs.forEach(m => { const { x: gx, y: gy } = groupPos.get(m); groupLayouts.get(m).nodeLPos.forEach((lp, id) => { const nx = gx + lp.lx, ny = gy + lp.ly; @@ -776,11 +724,6 @@

Setup Required

const allX = Object.values(nodePos).map(p => p.x + NW); const allY = Object.values(nodePos).map(p => p.y + NH); - collapsedGroups.forEach(m => { - const { x, y } = groupPos.get(m); - allX.push(x + groupLayouts.get(m).boxW); - allY.push(y + TOMBSTONE_H); - }); const contentW = (allX.length ? Math.max(...allX) + GP : 600) + MARGIN; const contentH = (allY.length ? Math.max(...allY) + GP : 400) + MARGIN; @@ -878,50 +821,11 @@

Setup Required

} // --- draw group boxes (behind edges and nodes) --- - visibleMs.forEach(m => { + sortedMs.forEach(m => { const { x, y } = groupPos.get(m); const { boxW, boxH, nodeLPos, subgroupMeta } = groupLayouts.get(m); const { bg, border, head } = groupPal.get(m); const memberIds = JSON.stringify([...nodeLPos.keys()]); - const completed = isGroupCompleted(m); - - if (collapsedGroups.has(m)) { - const grp = mainG.append('g') - .attr('class', 'group-box group-tombstone') - .attr('data-members', memberIds) - .style('cursor', 'pointer'); - - grp.append('rect') - .attr('x', x).attr('y', y).attr('width', boxW).attr('height', TOMBSTONE_H) - .attr('rx', 6) - .attr('fill', bg).attr('fill-opacity', 0.6) - .attr('stroke', border).attr('stroke-width', 1).attr('stroke-opacity', 0.4); - - const label = m.replace(/^Phase \d+:\s*/i, ''); - const tDisplay = label.length > 40 ? label.slice(0, 39) + '…' : label; - grp.append('text') - .attr('x', x + 10).attr('y', y + 18) - .attr('fill', head).attr('font-size', '9px').attr('font-weight', '600') - .attr('font-family', 'system-ui, sans-serif').attr('letter-spacing', '0.04em') - .attr('opacity', 0.7) - .text(tDisplay.toUpperCase()); - - const nodeCount = msMap.get(m).length; - grp.append('text') - .attr('x', x + boxW - 10).attr('y', y + 18) - .attr('text-anchor', 'end') - .attr('fill', '#a6e3a1').attr('font-size', '9px').attr('font-weight', '500') - .attr('font-family', 'system-ui, sans-serif') - .attr('opacity', 0.6) - .text(`${nodeCount} done`); - - grp.on('click', () => { - _lsSet(`sova-graph-expanded:${m}`, '1'); - if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); - renderGraph(_lastGraphData); - }); - return; - } const grp = mainG.append('g').attr('class', 'group-box').attr('data-members', memberIds); @@ -947,23 +851,6 @@

Setup Required

.attr('font-family', 'system-ui, sans-serif').attr('letter-spacing', '0.04em') .text(display.toUpperCase()); - if (completed) { - grp.append('text') - .attr('x', x + boxW - GP).attr('y', y + GH - 8) - .attr('text-anchor', 'end') - .attr('fill', head).attr('font-size', '8.5px').attr('font-weight', '500') - .attr('font-family', 'system-ui, sans-serif') - .attr('opacity', 0.5) - .style('cursor', 'pointer') - .text('collapse') - .on('click', (event) => { - event.stopPropagation(); - _lsSet(`sova-graph-expanded:${m}`, '0'); - if (_svgSel) _savedTransform = d3.zoomTransform(_svgSel.node()); - renderGraph(_lastGraphData); - }); - } - // Subgroup dividers and labels (for large groups only) subgroupMeta.forEach(sg => { // Divider line above this subgroup (except first) @@ -1023,28 +910,31 @@

Setup Required

bgRect.attr('stroke-dasharray', '4,3'); } - // Priority left-border stripe (3px, clipped to node corner radius) + arrow icon + // Priority left-border stripe (3px, clipped to node corner radius) + icon const prioColor = priorityColor(n.priority); if (prioColor) { g.append('rect') .attr('x', pos.x).attr('y', pos.y).attr('width', 3).attr('height', NH).attr('rx', 2) .attr('fill', prioColor).attr('opacity', 0.9); + } - // Priority arrow icon at bottom-left (shape + color for accessibility) - const ARROW_OFFSET_NORMAL = 5; - const ARROW_OFFSET_EPIC = 4 + EPIC_BADGE_WIDTH + 6; // epicBadgeX + width + margin - const ARROW_BOTTOM_MARGIN = 12; - const arrowX = isEpic ? pos.x + ARROW_OFFSET_EPIC : pos.x + ARROW_OFFSET_NORMAL; - const arrowY = pos.y + NH - ARROW_BOTTOM_MARGIN; - const arrowPath = priorityArrowPath(n.priority); - if (arrowPath) { - const arrow = g.append('path') - .attr('d', arrowPath) - .attr('transform', `translate(${arrowX}, ${arrowY})`) - .attr('fill', prioColor) - .attr('opacity', 0.8); - arrow.append('title').text(`Priority: ${n.priority}`); - } + // Priority icon at bottom-left (always render, even without color stripe) + { + const ICON_SIZE = 12; + const ICON_OFFSET_NORMAL = 5; + const ICON_OFFSET_EPIC = 4 + EPIC_BADGE_WIDTH + 6; // epicBadgeX + width + margin + const ICON_BOTTOM_MARGIN = 12; + const iconX = isEpic ? pos.x + ICON_OFFSET_EPIC : pos.x + ICON_OFFSET_NORMAL; + const iconY = pos.y + NH - ICON_BOTTOM_MARGIN; + const iconUrl = priorityIconUrl(n.priority); + const icon = g.append('image') + .attr('href', iconUrl) + .attr('x', iconX) + .attr('y', iconY) + .attr('width', ICON_SIZE) + .attr('height', ICON_SIZE) + .attr('opacity', 0.9); + icon.append('title').text(`Priority: ${n.priority || 'undefined'}`); } // Epic badge at bottom-left (similar to PR badge) @@ -1730,7 +1620,6 @@

Setup Required

} // -- Init -- -_updateHideDoneBtn(); loadStatus(); loadQuota(); loadCIBudget(); loadGraph(); loadDecisions(); loadPlan(); loadQueue(); loadSupervisorPersona(); // Poll every 30s as fallback; loadGraph re-renders only when data changes diff --git a/sova/supervisor/ci_budget.py b/sova/supervisor/ci_budget.py index 977963d0..e4e83130 100644 --- a/sova/supervisor/ci_budget.py +++ b/sova/supervisor/ci_budget.py @@ -95,8 +95,7 @@ async def _fetch(self, repo: str, github_user: str) -> CIBudget: try: data = json.loads(result.stdout) except (json.JSONDecodeError, TypeError): - log.warning("ci_budget.bad_json", repo=repo) - return _zero_budget() + data = None if not isinstance(data, dict): log.warning("ci_budget.bad_json", repo=repo) diff --git a/sova/supervisor/watchdog.py b/sova/supervisor/watchdog.py index 4ecabfc9..c6f7d7d6 100644 --- a/sova/supervisor/watchdog.py +++ b/sova/supervisor/watchdog.py @@ -5,8 +5,8 @@ (pipeline not adopted, no output, step timeout, zombie process), and takes corrective action (warn via feed event, kill via stop_agent). -Killed agents flow through the existing _wait_and_finalize path. -The watchdog never independently retries. +Killed agents flow through the existing _wait_and_finalize -> _schedule_retry() +path for auto-retry. The watchdog never independently retries. """ from __future__ import annotations diff --git a/tests/test_ci_budget.py b/tests/test_ci_budget.py index fd315adc..203d8868 100644 --- a/tests/test_ci_budget.py +++ b/tests/test_ci_budget.py @@ -575,6 +575,20 @@ async def test_no_plan_object_defaults_to_free(self) -> None: assert minutes == 2000 + @pytest.mark.asyncio + async def test_resolve_gh_env_exception_continues(self) -> None: + tracker = CIBudgetTracker() + mock_result = MagicMock() + mock_result.success = True + mock_result.stdout = '{"total_minutes_used": 200, "included_minutes": 3000}' + + with patch("sova.utils.shell.run", new_callable=AsyncMock, return_value=mock_result): + with patch("sova.utils.gh.resolve_gh_env", new_callable=AsyncMock, side_effect=RuntimeError("auth failed")): + budget = await tracker.get_budget("owner/repo", "user") + + assert budget.total == 3000 + assert budget.used == 200 + class TestCIBudgetTrackerFactory: def test_same_identity_returns_same_instance(self) -> None: diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 381f7850..399edbab 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -12,6 +12,7 @@ from httpx import ASGITransport, AsyncClient from sqlalchemy.ext.asyncio import AsyncSession +from sova.dashboard.app import create_app from sova.db.models import CostRecord, Memory, StepExecution, TaskRun from sova.db.session import close_db, get_session, init_db @@ -150,7 +151,6 @@ async def seed_data(session: AsyncSession): @pytest.fixture async def client(): - from sova.dashboard.app import create_app app = create_app(multi_project=False) transport = ASGITransport(app=app) @@ -3735,8 +3735,6 @@ async def multi_client(self, tmp_path): registry.register_project(p1, slug="alpha") registry.register_project(p2, slug="beta") - from sova.dashboard.app import create_app - app = create_app(multi_project=True) transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: @@ -13859,8 +13857,6 @@ def test_websocket_connect_and_receive_status_update(self) -> None: """WebSocket endpoint accepts connection and sends status_update with runs.""" from starlette.testclient import TestClient - from sova.dashboard.app import create_app - app = create_app(multi_project=False) client = TestClient(app) with client.websocket_connect("/api/ws/agents/status") as ws: @@ -13872,7 +13868,6 @@ def test_websocket_client_disconnect(self) -> None: """Verify connection is removed from manager after client disconnects.""" from starlette.testclient import TestClient - from sova.dashboard.app import create_app from sova.dashboard.routers.agents import _ws_manager app = create_app(multi_project=False) @@ -13887,8 +13882,6 @@ def test_websocket_sequential_clients(self) -> None: """Sequential clients can each connect and receive updates.""" from starlette.testclient import TestClient - from sova.dashboard.app import create_app - app = create_app(multi_project=False) # Use separate TestClient instances to avoid threading deadlock # when nesting websocket_connect context managers on the same client. @@ -13907,8 +13900,6 @@ def test_websocket_error_handling(self) -> None: from starlette.testclient import TestClient - from sova.dashboard.app import create_app - app = create_app(multi_project=False) client = TestClient(app) with ( @@ -14013,8 +14004,6 @@ async def install_client(self, tmp_path, monkeypatch): lambda: project_dir, ) - from sova.dashboard.app import create_app - app = create_app(project_dir=project_dir) transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: @@ -14776,7 +14765,6 @@ async def test_resume_clears_handoff(self) -> None: async def test_resume_endpoint_404(self) -> None: """The router endpoint returns 404 for missing run.""" - from sova.dashboard.app import create_app app = create_app() transport = ASGITransport(app=app) @@ -14786,7 +14774,6 @@ async def test_resume_endpoint_404(self) -> None: async def test_resume_endpoint_409(self) -> None: """The router endpoint returns 409 for wrong status.""" - from sova.dashboard.app import create_app async with await get_session() as session: async with session.begin(): @@ -14873,8 +14860,6 @@ async def test_resume_endpoint_500_on_spawn_error(self) -> None: """The router returns 500 for generic spawn errors (not 404/409).""" from unittest.mock import AsyncMock, patch - from sova.dashboard.app import create_app - async with await get_session() as session: async with session.begin(): run = TaskRun( diff --git a/tests/test_priority_icons.py b/tests/test_priority_icons.py new file mode 100644 index 00000000..e67b3312 --- /dev/null +++ b/tests/test_priority_icons.py @@ -0,0 +1,175 @@ +"""Tests for priority icon static files and rendering.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from httpx import ASGITransport, AsyncClient + +from sova.dashboard.app import create_app +from sova.db.session import close_db, init_db + +_ICON_NAMES = ["blocker", "critical", "high", "medium", "low", "undefined"] + + +@pytest.fixture(autouse=True) +async def setup_db(monkeypatch): + """Initialize an in-memory DB for icon tests.""" + monkeypatch.setenv("SOVA_DATABASE_URL", "sqlite+aiosqlite://") + await init_db(run_migrations=False) + yield + await close_db() + + +@pytest.fixture +async def client() -> AsyncClient: + """Create test client.""" + app = create_app(project_dir=None) + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as ac: + yield ac + + +class TestPriorityIcons: + """Test priority icon static files are served correctly.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize("icon", _ICON_NAMES) + async def test_icon_is_served(self, client: AsyncClient, icon: str) -> None: + resp = await client.get(f"/static/priority/{icon}.svg") + assert resp.status_code == 200 + assert resp.headers["content-type"] == "image/svg+xml" + assert b" None: + resp = await client.get("/static/priority/blocker.svg") + assert b"f38ba8" in resp.content + + @pytest.mark.asyncio + async def test_undefined_icon_has_gray_color(self, client: AsyncClient) -> None: + resp = await client.get("/static/priority/undefined.svg") + assert b"6c7086" in resp.content + + +class TestPriorityIconFiles: + """Test priority icon files exist and have correct structure.""" + + def test_all_icon_files_exist(self) -> None: + icon_dir = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "priority" + assert icon_dir.exists() + + for icon in _ICON_NAMES: + icon_path = icon_dir / f"{icon}.svg" + assert icon_path.exists(), f"Missing priority icon: {icon}.svg" + assert icon_path.stat().st_size > 0, f"Empty priority icon: {icon}.svg" + + @pytest.mark.parametrize("icon", ["blocker", "undefined"]) + def test_icon_structure(self, icon: str) -> None: + icon_path = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "priority" / f"{icon}.svg" + content = icon_path.read_text() + + assert " None: + components_path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "_components.html" + content = components_path.read_text() + + assert "macro priority_icon" in content + assert "/static/priority/" in content + assert "blocker" in content + + @pytest.mark.parametrize("template", ["supervisor.html", "agents.html", "queue.html"]) + def test_template_uses_shared_priorityIconUrl(self, template: str) -> None: + """Verify templates call priorityIconUrl (defined in app.js) without redefining it.""" + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / template + content = path.read_text() + assert "priorityIconUrl(" in content + assert "function priorityIconUrl" not in content + + def test_shared_helpers_in_app_js(self) -> None: + """Verify priorityIconUrl and _extractPriority are defined in app.js.""" + app_js = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "app.js" + content = app_js.read_text() + assert "function priorityIconUrl(" in content + assert "function _extractPriority(" in content + + +class TestPriorityIconMacroRendering: + """Test priority_icon Jinja2 macro renders correct HTML for all inputs.""" + + @pytest.fixture + def jinja_env(self): + from jinja2 import Environment, FileSystemLoader + + templates_dir = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" + return Environment(loader=FileSystemLoader(str(templates_dir))) + + @pytest.mark.parametrize( + "priority,expected_icon,expected_alt", + [ + (None, "undefined.svg", "Undefined"), + ("", "undefined.svg", "Undefined"), + ("critical", "blocker.svg", "Critical"), + ("high", "high.svg", "High"), + ("medium", "medium.svg", "Medium"), + ("low", "low.svg", "Low"), + ("unknown_value", "undefined.svg", "Unknown_value"), + ], + ) + def test_macro_renders_correct_icon(self, jinja_env, priority, expected_icon, expected_alt) -> None: + template = jinja_env.from_string('{%- from "_components.html" import priority_icon -%}{{ priority_icon(p) }}') + html = template.render(p=priority) + assert expected_icon in html + assert f'alt="{expected_alt} priority"' in html + + +class TestUndefinedPriorityRendering: + """Verify undefined.svg is rendered when priority is missing in JS templates.""" + + def test_supervisor_graph_renders_icon_without_priority(self) -> None: + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "supervisor.html" + content = path.read_text() + idx = content.find("Priority icon at bottom-left") + assert idx != -1, "Priority icon comment not found in supervisor.html" + block = content[idx : idx + 400] + assert "if (n.priority)" not in block, "Supervisor graph still guards icon rendering behind if(n.priority)" + + def test_queue_always_renders_priority_icon(self) -> None: + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "queue.html" + content = path.read_text() + assert "priority ? ' None: + path = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "agents.html" + content = path.read_text() + assert "priorityIconUrl(t.priority)" in content, "Agents template must call priorityIconUrl for planner tasks" + assert "t.priority ? ' None: + import re + + app_js = Path(__file__).parent.parent / "sova" / "dashboard" / "static" / "app.js" + js_content = app_js.read_text() + + components = Path(__file__).parent.parent / "sova" / "dashboard" / "templates" / "_components.html" + jinja_content = components.read_text() + + js_start = js_content.find("_PRIORITY_ICON_MAP") + js_map = {} + for m in re.finditer(r"(\w+):\s*'(\w+)'", js_content[js_start : js_start + 200]): + js_map[m.group(1)] = m.group(2) + + jinja_start = jinja_content.find("priority_map") + jinja_map = {} + for m in re.finditer(r"'(\w+)':\s*'(\w+)'", jinja_content[jinja_start : jinja_start + 200]): + jinja_map[m.group(1)] = m.group(2) + + assert js_map == jinja_map, f"JS map {js_map} != Jinja2 map {jinja_map}" diff --git a/tests/test_roles.py b/tests/test_roles.py index acdb3bbd..204d3a65 100644 --- a/tests/test_roles.py +++ b/tests/test_roles.py @@ -6,7 +6,7 @@ import os from decimal import Decimal from pathlib import Path -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock import pytest @@ -751,7 +751,7 @@ async def test_execute_reviews_pr(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -782,7 +782,7 @@ async def test_execute_discovers_pr_when_not_provided(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -816,7 +816,7 @@ async def test_reviewer_discovers_branch_when_pr_number_preset(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -852,7 +852,7 @@ async def test_reviewer_propagates_branch_name_in_handoff(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): await role.execute(ctx) @@ -907,7 +907,7 @@ async def test_execute_branch_discovery_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -934,7 +934,7 @@ async def test_execute_extract_memory_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch( "sova.knowledge.extraction.extract_memories", new_callable=AsyncMock, @@ -971,7 +971,7 @@ async def test_execute_handoff_db_failure_non_fatal(self) -> None: new_callable=AsyncMock, side_effect=RuntimeError("DB write failed"), ), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1029,7 +1029,7 @@ async def test_execute_review_rationale_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch( "sova.core.steps._spec_helpers.append_spec_section", side_effect=OSError("spec write failed"), @@ -1083,7 +1083,7 @@ async def test_execute_posts_inline_review_comments(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1119,7 +1119,7 @@ async def test_execute_falls_back_to_comment_on_review_failure(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1170,7 +1170,7 @@ async def fail_with_inline_succeed_without(pr_number, body, event, comments): patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1216,7 +1216,7 @@ async def test_reviewer_falls_back_to_comment_when_body_only_also_fails(self) -> patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): result = await role.execute(ctx) @@ -1281,7 +1281,7 @@ async def _capture_invoke(prompt, **kwargs): patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=_capture_invoke), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -1324,7 +1324,7 @@ async def _capture_invoke(prompt, **kwargs): patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=_capture_invoke), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -2998,7 +2998,7 @@ async def test_successful_review_with_findings(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["foo.py", "bar.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3027,7 +3027,7 @@ async def test_review_no_findings(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3052,7 +3052,7 @@ async def test_llm_failure_graceful_fallback(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=RuntimeError("LLM unavailable")), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3095,7 +3095,7 @@ async def test_large_diff_chunking(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py", "b.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, side_effect=side) as mock_invoke, patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3124,7 +3124,7 @@ async def test_handoff_address_review_when_actionable(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3158,7 +3158,7 @@ async def test_handoff_all_findings_actionable(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3193,7 +3193,7 @@ async def test_handoff_auto_execute_disabled_by_config(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3221,7 +3221,7 @@ async def test_handoff_approve_when_zero_findings(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["x.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): role = ReviewerRole() await role.execute(ctx) @@ -3294,7 +3294,7 @@ async def test_clears_current_step_sentinel(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): role = ReviewerRole() result = await role.execute(ctx) @@ -3334,7 +3334,7 @@ async def test_clear_current_step_db_failure_non_fatal(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch("sova.roles.reviewer.get_session", side_effect=OSError("DB connection refused")), ): role = ReviewerRole() @@ -3372,7 +3372,7 @@ async def test_no_clear_without_task_run_id(self) -> None: patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), patch("sova.roles.reviewer.read_handoff_file", return_value=None), patch("sova.roles.reviewer.get_session", new_callable=AsyncMock), patch.object(ReviewerRole, "_clear_current_step", mock_clear_step), @@ -3924,7 +3924,7 @@ async def test_write_verdict_label_skipped_when_no_issue(self) -> None: async def test_write_handoff_db_exception_non_fatal(self) -> None: """_write_handoff handles DB write failure gracefully (lines 733-734).""" - from unittest.mock import patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.roles.reviewer import ReviewerRole, ReviewResult @@ -3935,7 +3935,7 @@ async def test_write_handoff_db_exception_non_fatal(self) -> None: with ( patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock, side_effect=RuntimeError("DB down")), - patch("sova.roles.reviewer.write_handoff_file"), + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock), ): # Should not raise await role._write_handoff(ctx, review) @@ -3962,7 +3962,7 @@ async def test_execute_total_posting_failure_writes_post_failed_handoff(self) -> """ import json from decimal import Decimal - from unittest.mock import AsyncMock, patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.llm.models import LLMResult from sova.roles.reviewer import ReviewerRole @@ -3983,7 +3983,7 @@ async def test_execute_total_posting_failure_writes_post_failed_handoff(self) -> patch("sova.roles.reviewer.get_pr_files", new_callable=AsyncMock, return_value=["a.py"]), patch("sova.roles.reviewer.invoke", new_callable=AsyncMock, return_value=llm_result), patch("sova.roles.reviewer.write_handoff", new_callable=AsyncMock) as mock_db_handoff, - patch("sova.roles.reviewer.write_handoff_file") as mock_file_handoff, + patch("sova.roles.reviewer.write_handoff_file", new_callable=MagicMock) as mock_file_handoff, ): result = await role.execute(ctx) @@ -4578,7 +4578,7 @@ def test_read_vision_returns_empty_when_missing(self, tmp_path: Path) -> None: assert result == "" async def test_write_handoff_file_exception_non_fatal(self) -> None: - from unittest.mock import patch + from unittest.mock import MagicMock, patch from sova.roles.planner import PlannedTask, PlannerRole @@ -4594,12 +4594,12 @@ async def test_write_handoff_file_exception_non_fatal(self) -> None: rationale="r", ), ] - with patch("sova.roles.planner.write_handoff_file", side_effect=OSError("disk full")): + with patch("sova.roles.planner.write_handoff_file", new_callable=MagicMock, side_effect=OSError("disk full")): # Should not raise await role._write_handoff(ctx, tasks) async def test_write_handoff_db_path(self) -> None: - from unittest.mock import patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.roles.planner import PlannedTask, PlannerRole @@ -4616,7 +4616,7 @@ async def test_write_handoff_db_path(self) -> None: rationale="r", ), ] - mock_write_file = AsyncMock() + mock_write_file = MagicMock() mock_write_handoff = AsyncMock() with ( patch("sova.roles.planner.write_handoff_file", mock_write_file), @@ -4629,7 +4629,7 @@ async def test_write_handoff_db_path(self) -> None: assert call_args[0][0] == 99 async def test_write_handoff_db_exception_non_fatal(self) -> None: - from unittest.mock import patch + from unittest.mock import AsyncMock, MagicMock, patch from sova.roles.planner import PlannedTask, PlannerRole @@ -4647,7 +4647,7 @@ async def test_write_handoff_db_exception_non_fatal(self) -> None: ), ] with ( - patch("sova.roles.planner.write_handoff_file"), + patch("sova.roles.planner.write_handoff_file", new_callable=MagicMock), patch("sova.roles.planner.write_handoff", new=AsyncMock(side_effect=RuntimeError("DB down"))), ): # Should not raise diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 94a43ca6..e1f9d5c5 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -421,8 +421,6 @@ def test_server_start_help(self) -> None: assert "host" in result.output.lower() or "port" in result.output.lower() def test_server_status_shows_not_running(self) -> None: - from unittest.mock import patch - from typer.testing import CliRunner from sova.cli.app import app