diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e940da3 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,127 @@ +name: CI + +on: + pull_request: + branches: [main] + push: + branches: [main] + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + # ── 1. PR size gate ────────────────────────────────────────────────────────── + pr-size: + name: PR size gate (< 400 lines) + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - name: Fetch diff size + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + ADDITIONS=$(gh pr view ${{ github.event.pull_request.number }} \ + --repo ${{ github.repository }} \ + --json additions,deletions \ + --jq '.additions + .deletions') + echo "Diff size: ${ADDITIONS} lines" + if [ "$ADDITIONS" -gt 400 ]; then + echo "::error::PR diff is ${ADDITIONS} lines — exceeds the 400-line review limit." + echo "::error::Split this PR into smaller, focused changes. See code-review-standards.md." + exit 1 + fi + + # ── 2. Backend: lint, type-check, tests, coverage ──────────────────────────── + backend: + name: Backend — tests & coverage + runs-on: ubuntu-latest + defaults: + run: + working-directory: backend + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + cache: pip + cache-dependency-path: backend/requirements.txt + + - name: Install dependencies + run: | + pip install --upgrade pip + pip install -r requirements.txt + pip install pytest-cov ruff mypy + + - name: Lint (ruff) + run: ruff check . + + - name: Type-check (mypy) + run: mypy app/ --ignore-missing-imports --strict-optional + + - name: Tests + coverage + run: | + pytest tests/ \ + --cov=app \ + --cov-report=term-missing \ + --cov-report=xml:coverage.xml \ + --cov-fail-under=60 \ + -v + env: + DATABASE_URL: "sqlite+aiosqlite:///:memory:" + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@v4 + with: + name: backend-coverage + path: backend/coverage.xml + + # ── 3. Frontend: lint, type-check, build ───────────────────────────────────── + frontend: + name: Frontend — lint, type-check & build + runs-on: ubuntu-latest + defaults: + run: + working-directory: frontend + steps: + - uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + cache: npm + cache-dependency-path: frontend/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Lint (eslint) + run: npm run lint + + - name: Type-check (tsc) + run: npx tsc --noEmit + + - name: Build + run: npm run build + + # ── 4. Gate summary — all required checks must pass ────────────────────────── + ci-gate: + name: CI gate (required) + if: always() + needs: [backend, frontend] + runs-on: ubuntu-latest + steps: + - name: Check required jobs + run: | + BACKEND="${{ needs.backend.result }}" + FRONTEND="${{ needs.frontend.result }}" + echo "backend=${BACKEND} frontend=${FRONTEND}" + if [ "$BACKEND" != "success" ] || [ "$FRONTEND" != "success" ]; then + echo "::error::One or more required CI jobs did not succeed." + exit 1 + fi + echo "All required checks passed." diff --git a/backend/agent/tools.py b/backend/agent/tools.py index 5e94f32..c8299ad 100644 --- a/backend/agent/tools.py +++ b/backend/agent/tools.py @@ -68,7 +68,7 @@ def _resolve_dns_impl(target_url: str) -> str: addrs: list[dict[str, str]] = [] try: for family, _, _, _, sockaddr in socket.getaddrinfo(host, None): - ip = sockaddr[0] + ip = str(sockaddr[0]) fam = "IPv6" if family == socket.AF_INET6 else "IPv4" addrs.append({"family": fam, "address": ip}) except OSError as e: diff --git a/backend/agent/vecna_litellm.py b/backend/agent/vecna_litellm.py index 7b04ee3..8a2d669 100644 --- a/backend/agent/vecna_litellm.py +++ b/backend/agent/vecna_litellm.py @@ -77,7 +77,7 @@ async def _vecna_acompletion(*args: Any, **kwargs: Any): litellm_main.completion = _vecna_completion litellm_main.acompletion = _vecna_acompletion -import litellm as _litellm_pkg +import litellm as _litellm_pkg # noqa: E402 (intentional: must follow litellm_main patching) _litellm_pkg.completion = _vecna_completion _litellm_pkg.acompletion = _vecna_acompletion diff --git a/backend/app/main.py b/backend/app/main.py index 23f7cfe..abd3c96 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -6,12 +6,11 @@ import time import uuid from contextlib import asynccontextmanager -from fastapi import Depends, FastAPI, HTTPException, Request, WebSocket, WebSocketDisconnect +from fastapi import Depends, FastAPI, HTTPException, WebSocket, WebSocketDisconnect from fastapi.responses import PlainTextResponse from fastapi.middleware.cors import CORSMiddleware from pydantic import BaseModel, Field from sqlalchemy import func, select -from sqlalchemy.ext.asyncio import async_sessionmaker from app.config import Settings # loads agent.vecna_litellm (patches litellm for OpenRouter) diff --git a/backend/app/services/operation_context.py b/backend/app/services/operation_context.py index 96d0f5f..785683c 100644 --- a/backend/app/services/operation_context.py +++ b/backend/app/services/operation_context.py @@ -7,6 +7,7 @@ from __future__ import annotations import asyncio +import concurrent.futures import logging from contextvars import ContextVar, Token from typing import Any @@ -109,7 +110,9 @@ def schedule_tool_emit(payload: dict[str, Any]) -> None: return fut = asyncio.run_coroutine_threadsafe(_emit_tool_async(payload), loop) - def _done(f: asyncio.Future[None]) -> None: + # run_coroutine_threadsafe returns a concurrent.futures.Future, not an + # asyncio.Future, so the done-callback must be typed against that. + def _done(f: concurrent.futures.Future[None]) -> None: try: exc = f.exception() if exc: diff --git a/backend/pytest.ini b/backend/pytest.ini new file mode 100644 index 0000000..a86beef --- /dev/null +++ b/backend/pytest.ini @@ -0,0 +1,5 @@ +[pytest] +# Ensure the backend package root is importable when pytest is invoked via the +# console script (e.g. `pytest tests/`), which — unlike `python -m pytest` — +# does not add the current directory to sys.path. +pythonpath = . diff --git a/backend/tests/test_errors.py b/backend/tests/test_errors.py index f597ee3..1c5bca3 100644 --- a/backend/tests/test_errors.py +++ b/backend/tests/test_errors.py @@ -2,7 +2,6 @@ Tests error classification and retry delay behavior for the agent runner. """ -import math import pytest from app.services.errors import ( diff --git a/backend/tests/test_http_client.py b/backend/tests/test_http_client.py index d0f8fca..47891f7 100644 --- a/backend/tests/test_http_client.py +++ b/backend/tests/test_http_client.py @@ -2,7 +2,7 @@ import json import time -from unittest.mock import MagicMock, patch +from unittest.mock import patch import httpx import pytest diff --git a/backend/tests/test_lib_coverage.py b/backend/tests/test_lib_coverage.py new file mode 100644 index 0000000..af5dc22 --- /dev/null +++ b/backend/tests/test_lib_coverage.py @@ -0,0 +1,184 @@ +"""Unit coverage for pure helpers: url_guard, serialize, report.""" + +from __future__ import annotations + +from types import SimpleNamespace + +from strands.agent.agent_result import AgentResult + +from app.lib.report import build_structured_report +from app.lib.serialize import event_to_jsonable +from app.models import Operation +from app.url_guard import is_safe_public_target + + +def test_url_guard_allows_public_host() -> None: + ok, reason = is_safe_public_target("https://example.com/path") + assert ok + assert reason == "" + # scheme is added when missing + ok2, _ = is_safe_public_target("example.com") + assert ok2 + + +def test_url_guard_blocks_loopback_and_metadata() -> None: + for target in ( + "http://localhost:8000", + "https://metadata.google.internal", + "https://app.localhost", + "https://0.0.0.0", + ): + ok, reason = is_safe_public_target(target) + assert not ok, target + assert reason + + +def test_url_guard_blocks_private_and_special_ips() -> None: + for target in ( + "http://127.0.0.1", + "http://10.0.0.5", + "http://192.168.1.1", + "http://169.254.169.254", + "http://172.16.0.1", + ): + ok, _ = is_safe_public_target(target) + assert not ok, target + + +def test_url_guard_missing_host_and_ipv6_bracket() -> None: + ok, reason = is_safe_public_target("http://") + assert not ok + assert reason == "Missing host" + ok2, reason2 = is_safe_public_target("http://[::1]/") + assert not ok2 + assert reason2 + + +def test_url_guard_invalid_ipv4_literal() -> None: + # matches the dotted-quad regex but is not a valid address → "Invalid IP" + ok, reason = is_safe_public_target("http://999.1.1.1") + assert not ok + assert reason == "Invalid IP" + + +def test_serialize_lifecycle_events() -> None: + assert event_to_jsonable({"init_event_loop": True})["human_line"] == "Initializing agent…" + assert event_to_jsonable({"start_event_loop": True})["human_line"] == "LLM turn started" + assert event_to_jsonable({"start": True})["human_line"] == "Event loop starting" + + +def test_serialize_reasoning_and_drops() -> None: + out = event_to_jsonable( + {"reasoning": True, "reasoningText": "thinking hard", "agent": "drop-me"} + ) + assert out["human_line"].startswith("Reasoning:") + assert "agent" not in out + + +def test_serialize_content_block_events() -> None: + tool_start = {"event": {"contentBlockStart": {"start": {"toolUse": {"name": "scan"}}}}} + assert event_to_jsonable(tool_start)["human_line"] == "Tool call: scan" + + text_delta = {"event": {"contentBlockDelta": {"delta": {"text": "hello"}}}} + assert event_to_jsonable(text_delta)["human_line"] == "Text: hello" + + msg_start = {"event": {"messageStart": {"role": "assistant"}}} + assert event_to_jsonable(msg_start)["human_line"] == "Assistant message (assistant)" + + stop = {"event": {"messageStop": {"stopReason": "end_turn"}}} + assert event_to_jsonable(stop)["human_line"] == "Message stopped (end_turn)" + + +def test_serialize_metadata_usage_and_safe() -> None: + meta = {"event": {"metadata": {"usage": {"inputTokens": 10, "outputTokens": 5}}}} + assert "Token usage" in event_to_jsonable(meta)["human_line"] + # _safe coerces nested/non-primitive values + out = event_to_jsonable({"nested": {"k": (1, 2)}, "obj": object()}) + assert out["nested"] == {"k": [1, 2]} + assert isinstance(out["obj"], str) + + +def test_serialize_fallback_stream_event() -> None: + assert event_to_jsonable({"unknown": "x"})["human_line"] == "Stream event" + + +def test_serialize_agent_result_branch() -> None: + result = AgentResult( + stop_reason="end_turn", + message={"content": [{"text": "final answer"}, {"noise": 1}]}, + metrics=SimpleNamespace( + cycle_count=3, + tool_metrics={"scan": SimpleNamespace(call_count=2)}, + ), + state={}, + ) + out = event_to_jsonable({"result": result}) + assert out["type"] == "agent_result" + assert out["metrics"]["cycle_count"] == 3 + assert out["metrics"]["tool_calls"] == {"scan": 2} + assert "Cycle 3" in out["human_line"] + assert "scan×2" in out["human_line"] + assert "final answer" in out["human_line"] + + +def test_serialize_more_content_blocks() -> None: + # nested delta reasoningContent → fallback reasoning snippet + assert event_to_jsonable( + {"delta": {"reasoningContent": {"text": "deep thought"}}} + )["human_line"] == "Reasoning: deep thought" + # tool-args delta + tool_args = {"event": {"contentBlockDelta": {"delta": {"toolUse": {"input": "{}"}}}}} + assert event_to_jsonable(tool_args)["human_line"].startswith("Tool args:") + # content block start without toolUse, and content block stop + assert ( + event_to_jsonable({"event": {"contentBlockStart": {"start": {}}}})["human_line"] + == "Content block started" + ) + assert ( + event_to_jsonable({"event": {"contentBlockStop": {}}})["human_line"] + == "Content block finished" + ) + # metadata without usage numbers → generic label + assert ( + event_to_jsonable({"event": {"metadata": {}}})["human_line"] == "Response metadata" + ) + # OpenAI-style reasoningContent nested in a contentBlockDelta + reasoning_delta = { + "event": {"contentBlockDelta": {"delta": {"reasoningContent": {"reasoningText": "why"}}}} + } + assert event_to_jsonable(reasoning_delta)["human_line"] == "Reasoning: why" + + +def test_build_structured_report_full() -> None: + op = Operation( + id="op-1", + target_url="https://example.com", + status="completed", + session_id="sess-1", + summary_text="done", + findings_json=[ + {"category": "dns", "title": "a"}, + {"category": "weird", "title": "b"}, + "raw-string-finding", + ], + events_json=[{"x": 1}], + cost_usd=1.0, + scan_fee_usd=0.5, + tool_fee_usd=0.25, + ) + report = build_structured_report(op) + assert report["schema_version"] == "1.0" + assert report["operation"]["id"] == "op-1" + assert report["telemetry"]["stream_event_count"] == 1 + grouped = report["findings_by_category"] + assert "dns" in grouped + # unknown category and raw string both fold into "other" + assert len(grouped["other"]) == 2 + + +def test_build_structured_report_empty_defaults() -> None: + op = Operation(id="op-2", target_url="https://ex.com", status="pending") + report = build_structured_report(op) + assert report["summary_markdown"] == "" + assert report["findings"] == [] + assert report["findings_by_category"] == {} diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 5d686ce..51d85bd 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -65,7 +65,6 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -275,10 +274,31 @@ "node": ">=6.9.0" } }, + "node_modules/@emnapi/core": { + "version": "1.11.2", + "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.11.2.tgz", + "integrity": "sha512-TC8MkTuZUtcTSiFeuC0ksCh9QIJ5+F21MvZ4Wn4ORfYaFJ/0dsiudv5tVkejgwZlwQ39jL9WWDe2lz8x0WglOA==", + "license": "MIT", + "optional": true, + "dependencies": { + "@emnapi/wasi-threads": "1.2.2", + "tslib": "^2.4.0" + } + }, + "node_modules/@emnapi/runtime": { + "version": "1.11.2", + "resolved": "https://registry.npmjs.org/@emnapi/runtime/-/runtime-1.11.2.tgz", + "integrity": "sha512-kyOl3X0DuTiT1h2ft8r2fYO8JYtU9a9Xis/zBSiGArNaagCOWx90N1k2wxp18czFDH+OgcWGb5ZP/XMt3dcyPA==", + "license": "MIT", + "optional": true, + "dependencies": { + "tslib": "^2.4.0" + } + }, "node_modules/@emnapi/wasi-threads": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@emnapi/wasi-threads/-/wasi-threads-1.2.1.tgz", - "integrity": "sha512-uTII7OYF+/Mes/MrcIOYp5yOtSMLBWSIoLPpcgwipoiKbli6k322tcoFsxoIIxPDqW01SQGAgko4EzZi2BNv2w==", + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/@emnapi/wasi-threads/-/wasi-threads-1.2.2.tgz", + "integrity": "sha512-c95qOXkHdydNKhscBTebqEC1CVAZpyqOfVfBzQ1qgzyl3gfeldUjIggDbIZgDKsHLgnsM+igH7TJ/eAasaVuMA==", "license": "MIT", "optional": true, "dependencies": { @@ -1301,7 +1321,6 @@ "integrity": "sha512-A1sre26ke7HDIuY/M23nd9gfB+nrmhtYyMINbjI1zHJxYteKR6qSMX56FsmjMcDb3SMcjJg5BiRRgOCC/yBD0g==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -1312,7 +1331,6 @@ "integrity": "sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -1323,7 +1341,6 @@ "integrity": "sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==", "devOptional": true, "license": "MIT", - "peer": true, "peerDependencies": { "@types/react": "^19.2.0" } @@ -1373,7 +1390,6 @@ "integrity": "sha512-rLoGZIf9afaRBYsPUMtvkDWykwXwUPL60HebR4JgTI8mxfFe2cQTu3AGitANp4b9B2QlVru6WzjgB2IzJKiCSA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.58.0", "@typescript-eslint/types": "8.58.0", @@ -1656,7 +1672,6 @@ "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -1765,7 +1780,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.10.12", "caniuse-lite": "^1.0.30001782", @@ -1988,7 +2002,6 @@ "integrity": "sha512-XoMjdBOwe/esVgEvLmNsD3IRHkm7fbKIUGvrleloJXUZgDHig2IPWNniv+GwjyJXzuNqVjlr5+4yVUZjycJwfQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -2952,7 +2965,6 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -3013,7 +3025,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -3023,7 +3034,6 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.4.tgz", "integrity": "sha512-AXJdLo8kgMbimY95O2aKQqsz2iWi9jMgKJhRBAxECE4IFxfcazB2LmzloIoibJI3C12IlY20+KFaLv+71bUJeQ==", "license": "MIT", - "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -3238,7 +3248,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -3324,7 +3333,6 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-8.0.3.tgz", "integrity": "sha512-B9ifbFudT1TFhfltfaIPgjo9Z3mDynBTJSUYxTjOQruf/zHH+ezCQKcoqO+h7a9Pw9Nm/OtlXAiGT1axBgwqrQ==", "license": "MIT", - "peer": true, "dependencies": { "lightningcss": "^1.32.0", "picomatch": "^4.0.4", @@ -3449,7 +3457,6 @@ "integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==", "dev": true, "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index ac7751b..ee29ade 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -167,6 +167,9 @@ export default function App() { const wsPingIntervalRef = useRef | null>(null) const wsOperationIdRef = useRef(null) const deepLinkHandledRef = useRef(false) + // Holds the latest connectWs so the reconnect timer can call it without the + // callback referencing itself before declaration. + const connectWsRef = useRef<((operationId: string) => void) | null>(null) // WebSocket reconnect / ping tuning const WS_MAX_RECONNECT_ATTEMPTS = 5 @@ -202,7 +205,9 @@ export default function App() { }, []) useEffect(() => { - void loadOps() + void (async () => { + await loadOps() + })() }, [loadOps]) useEffect(() => { @@ -365,7 +370,7 @@ export default function App() { const delay = Math.min(WS_BASE_RECONNECT_MS * Math.pow(2, attempt - 1), 32000) setWsReconnecting(true) wsReconnectTimerRef.current = setTimeout(() => { - connectWs(operationId) + connectWsRef.current?.(operationId) }, delay) } else if ( !terminalStatuses.includes(current) && @@ -381,6 +386,11 @@ export default function App() { [loadOps, _clearWsTimers, WS_MAX_RECONNECT_ATTEMPTS, WS_BASE_RECONNECT_MS, WS_PING_INTERVAL_MS], ) + // Keep the reconnect timer pointed at the latest connectWs. + useEffect(() => { + connectWsRef.current = connectWs + }, [connectWs]) + const startOperation = async () => { setError(null) setLiveLines([]) @@ -533,7 +543,9 @@ export default function App() { const op = params.get('op') if (!op || !/^[0-9a-f-]{36}$/i.test(op)) return deepLinkHandledRef.current = true - void openPastOperation(op) + void (async () => { + await openPastOperation(op) + })() window.history.replaceState({}, '', window.location.pathname) }, [openPastOperation]) diff --git a/frontend/src/components/ui/badge.tsx b/frontend/src/components/ui/badge.tsx index 8853195..50a4fa9 100644 --- a/frontend/src/components/ui/badge.tsx +++ b/frontend/src/components/ui/badge.tsx @@ -27,4 +27,4 @@ function Badge({ className, variant, ...props }: BadgeProps) { return
} -export { Badge, badgeVariants } +export { Badge } diff --git a/frontend/src/components/ui/button.tsx b/frontend/src/components/ui/button.tsx index 09e154f..90fb3a4 100644 --- a/frontend/src/components/ui/button.tsx +++ b/frontend/src/components/ui/button.tsx @@ -41,4 +41,4 @@ const Button = React.forwardRef( ) Button.displayName = 'Button' -export { Button, buttonVariants } +export { Button }