diff --git a/Makefile b/Makefile index 7c9c077..da1a24d 100644 --- a/Makefile +++ b/Makefile @@ -75,8 +75,17 @@ backend-test: ## Backend tests (pytest) cd $(BACKEND_DIR) && uv run pytest .PHONY: backend-coverage -backend-coverage: ## Backend tests with coverage gate (100% stmt + branch on covered src) - cd $(BACKEND_DIR) && uv run pytest --cov=app --cov-branch --cov-report=term-missing --cov-report=xml:coverage.xml +backend-coverage: ## Backend tests with coverage gate (scoped 100% stmt+branch on selected modules) + # Policy: enforce 100% coverage only for the explicitly scoped, unit-testable backend modules. + # Rationale: overall API/DB coverage is currently low; we will expand the scope as we add tests. + cd $(BACKEND_DIR) && uv run pytest \ + --cov=app.core.error_handling \ + --cov=app.services.mentions \ + --cov-branch \ + --cov-report=term-missing \ + --cov-report=xml:coverage.xml \ + --cov-report=json:coverage.json \ + --cov-fail-under=100 .PHONY: frontend-test frontend-test: ## Frontend tests (vitest) diff --git a/README.md b/README.md index 59ac5e1..4498e82 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,11 @@ Open http://localhost:3000. ## Common commands +### Coverage policy + +CI enforces a **scoped 100% coverage gate** (statements + branches) for a small set of unit-testable modules. +See `docs/coverage-policy.md`. + From repo root: ```bash diff --git a/backend/tests/test_error_handling.py b/backend/tests/test_error_handling.py index daaa5ed..00d0947 100644 --- a/backend/tests/test_error_handling.py +++ b/backend/tests/test_error_handling.py @@ -4,7 +4,9 @@ from fastapi import FastAPI, HTTPException from fastapi.testclient import TestClient from pydantic import BaseModel, Field -from app.core.error_handling import REQUEST_ID_HEADER, install_error_handling +from starlette.requests import Request + +from app.core.error_handling import REQUEST_ID_HEADER, _error_payload, _get_request_id, install_error_handling def test_request_validation_error_includes_request_id(): @@ -80,3 +82,38 @@ def test_response_validation_error_returns_500_with_request_id(): assert body["detail"] == "Internal Server Error" assert isinstance(body.get("request_id"), str) and body["request_id"] assert resp.headers.get(REQUEST_ID_HEADER) == body["request_id"] + + +def test_client_provided_request_id_is_preserved(): + app = FastAPI() + install_error_handling(app) + + @app.get("/needs-int") + def needs_int(limit: int) -> dict[str, int]: + return {"limit": limit} + + client = TestClient(app) + resp = client.get("/needs-int?limit=abc", headers={REQUEST_ID_HEADER: " req-123 "}) + + assert resp.status_code == 422 + body = resp.json() + assert body["request_id"] == "req-123" + assert resp.headers.get(REQUEST_ID_HEADER) == "req-123" + + +def test_get_request_id_returns_none_for_missing_or_invalid_state() -> None: + # Empty state + req = Request({"type": "http", "headers": [], "state": {}}) + assert _get_request_id(req) is None + + # Non-string request_id + req = Request({"type": "http", "headers": [], "state": {"request_id": 123}}) + assert _get_request_id(req) is None + + # Empty string request_id + req = Request({"type": "http", "headers": [], "state": {"request_id": ""}}) + assert _get_request_id(req) is None + + +def test_error_payload_omits_request_id_when_none() -> None: + assert _error_payload(detail="x", request_id=None) == {"detail": "x"} diff --git a/backend/tests/test_mentions.py b/backend/tests/test_mentions.py index 66c283d..8cfdce2 100644 --- a/backend/tests/test_mentions.py +++ b/backend/tests/test_mentions.py @@ -12,6 +12,21 @@ def test_matches_agent_mention_matches_first_name(): assert matches_agent_mention(agent, {"cooper"}) is False +def test_matches_agent_mention_no_mentions_is_false(): + agent = Agent(name="Alice") + assert matches_agent_mention(agent, set()) is False + + +def test_matches_agent_mention_empty_agent_name_is_false(): + agent = Agent(name=" ") + assert matches_agent_mention(agent, {"alice"}) is False + + +def test_matches_agent_mention_matches_full_normalized_name(): + agent = Agent(name="Alice Cooper") + assert matches_agent_mention(agent, {"alice cooper"}) is True + + def test_matches_agent_mention_supports_reserved_lead_shortcut(): lead = Agent(name="Riya", is_board_lead=True) other = Agent(name="Lead", is_board_lead=False) diff --git a/backend/tests/test_request_id_middleware.py b/backend/tests/test_request_id_middleware.py new file mode 100644 index 0000000..1f2bb99 --- /dev/null +++ b/backend/tests/test_request_id_middleware.py @@ -0,0 +1,85 @@ +from __future__ import annotations + +import pytest + +from app.core.error_handling import REQUEST_ID_HEADER, RequestIdMiddleware + + +@pytest.mark.asyncio +async def test_request_id_middleware_passes_through_non_http_scope() -> None: + called = False + + async def app(scope, receive, send): # type: ignore[no-untyped-def] + nonlocal called + called = True + + middleware = RequestIdMiddleware(app) + + scope = {"type": "websocket", "headers": []} + await middleware(scope, lambda: None, lambda message: None) # type: ignore[arg-type] + + assert called is True + + +@pytest.mark.asyncio +async def test_request_id_middleware_ignores_blank_client_header_and_generates_one() -> None: + captured_request_id: str | None = None + response_headers: list[tuple[bytes, bytes]] = [] + + async def app(scope, receive, send): # type: ignore[no-untyped-def] + nonlocal captured_request_id + captured_request_id = scope.get("state", {}).get("request_id") + await send({"type": "http.response.start", "status": 200, "headers": []}) + await send({"type": "http.response.body", "body": b"ok"}) + + async def send(message): # type: ignore[no-untyped-def] + if message["type"] == "http.response.start": + response_headers.extend(list(message.get("headers") or [])) + + middleware = RequestIdMiddleware(app) + + scope = { + "type": "http", + "headers": [(REQUEST_ID_HEADER.lower().encode("latin-1"), b" ")], + } + await middleware(scope, lambda: None, send) + + assert isinstance(captured_request_id, str) and captured_request_id + # Header should reflect the generated id, not the blank one. + values = [v for k, v in response_headers if k.lower() == REQUEST_ID_HEADER.lower().encode("latin-1")] + assert values == [captured_request_id.encode("latin-1")] + + +@pytest.mark.asyncio +async def test_request_id_middleware_does_not_duplicate_existing_header() -> None: + sent_start = False + start_headers: list[tuple[bytes, bytes]] | None = None + + async def app(scope, receive, send): # type: ignore[no-untyped-def] + # Simulate an app that already sets the request id header. + await send( + { + "type": "http.response.start", + "status": 200, + "headers": [(REQUEST_ID_HEADER.lower().encode("latin-1"), b"already")], + } + ) + await send({"type": "http.response.body", "body": b"ok"}) + + async def send(message): # type: ignore[no-untyped-def] + nonlocal sent_start, start_headers + if message["type"] == "http.response.start": + sent_start = True + start_headers = list(message.get("headers") or []) + + middleware = RequestIdMiddleware(app) + + scope = {"type": "http", "headers": []} + await middleware(scope, lambda: None, send) + + assert sent_start is True + assert start_headers is not None + + # Ensure the middleware did not append a second copy. + values = [v for k, v in start_headers if k.lower() == REQUEST_ID_HEADER.lower().encode("latin-1")] + assert values == [b"already"] diff --git a/docs/coverage-policy.md b/docs/coverage-policy.md new file mode 100644 index 0000000..6ed1b56 --- /dev/null +++ b/docs/coverage-policy.md @@ -0,0 +1,47 @@ +# Coverage policy (CI gate) + +## Why scoped coverage gates? + +Today, overall repository coverage is low (especially for API routes and Next pages), but we still want CI to **enforce quality deterministically**. + +So we start with a strict gate (100% statements + branches) on a **small, explicitly scoped** set of modules that are: + +- unit-testable without external services +- stable and high-signal for regressions + +We then expand the gated scope as we add tests. + +## Backend scope (100% required) + +Enforced in `Makefile` target `backend-coverage`: + +- `app.core.error_handling` +- `app.services.mentions` + +Command (CI): + +```bash +cd backend && uv run pytest \ + --cov=app.core.error_handling \ + --cov=app.services.mentions \ + --cov-branch \ + --cov-report=term-missing \ + --cov-report=xml:coverage.xml \ + --cov-report=json:coverage.json \ + --cov-fail-under=100 +``` + +## Frontend scope (100% required) + +Enforced in `frontend/vitest.config.ts` coverage settings: + +- include: `src/lib/backoff.ts` +- thresholds: 100% for lines/statements/functions/branches + +This is intentionally limited to a single pure utility module first. As we add more unit tests in `src/lib/**` and React Testing Library component tests for `src/app/**` + `src/components/**`, we should expand the include list and keep thresholds strict. + +## How to expand the gate + +- Add tests for the next-highest-signal modules. +- Add them to the gated scope (backend `--cov=` list; frontend `coverage.include`). +- Keep the threshold at 100% for anything included in the gate. diff --git a/frontend/cypress/e2e/activity_feed.cy.ts b/frontend/cypress/e2e/activity_feed.cy.ts new file mode 100644 index 0000000..b0f2e23 --- /dev/null +++ b/frontend/cypress/e2e/activity_feed.cy.ts @@ -0,0 +1,96 @@ +/// + +describe("/activity feed", () => { + const apiBase = "**/api/v1"; + + function stubStreamEmpty() { + // Return a minimal SSE response that ends immediately. + cy.intercept( + "GET", + `${apiBase}/activity/task-comments/stream*`, + { + statusCode: 200, + headers: { + "content-type": "text/event-stream", + }, + body: "", + }, + ).as("activityStream"); + } + + it("happy path: renders task comment cards", () => { + cy.intercept("GET", `${apiBase}/activity/task-comments*`, { + statusCode: 200, + body: { + items: [ + { + id: "c1", + message: "Hello world", + agent_name: "Kunal", + agent_role: "QA 2", + board_id: "b1", + board_name: "Testing", + task_id: "t1", + task_title: "CI hardening", + created_at: "2026-02-07T00:00:00Z", + }, + { + id: "c2", + message: "Second comment", + agent_name: "Riya", + agent_role: "QA", + board_id: "b1", + board_name: "Testing", + task_id: "t2", + task_title: "Coverage policy", + created_at: "2026-02-07T00:01:00Z", + }, + ], + }, + }).as("activityList"); + + stubStreamEmpty(); + + cy.visit("/activity", { + onBeforeLoad(win: Window) { + win.localStorage.clear(); + }, + }); + + cy.wait("@activityList"); + + cy.contains(/live feed/i).should("be.visible"); + cy.contains("CI hardening").should("be.visible"); + cy.contains("Coverage policy").should("be.visible"); + cy.contains("Hello world").should("be.visible"); + }); + + it("empty state: shows waiting message when no items", () => { + cy.intercept("GET", `${apiBase}/activity/task-comments*`, { + statusCode: 200, + body: { items: [] }, + }).as("activityList"); + + stubStreamEmpty(); + + cy.visit("/activity"); + cy.wait("@activityList"); + + cy.contains(/waiting for new comments/i).should("be.visible"); + }); + + it("error state: shows failure UI when API errors", () => { + cy.intercept("GET", `${apiBase}/activity/task-comments*`, { + statusCode: 500, + body: { detail: "boom" }, + }).as("activityList"); + + stubStreamEmpty(); + + cy.visit("/activity"); + cy.wait("@activityList"); + + // UI uses query.error.message or fallback. + cy.contains(/unable to load feed|boom/i).should("be.visible"); + }); +}); diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 893fc76..3894dfd 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -4803,19 +4803,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/ansi-escapes/node_modules/type-fest": { - "version": "0.21.3", - "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.21.3.tgz", - "integrity": "sha512-t0rzBq87m3fVcduHDUFhKmyyX+9eo6WQjZvf51Ea/M0Q7+T374Jp1aUiyUl0GKxp8M/OETVHSDvmkyPgvX+X2w==", - "dev": true, - "license": "(MIT OR CC0-1.0)", - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/ansi-regex": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz", @@ -8310,6 +8297,16 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/hasha/node_modules/type-fest": { + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.8.1.tgz", + "integrity": "sha512-4dbzIzqvjtgiM5rw1k5rEHtBANKmdudhGyBEajN01fEyhaAIhsoKNy6y7+IN93IfpFtwY9iqi7kD+xwKhQsNJA==", + "dev": true, + "license": "(MIT OR CC0-1.0)", + "engines": { + "node": ">=8" + } + }, "node_modules/hasown": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.2.tgz", @@ -14036,13 +14033,16 @@ } }, "node_modules/type-fest": { - "version": "0.8.1", - "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.8.1.tgz", - "integrity": "sha512-4dbzIzqvjtgiM5rw1k5rEHtBANKmdudhGyBEajN01fEyhaAIhsoKNy6y7+IN93IfpFtwY9iqi7kD+xwKhQsNJA==", + "version": "0.21.3", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.21.3.tgz", + "integrity": "sha512-t0rzBq87m3fVcduHDUFhKmyyX+9eo6WQjZvf51Ea/M0Q7+T374Jp1aUiyUl0GKxp8M/OETVHSDvmkyPgvX+X2w==", "dev": true, "license": "(MIT OR CC0-1.0)", "engines": { - "node": ">=8" + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" } }, "node_modules/typed-array-buffer": { diff --git a/frontend/package.json b/frontend/package.json index 4568526..207cc25 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -43,6 +43,7 @@ "autoprefixer": "^10.4.24", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", + "cypress": "^14.0.0", "eslint": "^9", "eslint-config-next": "16.1.6", "jsdom": "^25.0.1", @@ -54,7 +55,6 @@ "tailwindcss": "^3.4.17", "tailwindcss-animate": "^1.0.7", "typescript": "^5", - "vitest": "^2.1.8", - "cypress": "^14.0.0" + "vitest": "^2.1.8" } } diff --git a/frontend/src/auth/clerk.tsx b/frontend/src/auth/clerk.tsx index 02cff2c..e5e4483 100644 --- a/frontend/src/auth/clerk.tsx +++ b/frontend/src/auth/clerk.tsx @@ -19,20 +19,29 @@ import type { ComponentProps } from "react"; import { isLikelyValidClerkPublishableKey } from "@/auth/clerkKey"; +function isE2EAuthBypassEnabled(): boolean { + // Used only for Cypress E2E to keep tests secretless and deterministic. + // When enabled, we treat the user as signed in and skip Clerk entirely. + return process.env.NEXT_PUBLIC_E2E_AUTH_BYPASS === "1"; +} + export function isClerkEnabled(): boolean { // IMPORTANT: keep this in sync with AuthProvider; otherwise components like // may render without a and crash during prerender. + if (isE2EAuthBypassEnabled()) return false; return isLikelyValidClerkPublishableKey( process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY, ); } export function SignedIn(props: { children: ReactNode }) { + if (isE2EAuthBypassEnabled()) return <>{props.children}; if (!isClerkEnabled()) return null; return {props.children}; } export function SignedOut(props: { children: ReactNode }) { + if (isE2EAuthBypassEnabled()) return null; if (!isClerkEnabled()) return <>{props.children}; return {props.children}; } @@ -58,6 +67,15 @@ export function useUser() { } export function useAuth() { + if (isE2EAuthBypassEnabled()) { + return { + isLoaded: true, + isSignedIn: true, + userId: "e2e-user", + sessionId: "e2e-session", + getToken: async () => "e2e-token", + } as const; + } if (!isClerkEnabled()) { return { isLoaded: true, diff --git a/frontend/src/lib/backoff.test.ts b/frontend/src/lib/backoff.test.ts index a83a6f7..9fbba8b 100644 --- a/frontend/src/lib/backoff.test.ts +++ b/frontend/src/lib/backoff.test.ts @@ -20,6 +20,23 @@ describe("createExponentialBackoff", () => { expect(backoff.nextDelayMs()).toBe(250); // capped }); + it("clamps invalid numeric options and treats negative jitter as zero", () => { + vi.spyOn(Math, "random").mockReturnValue(0.9999); + + // baseMs: NaN should clamp to min (50) + // maxMs: Infinity should clamp to min (= baseMs) + // jitter: negative -> treated as 0 (no extra delay) + const backoff = createExponentialBackoff({ + baseMs: Number.NaN, + maxMs: Number.POSITIVE_INFINITY, + jitter: -1, + }); + + // With maxMs clamped to baseMs, delay will always be baseMs + expect(backoff.nextDelayMs()).toBe(50); + expect(backoff.nextDelayMs()).toBe(50); + }); + it("reset brings attempt back to zero", () => { vi.spyOn(Math, "random").mockReturnValue(0); @@ -30,4 +47,15 @@ describe("createExponentialBackoff", () => { backoff.reset(); expect(backoff.attempt()).toBe(0); }); + + it("uses defaults when options are omitted", () => { + vi.spyOn(Math, "random").mockReturnValue(0); + + const backoff = createExponentialBackoff(); + expect(backoff.attempt()).toBe(0); + + // Default baseMs is 1000 (clamped within bounds), jitter default is 0.2. + // With Math.random=0, delay should be the normalized base (1000). + expect(backoff.nextDelayMs()).toBe(1000); + }); }); diff --git a/frontend/vitest.config.ts b/frontend/vitest.config.ts index 197150a..43c614e 100644 --- a/frontend/vitest.config.ts +++ b/frontend/vitest.config.ts @@ -9,8 +9,16 @@ export default defineConfig({ provider: "v8", reporter: ["text", "lcov"], reportsDirectory: "./coverage", - include: ["src/**/*.{ts,tsx}"], + // Policy (scoped gate): require 100% coverage on *explicitly listed* unit-testable modules first. + // We'll expand this include list as we add more unit/component tests. + include: ["src/lib/backoff.ts"], exclude: ["**/*.d.ts", "src/**/__generated__/**", "src/**/generated/**"], + thresholds: { + lines: 100, + statements: 100, + functions: 100, + branches: 100, + }, }, }, });