diff --git a/CLAUDE.md b/CLAUDE.md index 4828ae2c..e7f4c760 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,9 +36,11 @@ If you are an agent working in this repo: **do not improvise architecture**. Fol ### Current Focus: Phase 4A -**Phase 5.5 is in progress** — GitHub Issues import. Repo connection via PAT (#563) is **complete**: Settings → **Integrations** tab connects a GitHub repo with a Personal Access Token. Backend `POST/DELETE/GET /api/v2/integrations/github/{connect,disconnect,status}` (`ui/routers/github_integrations_v2.py`). Validation is headless in `core/github_connect_service.py` (httpx; verifies token, repo visibility, and issues-read access; typed errors → 401/404/403). The PAT is stored machine-wide via `CredentialManager` (`CredentialProvider.GIT_GITHUB`, the #555 pattern) and **never returned in any response**; non-secret repo metadata persists per-workspace in `.codeframe/github_integration.json` (`core/github_integration_config.py`). Frontend: `GitHubIntegrationCard` + `integrationsApi`. +**Phase 5.5 is complete** — GitHub Issues import. Repo connection via PAT (#563) is **complete**: Settings → **Integrations** tab connects a GitHub repo with a Personal Access Token. Backend `POST/DELETE/GET /api/v2/integrations/github/{connect,disconnect,status}` (`ui/routers/github_integrations_v2.py`). Validation is headless in `core/github_connect_service.py` (httpx; verifies token, repo visibility, and issues-read access; typed errors → 401/404/403). The PAT is stored machine-wide via `CredentialManager` (`CredentialProvider.GIT_GITHUB`, the #555 pattern) and **never returned in any response**; non-secret repo metadata persists per-workspace in `.codeframe/github_integration.json` (`core/github_integration_config.py`). Frontend: `GitHubIntegrationCard` + `integrationsApi`. -Issue **browse** (#564) is **complete**: `GET /api/v2/integrations/github/issues?page&per_page&search&label` on the same router lists the connected repo's **open** issues (PRs filtered out) — repo from `.codeframe/github_integration.json`, PAT from `CredentialManager`, **409** when not connected. Headless fetch in `core/github_issues_service.py` (`list_issues`): plain `/repos/{o}/{r}/issues` by default, routes to `/search/issues` for free-text search, `labels=` filter, `Link`-header pagination, 60s in-process TTL cache, typed errors → 401/403/502. Frontend: `GitHubIssueImportModal` (paginated list, debounced search, label filter, multi-select that persists across pages, select-all-on-page, Import-Selected gated on ≥1) + `integrationsApi.getIssues`; an **Import from GitHub** button on `/tasks` (`TaskBoardView`) shown only when connected. The import action itself is the remaining 5.5 work (#565). +Issue **browse** (#564) is **complete**: `GET /api/v2/integrations/github/issues?page&per_page&search&label` on the same router lists the connected repo's **open** issues (PRs filtered out) — repo from `.codeframe/github_integration.json`, PAT from `CredentialManager`, **409** when not connected. Headless fetch in `core/github_issues_service.py` (`list_issues`): plain `/repos/{o}/{r}/issues` by default, routes to `/search/issues` for free-text search, `labels=` filter, `Link`-header pagination, 60s in-process TTL cache, typed errors → 401/403/502. Frontend: `GitHubIssueImportModal` (paginated list, debounced search, label filter, multi-select that persists across pages, select-all-on-page, Import-Selected gated on ≥1) + `integrationsApi.getIssues`; an **Import from GitHub** button on `/tasks` (`TaskBoardView`) shown only when connected. + +Issue **import + traceability** (#565) is **complete**: `POST /api/v2/integrations/github/import` (same router) turns selected issues into tasks — title verbatim, body as description (+ a best-effort `**Labels:**` footer), linked via `github_issue_number` + `external_url`; PRs are rejected (`NotAnIssueError`→422), missing issues 404, fetch failures 502, malformed saved repo 409. Import is two-phase (fetch+dedupe all, then create) with rollback on a mid-create DB error; dedup is keyed on the full issue URL and backed by a `UNIQUE(workspace_id, external_url)` index (atomic across concurrent imports). Issue ops live in `core/github_issues_service.py` (`get_issue`, `close_issue`). **Auto-close**: marking an opted-in imported task DONE closes the linked issue — fired from core `tasks.update_status` so the web UI, CLI, and agent/batch paths all trigger it; the close targets the task's *source* repo parsed from `external_url` (not the live connection) and runs off the caller's path (event loop in the server, non-daemon thread in CLI). `TaskResponse` exposes the three traceability fields; `PATCH /api/v2/tasks/{id}` accepts `auto_close_github_issue` (persist-first + rollback-on-rejected-transition, with late opt-in on already-DONE tasks). Frontend: `GitHubIssueBadge`, import wiring in `TaskBoardView` (progress, in-modal error, summary banner), badge + auto-close checkbox in `TaskDetailModal`, `integrationsApi.importIssues` + `tasksApi.updateGitHubSettings`. **Known limitation**: auto-close uses the single machine-wide GIT_GITHUB PAT, so closing an older imported repo's issue after reconnecting to a different repo may fail if that PAT lacks access. **Phase 5.4 is complete** — PRD stress-test web UI: trigger + streaming (#561). Backend: `GET /api/v2/prd/stress-test` SSE endpoint streams `goals_extracted`, `goal_analyzed`, `complete`, and `error` events from `core/prd_stress_test.py:stress_test_prd_stream()`, resolving the LLM provider via the standard chain and applying the standard rate limit. Frontend: `useStressTestStream` hook manages the SSE connection and event accumulation; `StressTestModal` renders the streaming progress and is opened via a "Stress Test" button on the `/prd` page (enabled only when a PRD exists). Results rendering + refinement (#562) is **complete**: the `complete` SSE event now carries structured, severity-tagged `ambiguities` (`Ambiguity.severity` is `"blocking"`/`"warning"`); `StressTestModal` shows a results view of `AmbiguityCard`s (question text, severity badge, answer textarea) with an "X of Y answered" progress indicator and a **[Refine PRD]** button (disabled until every blocking ambiguity is answered). Refine posts to `POST /api/v2/prd/stress-test/refine`, which folds the answers into a new PRD version via `resolve_ambiguities_into_prd` (offloaded with `asyncio.to_thread`) and `prd.create_new_version`, then `mutatePrd` reflects it in the editor. diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index ec26ff2a..6dc837d1 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -37,6 +37,23 @@ _TIMEOUT = 15.0 + +class NotAnIssueError(Exception): + """The requested number refers to a pull request, not an issue (#565). + + Intentionally NOT a ``GitHubConnectError`` subclass: callers map it to a + client error (the caller sent a PR number), not a GitHub upstream failure. + """ + + +class IssueNotFoundError(Exception): + """The requested issue number does not exist in the repo (404) (#565). + + Intentionally NOT a ``GitHubConnectError`` subclass: a missing/stale issue + number is a client error (bad payload), not a GitHub upstream failure, so + callers map it to a 4xx rather than a 502. + """ + # Parse the ``page=N`` query param out of a Link header's rel="last" URL. _LAST_PAGE_RE = re.compile(r'[?&]page=(\d+)[^>]*>;\s*rel="last"') @@ -190,6 +207,207 @@ async def _list_issues( return issues, total +class GitHubIssueDetail(TypedDict): + number: int + title: str + body: str + labels: list[str] + html_url: str + + +async def get_issue( + pat: str, + repo: str, + number: int, + *, + client: Optional[httpx.AsyncClient] = None, +) -> GitHubIssueDetail: + """Fetch a single issue's details for import (issue #565). + + Unlike the list endpoint, this returns the issue ``body`` so the importer + can populate the task description. + + Args: + pat: GitHub Personal Access Token. + repo: Repository in ``owner/repo`` format. + number: Issue number to fetch. + client: Optional httpx client (injected by tests). When ``None`` a + short-lived client is created and closed internally. + + Returns: + ``{number, title, body, labels, html_url}`` — ``body`` is normalized to + ``""`` when GitHub returns null. + + Raises: + ValueError: if ``repo`` is not a valid ``owner/repo`` string. + InvalidTokenError: GitHub returned 401. + InsufficientScopeError: the token cannot read issues (403). + GitHubConnectError: any other non-success response or network error. + """ + owner, name = parse_repo(repo) + + own_client = client is None + if own_client: + client = httpx.AsyncClient(timeout=_TIMEOUT) + try: + try: + resp = await client.get( + f"{GITHUB_API_BASE}/repos/{owner}/{name}/issues/{number}", + headers=_headers(pat), + ) + except httpx.HTTPError as exc: + logger.warning("GitHub get issue failed: %s", type(exc).__name__) + raise GitHubConnectError("Could not reach GitHub. Try again later.") + + # A 404 on /issues/{n} is ambiguous: the issue may genuinely not exist, + # OR the repo/token became inaccessible (renamed/deleted repo, rotated + # token). Probe the repo to tell a client typo (-> IssueNotFoundError, + # 404) apart from a broken integration (-> connect/auth error) so callers + # get the right recovery path. The probe only runs on the 404 path. + if resp.status_code == 404: + try: + repo_resp = await client.get( + f"{GITHUB_API_BASE}/repos/{owner}/{name}", headers=_headers(pat) + ) + except httpx.HTTPError as exc: + logger.warning("GitHub repo probe failed: %s", type(exc).__name__) + raise GitHubConnectError("Could not reach GitHub. Try again later.") + if repo_resp.status_code == 401: + raise InvalidTokenError("Invalid GitHub token.") + if repo_resp.status_code == 403: + raise InsufficientScopeError( + "Token lacks access to this repository." + ) + if repo_resp.status_code == 404: + raise GitHubConnectError( + f"Repository '{repo}' is no longer accessible." + ) + if repo_resp.status_code >= 400: + # Rate limit / 5xx / other failure on the probe — a real upstream + # problem, NOT a missing issue. Surface it as such so the caller + # retries rather than blaming the issue number. + raise GitHubConnectError( + f"GitHub repo check returned status {repo_resp.status_code}." + ) + # Repo probe succeeded (2xx) → the issue itself genuinely does not + # exist. (A 3xx would also land here, but GitHub answers repo lookups + # with 2xx/4xx, not redirects, for this endpoint.) + if repo_resp.status_code >= 300: + raise GitHubConnectError( + f"GitHub repo check returned status {repo_resp.status_code}." + ) + raise IssueNotFoundError(f"Issue #{number} was not found in '{repo}'.") + _raise_for_status(resp.status_code, context="get issue") + + raw = resp.json() + if not isinstance(raw, dict): + raw = {} + # The issues endpoint also returns pull requests (a PR is an issue with a + # ``pull_request`` member). Reject them so the import stays consistent + # with ``list_issues`` (which excludes PRs) and never links a PR as an + # issue. + if "pull_request" in raw: + raise NotAnIssueError(f"#{number} is a pull request, not an issue.") + labels_raw = raw.get("labels") or [] + labels = [ + (lbl.get("name") if isinstance(lbl, dict) else str(lbl)) + for lbl in labels_raw + ] + labels = [n for n in labels if n] + return { + "number": int(raw.get("number", number)), + "title": str(raw.get("title") or ""), + "body": str(raw.get("body") or ""), + "labels": labels, + "html_url": str(raw.get("html_url") or ""), + } + finally: + if own_client: + await client.aclose() + + +async def close_issue( + pat: str, + repo: str, + number: int, + *, + comment: Optional[str] = None, + timeout: float = _TIMEOUT, + client: Optional[httpx.AsyncClient] = None, +) -> bool: + """Close a GitHub issue, optionally posting a comment first (issue #565). + + Args: + pat: GitHub Personal Access Token. + repo: Repository in ``owner/repo`` format. + number: Issue number to close. + comment: Optional comment body to post before closing. + timeout: HTTP timeout in seconds for the (self-created) client. Auto-close + passes a short value so a hung close never stalls a caller for long. + client: Optional httpx client (injected by tests). When ``None`` a + short-lived client is created and closed internally. + + Returns: + ``True`` when the issue was closed. + + Raises: + ValueError: if ``repo`` is not a valid ``owner/repo`` string. + InvalidTokenError: GitHub returned 401. + InsufficientScopeError: the token cannot write issues (403). + GitHubConnectError: any other non-success response or network error. + """ + owner, name = parse_repo(repo) + + own_client = client is None + if own_client: + client = httpx.AsyncClient(timeout=timeout) + try: + headers = _headers(pat) + base = f"{GITHUB_API_BASE}/repos/{owner}/{name}/issues/{number}" + + if comment: + # Best-effort: the comment is cosmetic. A failure to post it (locked + # issue, repo with commenting disabled, transient error) must NOT + # prevent the close itself, which is the operation that matters. + try: + cresp = await client.post( + f"{base}/comments", json={"body": comment}, headers=headers + ) + if cresp.status_code >= 400: + logger.warning( + "GitHub issue comment returned %s; closing anyway.", + cresp.status_code, + ) + except httpx.HTTPError as exc: + logger.warning( + "GitHub issue comment failed (%s); closing anyway.", + type(exc).__name__, + ) + + try: + resp = await client.patch( + base, json={"state": "closed"}, headers=headers + ) + except httpx.HTTPError as exc: + logger.warning("GitHub close issue failed: %s", type(exc).__name__) + raise GitHubConnectError("Could not reach GitHub. Try again later.") + + _raise_for_status(resp.status_code, context="close issue") + # A redirect (3xx) — e.g. a moved/renamed/transferred repo — means the + # PATCH was NOT applied (httpx does not follow redirects by default), so + # the issue is still open. Treat it as a failure rather than reporting a + # silent success. + if resp.status_code >= 300: + raise GitHubConnectError( + f"GitHub close returned status {resp.status_code}; " + "issue was not closed (repository may have moved)." + ) + return True + finally: + if own_client: + await client.aclose() + + async def _search_issues( client: httpx.AsyncClient, headers: dict[str, str], diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index 6834f011..0db062b1 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -5,17 +5,23 @@ This module is headless - no FastAPI or HTTP dependencies. """ +import asyncio import json +import logging import re +import threading import uuid from dataclasses import dataclass, field from datetime import datetime, timezone from typing import Optional +from urllib.parse import urlparse from codeframe.core.state_machine import TaskStatus, validate_transition from codeframe.core.workspace import Workspace, get_db_connection from codeframe.core.prd import PrdRecord +logger = logging.getLogger(__name__) + def _utc_now() -> datetime: """Get current UTC time as timezone-aware datetime.""" @@ -61,6 +67,8 @@ class Task: is_leaf: bool = True hierarchical_id: Optional[str] = None requirement_ids: list[str] = field(default_factory=list) + external_url: Optional[str] = None + auto_close_github_issue: bool = False def create( @@ -79,6 +87,9 @@ def create( is_leaf: bool = True, hierarchical_id: Optional[str] = None, requirement_ids: Optional[list[str]] = None, + github_issue_number: Optional[int] = None, + external_url: Optional[str] = None, + auto_close_github_issue: bool = False, ) -> Task: """Create a new task. @@ -113,10 +124,10 @@ def create( cursor = conn.cursor() cursor.execute( """ - INSERT INTO tasks (id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, parent_id, lineage, is_leaf, hierarchical_id, created_at, updated_at, requirement_ids) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + INSERT INTO tasks (id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, parent_id, lineage, is_leaf, hierarchical_id, created_at, updated_at, requirement_ids, github_issue_number, external_url, auto_close_github_issue) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, - (task_id, workspace.id, prd_id, title, description, status.value, priority, json.dumps(depends_on_list), estimated_hours, complexity_score, uncertainty_level, parent_id, json.dumps(lineage_list), 1 if is_leaf else 0, hierarchical_id, now, now, json.dumps(requirement_ids_list)), + (task_id, workspace.id, prd_id, title, description, status.value, priority, json.dumps(depends_on_list), estimated_hours, complexity_score, uncertainty_level, parent_id, json.dumps(lineage_list), 1 if is_leaf else 0, hierarchical_id, now, now, json.dumps(requirement_ids_list), github_issue_number, external_url, 1 if auto_close_github_issue else 0), ) conn.commit() finally: @@ -139,6 +150,9 @@ def create( is_leaf=is_leaf, hierarchical_id=hierarchical_id, requirement_ids=requirement_ids_list, + github_issue_number=github_issue_number, + external_url=external_url, + auto_close_github_issue=auto_close_github_issue, created_at=datetime.fromisoformat(now), updated_at=datetime.fromisoformat(now), ) @@ -159,7 +173,7 @@ def get(workspace: Workspace, task_id: str) -> Optional[Task]: cursor.execute( """ - SELECT id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, is_leaf, hierarchical_id, requirement_ids + SELECT id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, is_leaf, hierarchical_id, requirement_ids, external_url, auto_close_github_issue FROM tasks WHERE workspace_id = ? AND id = ? """, @@ -174,6 +188,90 @@ def get(workspace: Workspace, task_id: str) -> Optional[Task]: return _row_to_task(row) +def get_by_external_url( + workspace: Workspace, external_url: str +) -> Optional[Task]: + """Get a task previously imported from a given external (issue) URL. + + Used for duplicate-import protection (issue #565). Keying on the full issue + URL — not just the issue number — keeps de-duplication correct when a + workspace is reconnected to a different repository (where the same issue + number refers to a different issue). + + Args: + workspace: Workspace to query + external_url: The issue's ``html_url`` to look up + + Returns: + The matching Task if one exists, otherwise None + """ + conn = get_db_connection(workspace) + try: + cursor = conn.cursor() + cursor.execute( + """ + SELECT id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, is_leaf, hierarchical_id, requirement_ids, external_url, auto_close_github_issue + FROM tasks + WHERE workspace_id = ? AND external_url = ? + LIMIT 1 + """, + (workspace.id, external_url), + ) + row = cursor.fetchone() + finally: + conn.close() + + if not row: + return None + + return _row_to_task(row) + + +def update_auto_close( + workspace: Workspace, + task_id: str, + auto_close: bool, +) -> Task: + """Update whether the linked GitHub issue should close when the task is DONE. + + Args: + workspace: Target workspace + task_id: Task to update + auto_close: New auto-close setting + + Returns: + Updated Task + + Raises: + ValueError: If task not found + """ + task = get(workspace, task_id) + if not task: + raise ValueError(f"Task not found: {task_id}") + + now = _utc_now().isoformat() + + conn = get_db_connection(workspace) + try: + cursor = conn.cursor() + cursor.execute( + """ + UPDATE tasks + SET auto_close_github_issue = ?, updated_at = ? + WHERE workspace_id = ? AND id = ? + """, + (1 if auto_close else 0, now, workspace.id, task_id), + ) + conn.commit() + finally: + conn.close() + + task.auto_close_github_issue = auto_close + task.updated_at = datetime.fromisoformat(now) + + return task + + def list_tasks( workspace: Workspace, status: Optional[TaskStatus] = None, @@ -195,7 +293,7 @@ def list_tasks( if status: cursor.execute( """ - SELECT id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, is_leaf, hierarchical_id, requirement_ids + SELECT id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, is_leaf, hierarchical_id, requirement_ids, external_url, auto_close_github_issue FROM tasks WHERE workspace_id = ? AND status = ? ORDER BY priority ASC, created_at ASC @@ -206,7 +304,7 @@ def list_tasks( else: cursor.execute( """ - SELECT id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, is_leaf, hierarchical_id, requirement_ids + SELECT id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, is_leaf, hierarchical_id, requirement_ids, external_url, auto_close_github_issue FROM tasks WHERE workspace_id = ? ORDER BY priority ASC, created_at ASC @@ -287,9 +385,136 @@ def update_status( task.status = new_status task.updated_at = datetime.fromisoformat(now) + # On completion, best-effort close the linked GitHub issue when opted in + # (issue #565). Placed here — the single chokepoint every DONE transition + # flows through (HTTP, CLI, agent/batch via runtime.complete_run) — so the + # behavior is consistent regardless of how the task was completed. + if new_status == TaskStatus.DONE: + _dispatch_github_autoclose(workspace, task) + return task +def _repo_from_issue_url(url: Optional[str]) -> Optional[str]: + """Extract ``owner/repo`` from a GitHub issue ``html_url``. + + e.g. ``https://github.com/acme/app/issues/12`` -> ``"acme/app"``. Returns + ``None`` when the URL is missing or not in the expected issue-URL shape. + """ + if not url: + return None + try: + parts = urlparse(url).path.strip("/").split("/") + except (ValueError, AttributeError): + return None + # .../{owner}/{repo}/issues/{number} + if len(parts) >= 4 and parts[-2] == "issues": + return f"{parts[-4]}/{parts[-3]}" + return None + + +def autoclose_if_done(workspace: Workspace, task: Task) -> None: + """Fire the GitHub auto-close when opting in on an already-DONE task (#565). + + The normal path closes the issue on the DONE *transition*. But a user can + enable the auto-close toggle on a task that is already DONE — in which case + no transition occurs, so this is called explicitly to honor the intent. + Best-effort and fully guarded (no-op unless DONE + opted in + linked). + """ + if task.status == TaskStatus.DONE: + _dispatch_github_autoclose(workspace, task) + + +def _dispatch_github_autoclose(workspace: Workspace, task: Task) -> None: + """Best-effort close of the linked GitHub issue when a task is DONE (#565). + + Mirrors the outbound-webhook dispatch pattern (``blockers._dispatch_*``): + fully guarded so a missing connection or any GitHub error never affects the + task transition. The repo is taken from the task's own ``external_url`` (its + source repo) — NOT the workspace's current connection — so completing an + older imported task always closes the right issue even after the workspace + is reconnected to a different repository. The PAT comes from the machine-wide + credential store. + """ + if not task.auto_close_github_issue or task.github_issue_number is None: + return + repo = _repo_from_issue_url(task.external_url) + if repo is None: + logger.info( + "Skipping GitHub auto-close for issue #%s: no source repo on task.", + task.github_issue_number, + ) + return + try: + from codeframe.core.credentials import CredentialManager, CredentialProvider + + pat = CredentialManager().get_credential(CredentialProvider.GIT_GITHUB) + if not pat: + logger.info( + "Skipping GitHub auto-close for issue #%s: no stored PAT.", + task.github_issue_number, + ) + return + _close_issue_background(pat, repo, task.github_issue_number) + except Exception: # noqa: BLE001 - must never break the task transition + logger.warning( + "Failed to dispatch GitHub auto-close for issue #%s", + task.github_issue_number, + exc_info=True, + ) + + +# Bounded timeout for the auto-close call so a hung GitHub request never stalls +# a short-lived CLI process for long at exit. +_AUTOCLOSE_TIMEOUT = 10.0 + + +async def _safe_close_issue(pat: str, repo: str, issue_number: int) -> None: + """Close the issue, swallowing every error (best-effort, off the hot path).""" + try: + from codeframe.core.github_issues_service import close_issue + + await close_issue( + pat, + repo, + issue_number, + comment="Completed via CodeFRAME", + timeout=_AUTOCLOSE_TIMEOUT, + ) + except Exception: # noqa: BLE001 - background best-effort + logger.warning( + "GitHub auto-close of issue #%s failed", issue_number, exc_info=True + ) + + +def _close_issue_background(pat: str, repo: str, issue_number: int) -> None: + """Run the auto-close off the caller's path (#565). + + Context-aware, mirroring ``WebhookNotificationService.send_event_background``: + + * **Async (server)**: there is a running event loop (the FastAPI handler), + so schedule the close on it and return immediately. No thread is created, + so nothing is left to join at process shutdown. + * **Sync (CLI / agent worker thread)**: no running loop, so run the close to + completion on a **non-daemon** thread. Unlike a fire-and-forget + notification, leaving the issue open is a real failure, so a short-lived + CLI process waits for the close at interpreter exit (bounded by + ``_AUTOCLOSE_TIMEOUT``) instead of abandoning it. + """ + try: + loop = asyncio.get_running_loop() + except RuntimeError: + threading.Thread( + target=lambda: asyncio.run( + _safe_close_issue(pat, repo, issue_number) + ), + daemon=False, + name=f"gh-autoclose-{issue_number}", + ).start() + else: + loop.create_task(_safe_close_issue(pat, repo, issue_number)) + + def update( workspace: Workspace, task_id: str, @@ -753,7 +978,8 @@ def _row_to_task(row: tuple) -> Task: Row columns: id, workspace_id, prd_id, title, description, status, priority, depends_on, estimated_hours, complexity_score, uncertainty_level, created_at, updated_at, github_issue_number, parent_id, lineage, - is_leaf, hierarchical_id, requirement_ids + is_leaf, hierarchical_id, requirement_ids, external_url, + auto_close_github_issue """ # Parse depends_on from JSON string (default to empty list if null) depends_on_raw = row[7] @@ -791,4 +1017,6 @@ def _row_to_task(row: tuple) -> Task: is_leaf=is_leaf, hierarchical_id=row[17] if len(row) > 17 else None, requirement_ids=requirement_ids, + external_url=row[19] if len(row) > 19 else None, + auto_close_github_issue=bool(row[20]) if len(row) > 20 and row[20] is not None else False, ) diff --git a/codeframe/core/workspace.py b/codeframe/core/workspace.py index e202f9d1..adf372aa 100644 --- a/codeframe/core/workspace.py +++ b/codeframe/core/workspace.py @@ -154,6 +154,10 @@ def _init_database(db_path: Path) -> None: cursor.execute("ALTER TABLE tasks ADD COLUMN hierarchical_id TEXT") if "requirement_ids" not in columns: cursor.execute("ALTER TABLE tasks ADD COLUMN requirement_ids TEXT DEFAULT '[]'") + if "external_url" not in columns: + cursor.execute("ALTER TABLE tasks ADD COLUMN external_url TEXT") + if "auto_close_github_issue" not in columns: + cursor.execute("ALTER TABLE tasks ADD COLUMN auto_close_github_issue INTEGER DEFAULT 0") # Append-only event log cursor.execute(""" @@ -366,6 +370,13 @@ def _init_database(db_path: Path) -> None: # Create indexes for common queries cursor.execute("CREATE INDEX IF NOT EXISTS idx_tasks_workspace ON tasks(workspace_id)") cursor.execute("CREATE INDEX IF NOT EXISTS idx_tasks_status ON tasks(status)") + # Atomic duplicate-import protection (#565): one task per (workspace, issue + # URL). SQLite treats NULLs as distinct, so non-imported tasks (NULL + # external_url) are unaffected. + cursor.execute( + "CREATE UNIQUE INDEX IF NOT EXISTS idx_tasks_external_url " + "ON tasks(workspace_id, external_url)" + ) cursor.execute("CREATE INDEX IF NOT EXISTS idx_events_workspace ON events(workspace_id)") cursor.execute("CREATE INDEX IF NOT EXISTS idx_blockers_workspace ON blockers(workspace_id)") cursor.execute("CREATE INDEX IF NOT EXISTS idx_blockers_status ON blockers(status)") @@ -522,6 +533,20 @@ def _ensure_schema_upgrades(db_path: Path) -> None: if "github_issue_number" not in task_columns: cursor.execute("ALTER TABLE tasks ADD COLUMN github_issue_number INTEGER") conn.commit() + if "external_url" not in task_columns: + cursor.execute("ALTER TABLE tasks ADD COLUMN external_url TEXT") + conn.commit() + if "auto_close_github_issue" not in task_columns: + cursor.execute( + "ALTER TABLE tasks ADD COLUMN auto_close_github_issue INTEGER DEFAULT 0" + ) + conn.commit() + # Atomic duplicate-import protection (#565) for existing workspaces. + cursor.execute( + "CREATE UNIQUE INDEX IF NOT EXISTS idx_tasks_external_url " + "ON tasks(workspace_id, external_url)" + ) + conn.commit() # Ensure runs table exists before creating dependent tables (run_logs, diagnostic_reports) cursor.execute( diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index 1bf52752..d2e62b2e 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -14,6 +14,7 @@ """ import logging +import sqlite3 import time from typing import Any, Optional @@ -29,12 +30,18 @@ parse_repo, validate_connection, ) -from codeframe.core.github_issues_service import list_issues +from codeframe.core.github_issues_service import ( + IssueNotFoundError, + NotAnIssueError, + get_issue, + list_issues, +) from codeframe.core.github_integration_config import ( clear_github_integration_config, load_github_integration_config, save_github_integration_config, ) +from codeframe.core import tasks from codeframe.core.workspace import Workspace from codeframe.lib.rate_limiter import rate_limit_ai, rate_limit_standard from codeframe.ui.dependencies import get_v2_workspace @@ -88,6 +95,24 @@ class GitHubIssuesResponse(BaseModel): per_page: int +class ImportRequest(BaseModel): + issue_numbers: list[int] = Field( + ..., min_length=1, description="GitHub issue numbers to import as tasks" + ) + + +class ImportedTaskSummary(BaseModel): + task_id: str + issue_number: int + title: str + + +class ImportResponse(BaseModel): + created: list[ImportedTaskSummary] + skipped: list[int] + total_created: int + + # In-process TTL cache for issue listings (#564). Keyed by the full query # (repo + page + per_page + search + label); entries expire after 60s to avoid # hammering GitHub's rate limit on repeated browses. Module-level so it is @@ -111,6 +136,17 @@ def _issue_cache_set(key: str, payload: Any) -> None: _ISSUE_CACHE[key] = (time.monotonic() + _ISSUE_CACHE_TTL_SECONDS, payload) +def _issue_cache_invalidate(repo: str) -> None: + """Drop all cached issue listings for ``repo``. + + Called after an import so reopening the browse modal doesn't keep offering + just-imported issues as selectable (they would now be skipped as dupes). + """ + prefix = f"{repo}|" + for key in [k for k in _ISSUE_CACHE if k.startswith(prefix)]: + _ISSUE_CACHE.pop(key, None) + + @router.get("/status", response_model=StatusResponse) @rate_limit_standard() async def get_status( @@ -338,3 +374,159 @@ async def get_issues( ) _issue_cache_set(cache_key, response) return response + + +def _require_connection( + workspace: Workspace, manager: CredentialManager +) -> tuple[str, str]: + """Return ``(repo, pat)`` for a connected workspace, or raise 409. + + Mirrors the not-connected guard used by ``get_issues``: both per-workspace + repo metadata AND a stored PAT must be present. + """ + cfg = load_github_integration_config(workspace) + pat = manager.get_credential(CredentialProvider.GIT_GITHUB) + if cfg is None or not pat: + raise HTTPException( + status_code=409, + detail=api_error( + "No GitHub repository is connected. Connect one in Settings → " + "Integrations first.", + ErrorCodes.CONFLICT, + ), + ) + return cfg["repo"], pat + + +def _map_github_error(e: Exception) -> HTTPException: + """Map a typed GitHub service error to an HTTPException (shared mapping).""" + if isinstance(e, InvalidTokenError): + return HTTPException( + status_code=401, detail=api_error(str(e), ErrorCodes.VALIDATION_ERROR) + ) + if isinstance(e, InsufficientScopeError): + return HTTPException( + status_code=403, detail=api_error(str(e), ErrorCodes.VALIDATION_ERROR) + ) + return HTTPException( + status_code=502, detail=api_error(str(e), ErrorCodes.EXECUTION_FAILED) + ) + + +@router.post("/import", response_model=ImportResponse) +@rate_limit_ai() +async def import_issues( + request: Request, + body: ImportRequest, + workspace: Workspace = Depends(get_v2_workspace), + manager: CredentialManager = Depends(get_credential_manager), +) -> ImportResponse: + """Import selected GitHub issues as CodeFRAME tasks (issue #565). + + Each issue becomes a task (title + body, with a best-effort ``Labels:`` + footer) linked back to the issue via ``github_issue_number`` + ``external_url``. + Issues already imported into this workspace are skipped (duplicate-import + protection), keyed on the full issue URL so the same number in a different + repo is still importable. + + The import is all-or-nothing on fetch: every selected issue is fetched and + de-duplicated *before* any task is created, so a single stale/inaccessible + issue fails the request cleanly without leaving a partial set of tasks + behind (which would confuse a retry with phantom duplicates). + """ + repo, pat = _require_connection(workspace, manager) + + skipped: list[int] = [] + to_create: list[tuple[int, dict]] = [] + seen_urls: set[str] = set() + + # Phase 1 — fetch + de-dupe everything first. Any fetch error aborts here, + # before a single task has been created. + for number in body.issue_numbers: + try: + issue = await get_issue(pat, repo, number) + except ValueError as e: + # Malformed saved repo slug (parse_repo) — surface as a recoverable + # conflict like the browse endpoint, not a 500. + raise HTTPException( + status_code=409, + detail=api_error(str(e), ErrorCodes.CONFLICT), + ) + except NotAnIssueError as e: + # A PR number slipped in (the browse UI filters PRs, so this only + # happens with a malformed/manual payload). Reject the request. + raise HTTPException( + status_code=422, + detail=api_error(str(e), ErrorCodes.VALIDATION_ERROR), + ) + except IssueNotFoundError as e: + # A stale/typo'd issue number — a client error, not a 502. + raise HTTPException( + status_code=404, + detail=api_error(str(e), ErrorCodes.NOT_FOUND), + ) + except GitHubConnectError as e: + raise _map_github_error(e) + + url = issue["html_url"] + # Skip both already-imported issues and in-payload duplicates (the same + # number repeated in one request must not create two tasks). + if url in seen_urls or tasks.get_by_external_url(workspace, url) is not None: + skipped.append(number) + continue + seen_urls.add(url) + to_create.append((number, issue)) + + # Phase 2 — create the tasks. Each create commits independently, so an + # unexpected mid-loop DB error (e.g. OperationalError on a locked DB) is + # rolled back below to preserve the all-or-nothing contract. (The + # URL-unique index additionally makes a retry idempotent.) + created: list[ImportedTaskSummary] = [] + created_ids: list[str] = [] + try: + for number, issue in to_create: + description = issue["body"] or "" + if issue["labels"]: + footer = "**Labels:** " + ", ".join(issue["labels"]) + description = f"{description}\n\n{footer}" if description else footer + + try: + task = tasks.create( + workspace, + title=issue["title"], + description=description, + github_issue_number=number, + external_url=issue["html_url"], + ) + except sqlite3.IntegrityError: + # A concurrent import (double-submit / second tab) created this + # task between our de-dupe read and now. The unique index on + # (workspace_id, external_url) makes that race a no-op: treat it + # as an already-imported skip rather than a duplicate task. + skipped.append(number) + continue + + created_ids.append(task.id) + created.append( + ImportedTaskSummary( + task_id=task.id, issue_number=number, title=task.title + ) + ) + except Exception: + # Roll back the tasks created so far so a mid-create failure never leaves + # a partial import behind. + for task_id in created_ids: + try: + tasks.delete(workspace, task_id) + except Exception: # noqa: BLE001 - best-effort cleanup + logger.warning("Failed to roll back imported task %s", task_id) + raise + + # Invalidate the browse cache so a re-open reflects current duplicate state. + # Always — even a skipped-only import (issues created by another tab/process) + # must drop the stale listing that still offers them as selectable. + _issue_cache_invalidate(repo) + + return ImportResponse( + created=created, skipped=skipped, total_created=len(created) + ) diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index de97e41b..8a35461e 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -26,7 +26,11 @@ from codeframe.lib.rate_limiter import rate_limit_ai, rate_limit_standard from codeframe.core import runtime, tasks, conductor, streaming from codeframe.core.runtime import RunStatus -from codeframe.core.state_machine import TaskStatus +from codeframe.core.state_machine import ( + InvalidTransitionError, + TaskStatus, + can_transition, +) from codeframe.ui.dependencies import get_v2_workspace from codeframe.ui.response_models import api_error, ErrorCodes @@ -145,6 +149,29 @@ class TaskResponse(BaseModel): estimated_hours: Optional[float] = None created_at: Optional[str] = None updated_at: Optional[str] = None + # GitHub issue traceability (#565) + github_issue_number: Optional[int] = None + external_url: Optional[str] = None + auto_close_github_issue: bool = False + + @classmethod + def from_task(cls, task: tasks.Task) -> "TaskResponse": + """Build a response from a core ``Task`` (single source of mapping).""" + return cls( + id=task.id, + title=task.title, + description=task.description, + status=task.status.value, + priority=task.priority, + depends_on=task.depends_on, + requirement_ids=task.requirement_ids, + estimated_hours=task.estimated_hours, + created_at=task.created_at.isoformat() if task.created_at else None, + updated_at=task.updated_at.isoformat() if task.updated_at else None, + github_issue_number=task.github_issue_number, + external_url=task.external_url, + auto_close_github_issue=task.auto_close_github_issue, + ) class TaskListResponse(BaseModel): @@ -162,6 +189,10 @@ class UpdateTaskRequest(BaseModel): description: Optional[str] = Field(None, description="New task description") priority: Optional[int] = Field(None, ge=0, description="New task priority (0 = highest)") status: Optional[str] = Field(None, description="New task status (use for manual transitions)") + auto_close_github_issue: Optional[bool] = Field( + None, + description="Whether to close the linked GitHub issue when this task is DONE (#565)", + ) # ============================================================================ @@ -209,21 +240,7 @@ async def list_tasks( status_counts = tasks.count_by_status(workspace) return TaskListResponse( - tasks=[ - TaskResponse( - id=t.id, - title=t.title, - description=t.description, - status=t.status.value, - priority=t.priority, - depends_on=t.depends_on, - requirement_ids=t.requirement_ids, - estimated_hours=t.estimated_hours, - created_at=t.created_at.isoformat() if t.created_at else None, - updated_at=t.updated_at.isoformat() if t.updated_at else None, - ) - for t in task_list - ], + tasks=[TaskResponse.from_task(t) for t in task_list], total=len(task_list), by_status=status_counts, ) @@ -255,18 +272,7 @@ async def get_task( detail=api_error("Task not found", ErrorCodes.NOT_FOUND, f"No task with id {task_id}"), ) - return TaskResponse( - id=task.id, - title=task.title, - description=task.description, - status=task.status.value, - priority=task.priority, - depends_on=task.depends_on, - requirement_ids=task.requirement_ids, - estimated_hours=task.estimated_hours, - created_at=task.created_at.isoformat() if task.created_at else None, - updated_at=task.updated_at.isoformat() if task.updated_at else None, - ) + return TaskResponse.from_task(task) @router.patch("/{task_id}", response_model=TaskResponse) @@ -296,28 +302,91 @@ async def update_task( - 400: Invalid status or status transition """ try: - # Handle status update separately if provided + # Validate everything that can reject the request BEFORE any mutation, so + # a rejected PATCH (bad status, illegal transition, missing task) never + # leaves a side effect. (#565) + current = None + if body.status or body.auto_close_github_issue is not None: + current = tasks.get(workspace, task_id) + if current is None: + raise HTTPException( + status_code=404, + detail=api_error( + "Task not found", + ErrorCodes.NOT_FOUND, + f"No task with id {task_id}", + ), + ) + + new_status: Optional[TaskStatus] = None if body.status: try: new_status = TaskStatus(body.status.upper()) - tasks.update_status(workspace, task_id, new_status) - except ValueError as e: - if "Invalid status" in str(e) or "not a valid" in str(e).lower(): - raise HTTPException( - status_code=400, - detail=api_error( - f"Invalid status: {body.status}", - ErrorCodes.VALIDATION_ERROR, - f"Valid values: {[s.value for s in TaskStatus]}", - ), - ) - # Status transition error + except ValueError: raise HTTPException( status_code=400, - detail=api_error("Invalid status transition", ErrorCodes.INVALID_STATE, str(e)), + detail=api_error( + f"Invalid status: {body.status}", + ErrorCodes.VALIDATION_ERROR, + f"Valid values: {[s.value for s in TaskStatus]}", + ), ) + if not can_transition(current.status, new_status): + raise HTTPException( + status_code=400, + detail=api_error( + "Invalid status transition", + ErrorCodes.INVALID_STATE, + f"Cannot transition from {current.status.value} to " + f"{new_status.value}", + ), + ) + + # Persist the auto-close preference FIRST so the DONE transition below + # sees the value requested in THIS PATCH. That makes a combined request + # behave correctly both ways: {status: DONE, opt-in} closes the issue and + # {status: DONE, opt-OUT} does not — core reads the freshly-saved flag. + original_auto_close = ( + current.auto_close_github_issue + if (current is not None and body.auto_close_github_issue is not None) + else None + ) + if body.auto_close_github_issue is not None: + tasks.update_auto_close( + workspace, task_id, body.auto_close_github_issue + ) + + # Apply the (pre-validated) status transition. Core closes the linked + # issue here when the task is opted in and transitions to DONE. + if new_status is not None: + try: + tasks.update_status(workspace, task_id, new_status) + except InvalidTransitionError: + # Pre-validation passed, so the task state changed concurrently + # between the check and the apply. Roll back the flag we just + # persisted so this failed request leaves no side effect. + if original_auto_close is not None: + tasks.update_auto_close(workspace, task_id, original_auto_close) + raise HTTPException( + status_code=409, + detail=api_error( + "Task state changed concurrently; please retry.", + ErrorCodes.CONFLICT, + ), + ) + + # Late opt-in: enabling the flag (false -> true) on a task that is ALREADY + # DONE — with no status transition in this request — won't trigger core's + # transition-based close, so fire it explicitly here. + if ( + body.auto_close_github_issue + and original_auto_close is False + and new_status is None + ): + tasks.autoclose_if_done(workspace, tasks.get(workspace, task_id)) - # Update other fields + # Update remaining fields; re-reads current state so the returned task + # reflects every change (including the auto-close flag above). task = tasks.update( workspace, task_id, @@ -326,18 +395,7 @@ async def update_task( priority=body.priority, ) - return TaskResponse( - id=task.id, - title=task.title, - description=task.description, - status=task.status.value, - priority=task.priority, - depends_on=task.depends_on, - requirement_ids=task.requirement_ids, - estimated_hours=task.estimated_hours, - created_at=task.created_at.isoformat() if task.created_at else None, - updated_at=task.updated_at.isoformat() if task.updated_at else None, - ) + return TaskResponse.from_task(task) except ValueError as e: error_msg = str(e) diff --git a/tests/conftest.py b/tests/conftest.py index 691daa9e..3bb5a159 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,17 @@ from typing import Generator import pytest +# Disable the global default rate limiter for the whole test suite. The +# @rate_limit_* decorators bind the limiter at ROUTER IMPORT time and the limits +# library keeps counters in a process-shared in-memory store, so the fixed +# ``ip:testclient`` bucket is shared across every test and exhausts in the full +# suite (notably the AI bucket: connect/import are 20/min) — causing spurious +# 429s. Setting this BEFORE any router is imported makes every route import +# unwrapped. Tests that specifically exercise rate limiting construct their own +# explicit Limiter, so they are unaffected; ``setdefault`` lets an explicit +# environment override win. +os.environ.setdefault("RATE_LIMIT_ENABLED", "false") + # All v1 legacy tests have been removed; nothing to ignore at the root. collect_ignore: list[str] = [] diff --git a/tests/core/test_github_issues_service.py b/tests/core/test_github_issues_service.py index 8265d207..dc56cb1e 100644 --- a/tests/core/test_github_issues_service.py +++ b/tests/core/test_github_issues_service.py @@ -223,3 +223,227 @@ def handler(request: httpx.Request) -> httpx.Response: ) assert issues == [] assert total == 0 + + +# ───────────────────────────────────────────────────────────────────────────── +# get_issue / close_issue (issue #565 — import execution + auto-close) +# ───────────────────────────────────────────────────────────────────────────── + +from codeframe.core.github_connect_service import GitHubConnectError # noqa: E402 +from codeframe.core.github_issues_service import ( # noqa: E402 + IssueNotFoundError, + NotAnIssueError, + close_issue, + get_issue, +) + + +class TestGetIssue: + @pytest.mark.asyncio + async def test_rejects_pull_requests(self): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "number": 50, + "title": "A PR", + "body": "diff", + "labels": [], + "html_url": "https://github.com/acme/app/pull/50", + "pull_request": {"url": "https://api.github.com/.../pulls/50"}, + }, + ) + + async with _client(handler) as client: + with pytest.raises(NotAnIssueError): + await get_issue(VALID_PAT, "acme/app", 50, client=client) + + @pytest.mark.asyncio + async def test_returns_issue_fields(self): + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "GET" + assert request.url.path == "/repos/acme/app/issues/42" + return httpx.Response( + 200, + json={ + "number": 42, + "title": "Fix login bug", + "body": "Steps to reproduce...", + "labels": [{"name": "bug"}, {"name": "auth"}], + "html_url": "https://github.com/acme/app/issues/42", + }, + ) + + async with _client(handler) as client: + issue = await get_issue(VALID_PAT, "acme/app", 42, client=client) + assert issue["number"] == 42 + assert issue["title"] == "Fix login bug" + assert issue["body"] == "Steps to reproduce..." + assert issue["labels"] == ["bug", "auth"] + assert issue["html_url"] == "https://github.com/acme/app/issues/42" + + @pytest.mark.asyncio + async def test_handles_null_body(self): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "number": 7, + "title": "No body issue", + "body": None, + "labels": [], + "html_url": "https://github.com/acme/app/issues/7", + }, + ) + + async with _client(handler) as client: + issue = await get_issue(VALID_PAT, "acme/app", 7, client=client) + assert issue["body"] == "" + assert issue["labels"] == [] + + @pytest.mark.asyncio + async def test_401_maps_to_invalid_token(self): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(401, json={"message": "Bad credentials"}) + + async with _client(handler) as client: + with pytest.raises(InvalidTokenError): + await get_issue(VALID_PAT, "acme/app", 1, client=client) + + @pytest.mark.asyncio + async def test_404_with_reachable_repo_maps_to_issue_not_found(self): + """Issue 404 + repo reachable → the issue genuinely doesn't exist.""" + + def handler(request: httpx.Request) -> httpx.Response: + # The repo probe (GET /repos/acme/app) succeeds; the issue 404s. + if request.url.path == "/repos/acme/app": + return httpx.Response(200, json={"full_name": "acme/app"}) + return httpx.Response(404, json={"message": "Not Found"}) + + async with _client(handler) as client: + with pytest.raises(IssueNotFoundError): + await get_issue(VALID_PAT, "acme/app", 999, client=client) + + @pytest.mark.asyncio + async def test_404_with_unreachable_repo_maps_to_connect_error(self): + """Issue 404 + repo also 404 → broken integration, not a missing issue.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(404, json={"message": "Not Found"}) + + async with _client(handler) as client: + with pytest.raises(GitHubConnectError): + await get_issue(VALID_PAT, "acme/app", 999, client=client) + + @pytest.mark.asyncio + async def test_404_with_degraded_repo_probe_maps_to_connect_error(self): + """Issue 404 + repo probe 5xx/429 → upstream error, not 'issue missing'.""" + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/acme/app": + return httpx.Response(503, json={"message": "Service Unavailable"}) + return httpx.Response(404, json={"message": "Not Found"}) + + async with _client(handler) as client: + with pytest.raises(GitHubConnectError): + await get_issue(VALID_PAT, "acme/app", 999, client=client) + + @pytest.mark.asyncio + async def test_404_with_revoked_token_maps_to_invalid_token(self): + """Issue 404 + repo probe 401 → token problem, surfaced as such.""" + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/repos/acme/app": + return httpx.Response(401, json={"message": "Bad credentials"}) + return httpx.Response(404, json={"message": "Not Found"}) + + async with _client(handler) as client: + with pytest.raises(InvalidTokenError): + await get_issue(VALID_PAT, "acme/app", 999, client=client) + + +class TestCloseIssue: + @pytest.mark.asyncio + async def test_patches_state_closed(self): + calls = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append((request.method, request.url.path)) + return httpx.Response(200, json={"number": 42, "state": "closed"}) + + async with _client(handler) as client: + ok = await close_issue(VALID_PAT, "acme/app", 42, client=client) + assert ok is True + assert ("PATCH", "/repos/acme/app/issues/42") in calls + + @pytest.mark.asyncio + async def test_posts_comment_then_closes_when_comment_given(self): + calls = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append((request.method, request.url.path)) + if request.url.path.endswith("/comments"): + return httpx.Response(201, json={"id": 1}) + return httpx.Response(200, json={"number": 42, "state": "closed"}) + + async with _client(handler) as client: + ok = await close_issue( + VALID_PAT, "acme/app", 42, comment="Completed via CodeFRAME", client=client + ) + assert ok is True + assert ("POST", "/repos/acme/app/issues/42/comments") in calls + assert ("PATCH", "/repos/acme/app/issues/42") in calls + # Comment must be posted before the close patch. + assert calls.index(("POST", "/repos/acme/app/issues/42/comments")) < calls.index( + ("PATCH", "/repos/acme/app/issues/42") + ) + + @pytest.mark.asyncio + async def test_closes_even_when_comment_fails(self): + """A failed completion comment must not prevent the close itself.""" + calls = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append((request.method, request.url.path)) + if request.url.path.endswith("/comments"): + return httpx.Response(403, json={"message": "locked"}) + return httpx.Response(200, json={"number": 42, "state": "closed"}) + + async with _client(handler) as client: + ok = await close_issue( + VALID_PAT, "acme/app", 42, comment="hi", client=client + ) + assert ok is True + assert ("PATCH", "/repos/acme/app/issues/42") in calls + + @pytest.mark.asyncio + async def test_no_comment_skips_comment_call(self): + calls = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append((request.method, request.url.path)) + return httpx.Response(200, json={"number": 42, "state": "closed"}) + + async with _client(handler) as client: + await close_issue(VALID_PAT, "acme/app", 42, client=client) + assert not any(p.endswith("/comments") for _, p in calls) + + @pytest.mark.asyncio + async def test_401_maps_to_invalid_token(self): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(401, json={"message": "Bad credentials"}) + + async with _client(handler) as client: + with pytest.raises(InvalidTokenError): + await close_issue(VALID_PAT, "acme/app", 1, client=client) + + @pytest.mark.asyncio + async def test_redirect_is_not_treated_as_success(self): + """A 3xx (moved/renamed repo) means the close was not applied.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(301, headers={"Location": "/repos/new/app/issues/1"}) + + async with _client(handler) as client: + with pytest.raises(GitHubConnectError): + await close_issue(VALID_PAT, "acme/app", 1, client=client) diff --git a/tests/core/test_task_github_traceability.py b/tests/core/test_task_github_traceability.py new file mode 100644 index 00000000..71b6f2df --- /dev/null +++ b/tests/core/test_task_github_traceability.py @@ -0,0 +1,255 @@ +"""Tests for GitHub-issue traceability fields on tasks (issue #565). + +Covers the new ``external_url`` and ``auto_close_github_issue`` columns, the +persistence of ``github_issue_number`` through ``create()``, and the two new +helpers ``get_by_github_issue_number`` and ``update_auto_close`` that the import +flow relies on (duplicate-import protection + auto-close toggle). +""" + +import pytest + +from codeframe.core import tasks +from codeframe.core.state_machine import TaskStatus +from codeframe.core.workspace import create_or_load_workspace + +pytestmark = pytest.mark.v2 + + +@pytest.fixture +def workspace(tmp_path): + """Create a test workspace.""" + return create_or_load_workspace(tmp_path) + + +class TestTraceabilityFields: + def test_defaults(self, workspace): + """A plain task has no GitHub linkage and auto-close is off.""" + task = tasks.create(workspace, title="Plain task") + assert task.github_issue_number is None + assert task.external_url is None + assert task.auto_close_github_issue is False + + def test_create_with_github_fields(self, workspace): + task = tasks.create( + workspace, + title="Imported task", + description="From GitHub", + github_issue_number=42, + external_url="https://github.com/acme/app/issues/42", + ) + assert task.github_issue_number == 42 + assert task.external_url == "https://github.com/acme/app/issues/42" + assert task.auto_close_github_issue is False + + def test_github_fields_persist_across_get(self, workspace): + created = tasks.create( + workspace, + title="Imported", + github_issue_number=7, + external_url="https://github.com/acme/app/issues/7", + ) + fetched = tasks.get(workspace, created.id) + assert fetched.github_issue_number == 7 + assert fetched.external_url == "https://github.com/acme/app/issues/7" + + def test_github_fields_present_in_list(self, workspace): + plain = tasks.create(workspace, title="Plain") + imported = tasks.create( + workspace, + title="Imported", + github_issue_number=99, + external_url="https://github.com/acme/app/issues/99", + ) + by_id = {t.id: t for t in tasks.list_tasks(workspace)} + assert by_id[plain.id].github_issue_number is None + assert by_id[imported.id].github_issue_number == 99 + assert by_id[imported.id].external_url.endswith("/issues/99") + + +class TestExternalUrlUniqueIndex: + def test_duplicate_external_url_rejected_by_db(self, workspace): + """A unique index enforces one task per (workspace, issue URL) (#565).""" + import sqlite3 + + url = "https://github.com/acme/app/issues/12" + tasks.create(workspace, title="First", external_url=url) + with pytest.raises(sqlite3.IntegrityError): + tasks.create(workspace, title="Dup", external_url=url) + + def test_multiple_null_external_urls_allowed(self, workspace): + """Non-imported tasks (NULL external_url) are unaffected by the index.""" + tasks.create(workspace, title="A") + tasks.create(workspace, title="B") + assert len(tasks.list_tasks(workspace)) == 2 + + +class TestGetByExternalUrl: + def test_returns_matching_task(self, workspace): + url = "https://github.com/acme/app/issues/123" + created = tasks.create( + workspace, title="Imported", github_issue_number=123, external_url=url + ) + found = tasks.get_by_external_url(workspace, url) + assert found is not None + assert found.id == created.id + + def test_returns_none_when_absent(self, workspace): + tasks.create(workspace, title="Plain") + assert ( + tasks.get_by_external_url( + workspace, "https://github.com/acme/app/issues/555" + ) + is None + ) + + def test_distinguishes_repos_with_same_issue_number(self, workspace): + """Same issue number, different repo URL → not a duplicate.""" + tasks.create( + workspace, + title="acme #12", + github_issue_number=12, + external_url="https://github.com/acme/app/issues/12", + ) + # A different repo's #12 must not be seen as already imported. + assert ( + tasks.get_by_external_url( + workspace, "https://github.com/other/app/issues/12" + ) + is None + ) + + def test_scoped_to_workspace(self, workspace, tmp_path): + """A different workspace must not see this workspace's imported issue.""" + url = "https://github.com/acme/app/issues/10" + tasks.create( + workspace, title="Imported", github_issue_number=10, external_url=url + ) + other_path = tmp_path / "other" + other_path.mkdir() + other = create_or_load_workspace(other_path) + assert tasks.get_by_external_url(other, url) is None + + +class TestAutoCloseDispatch: + """update_status fires the GitHub auto-close on DONE for all callers (#565).""" + + @staticmethod + def _record_calls(monkeypatch): + from codeframe.core import tasks as tasks_mod + + calls = [] + monkeypatch.setattr( + tasks_mod, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), + ) + return calls + + def test_done_dispatches_when_opted_in(self, workspace, monkeypatch): + calls = self._record_calls(monkeypatch) + # The dispatch resolves the PAT via CredentialManager.get_credential, + # which reads the GITHUB_TOKEN env var first (see credentials.py) — that + # is what makes the credential non-empty here so the close is dispatched. + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + + task = tasks.create( + workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + external_url="https://github.com/acme/app/issues/99", + auto_close_github_issue=True, + ) + tasks.update_status(workspace, task.id, TaskStatus.DONE) + assert calls == [("acme/app", 99)] + + def test_done_targets_source_repo_from_external_url(self, workspace, monkeypatch): + """The close targets the task's source repo, not the live connection.""" + calls = self._record_calls(monkeypatch) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + # Workspace is currently connected to a DIFFERENT repo than the task came + # from — the close must still target the task's original repo. + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + + save_github_integration_config( + workspace, + {"repo": "newowner/newrepo", "owner_login": "newowner", + "owner_avatar_url": ""}, + ) + task = tasks.create( + workspace, + title="From old repo", + status=TaskStatus.IN_PROGRESS, + github_issue_number=42, + external_url="https://github.com/acme/app/issues/42", + auto_close_github_issue=True, + ) + tasks.update_status(workspace, task.id, TaskStatus.DONE) + assert calls == [("acme/app", 42)] + + def test_done_does_not_dispatch_when_not_opted_in(self, workspace, monkeypatch): + calls = self._record_calls(monkeypatch) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + + task = tasks.create( + workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + external_url="https://github.com/acme/app/issues/99", + auto_close_github_issue=False, + ) + tasks.update_status(workspace, task.id, TaskStatus.DONE) + assert calls == [] + + def test_done_skips_when_no_source_repo(self, workspace, monkeypatch): + """No external_url → no parseable repo → nothing dispatched.""" + calls = self._record_calls(monkeypatch) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + task = tasks.create( + workspace, + title="Imported, no url", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + auto_close_github_issue=True, + ) + tasks.update_status(workspace, task.id, TaskStatus.DONE) + assert calls == [] + + def test_done_skips_when_no_pat(self, workspace, monkeypatch): + """A parseable repo but no stored PAT → nothing dispatched.""" + calls = self._record_calls(monkeypatch) + monkeypatch.setattr( + "codeframe.core.credentials.CredentialManager.get_credential", + lambda self, provider, name=None: None, + ) + task = tasks.create( + workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + external_url="https://github.com/acme/app/issues/99", + auto_close_github_issue=True, + ) + tasks.update_status(workspace, task.id, TaskStatus.DONE) + assert calls == [] + + +class TestUpdateAutoClose: + def test_toggle_on_and_off(self, workspace): + task = tasks.create(workspace, title="Imported", github_issue_number=5) + assert task.auto_close_github_issue is False + + updated = tasks.update_auto_close(workspace, task.id, True) + assert updated.auto_close_github_issue is True + assert tasks.get(workspace, task.id).auto_close_github_issue is True + + updated = tasks.update_auto_close(workspace, task.id, False) + assert updated.auto_close_github_issue is False + assert tasks.get(workspace, task.id).auto_close_github_issue is False + + def test_raises_for_missing_task(self, workspace): + with pytest.raises(ValueError, match="not found"): + tasks.update_auto_close(workspace, "does-not-exist", True) diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index f633f463..3015b35c 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -362,3 +362,332 @@ def test_pat_never_echoed(self, client, monkeypatch): _mock_list_issues(monkeypatch, result=([], 0)) r = client.get("/api/v2/integrations/github/issues") assert VALID_PAT not in r.text + + +# ───────────────────────────────────────────────────────────────────────────── +# Import execution + issue close (issue #565) +# ───────────────────────────────────────────────────────────────────────────── + + +def _mock_get_issue(monkeypatch, issues_by_number, *, exc=None): + """Patch get_issue on the router. ``issues_by_number`` maps number -> dict.""" + from codeframe.ui.routers import github_integrations_v2 + + async def fake(pat, repo, number, **kwargs): + if exc is not None: + raise exc + data = issues_by_number[number] + return { + "number": number, + "title": data["title"], + "body": data.get("body", ""), + "labels": data.get("labels", []), + "html_url": data.get( + "html_url", f"https://github.com/{repo}/issues/{number}" + ), + } + + monkeypatch.setattr(github_integrations_v2, "get_issue", fake) + + +class TestImport: + def test_requires_connection(self, client): + r = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [1]} + ) + assert r.status_code == 409 + + def test_imports_create_tasks(self, client, monkeypatch, workspace): + _connect(client, monkeypatch) + _mock_get_issue( + monkeypatch, + { + 12: {"title": "Fix login", "body": "Repro steps", "labels": ["bug"]}, + 34: {"title": "Dark mode", "body": "", "labels": []}, + }, + ) + r = client.post( + "/api/v2/integrations/github/import", + json={"issue_numbers": [12, 34]}, + ) + assert r.status_code == 200 + data = r.json() + assert data["total_created"] == 2 + assert data["skipped"] == [] + titles = {c["title"] for c in data["created"]} + assert titles == {"Fix login", "Dark mode"} + + # Tasks really exist in the workspace with the right linkage. + from codeframe.core import tasks + + t12 = tasks.get_by_external_url( + workspace, "https://github.com/acme/app/issues/12" + ) + assert t12 is not None + assert t12.title == "Fix login" + assert "Repro steps" in t12.description + assert t12.external_url == "https://github.com/acme/app/issues/12" + assert t12.github_issue_number == 12 + + def test_labels_appended_to_description(self, client, monkeypatch, workspace): + _connect(client, monkeypatch) + _mock_get_issue( + monkeypatch, + {5: {"title": "Tagged", "body": "Body text", "labels": ["bug", "ui"]}}, + ) + client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [5]} + ) + from codeframe.core import tasks + + t = tasks.get_by_external_url( + workspace, "https://github.com/acme/app/issues/5" + ) + assert "bug" in t.description and "ui" in t.description + + def test_duplicate_import_is_skipped(self, client, monkeypatch, workspace): + _connect(client, monkeypatch) + _mock_get_issue(monkeypatch, {7: {"title": "Once", "body": "x"}}) + first = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [7]} + ) + assert first.json()["total_created"] == 1 + + second = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [7]} + ) + body = second.json() + assert body["total_created"] == 0 + assert body["skipped"] == [7] + + # Only one task exists for issue 7. + from codeframe.core import tasks + + matching = [ + t for t in tasks.list_tasks(workspace) if t.github_issue_number == 7 + ] + assert len(matching) == 1 + + def test_repeated_number_in_payload_creates_one_task( + self, client, monkeypatch, workspace + ): + """The same issue number twice in one request must create one task.""" + _connect(client, monkeypatch) + _mock_get_issue(monkeypatch, {3: {"title": "Once", "body": "x"}}) + r = client.post( + "/api/v2/integrations/github/import", + json={"issue_numbers": [3, 3]}, + ) + body = r.json() + assert body["total_created"] == 1 + assert body["skipped"] == [3] + + from codeframe.core import tasks + + matching = [ + t for t in tasks.list_tasks(workspace) if t.github_issue_number == 3 + ] + assert len(matching) == 1 + + def test_pull_request_number_is_rejected(self, client, monkeypatch, workspace): + """A PR number is rejected (422) and creates no task.""" + _connect(client, monkeypatch) + from codeframe.core.github_issues_service import NotAnIssueError + from codeframe.ui.routers import github_integrations_v2 + + async def pr_get_issue(pat, repo, number, **kwargs): + raise NotAnIssueError(f"#{number} is a pull request, not an issue.") + + monkeypatch.setattr(github_integrations_v2, "get_issue", pr_get_issue) + r = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [50]} + ) + assert r.status_code == 422 + + from codeframe.core import tasks + + assert tasks.list_tasks(workspace) == [] + + def test_import_invalidates_issue_cache(self, client, monkeypatch, workspace): + """After import, the browse cache for the repo is cleared (#565).""" + _clear_issue_cache() + _connect(client, monkeypatch) + # Prime the issue-list cache via the browse endpoint. + _mock_list_issues(monkeypatch, result=([], 0)) + client.get("/api/v2/integrations/github/issues") + from codeframe.ui.routers import github_integrations_v2 + + assert len(github_integrations_v2._ISSUE_CACHE) > 0 + + _mock_get_issue(monkeypatch, {1: {"title": "One", "body": "x"}}) + r = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [1]} + ) + assert r.json()["total_created"] == 1 + # The repo's cached listings were dropped. + assert all( + not k.startswith("acme/app|") + for k in github_integrations_v2._ISSUE_CACHE + ) + + def test_skipped_only_import_also_invalidates_cache( + self, client, monkeypatch, workspace + ): + """Even an all-skipped import drops the stale browse cache (#565).""" + _clear_issue_cache() + _connect(client, monkeypatch) + # Pre-create the task so the import will skip it. + from codeframe.core import tasks + + tasks.create( + workspace, + title="Already here", + github_issue_number=1, + external_url="https://github.com/acme/app/issues/1", + ) + # Prime the browse cache. + _mock_list_issues(monkeypatch, result=([], 0)) + client.get("/api/v2/integrations/github/issues") + from codeframe.ui.routers import github_integrations_v2 + + assert len(github_integrations_v2._ISSUE_CACHE) > 0 + + _mock_get_issue(monkeypatch, {1: {"title": "Already here", "body": "x"}}) + r = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [1]} + ) + assert r.json()["total_created"] == 0 + assert r.json()["skipped"] == [1] + assert all( + not k.startswith("acme/app|") + for k in github_integrations_v2._ISSUE_CACHE + ) + + def test_malformed_saved_repo_maps_to_409(self, client, monkeypatch, workspace): + """A malformed stored repo slug surfaces as a 409, not a 500.""" + _connect(client, monkeypatch) + from codeframe.ui.routers import github_integrations_v2 + + async def bad_repo(pat, repo, number, **kwargs): + raise ValueError("Invalid repository format: 'acme'. Expected 'owner/repo'.") + + monkeypatch.setattr(github_integrations_v2, "get_issue", bad_repo) + r = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [1]} + ) + assert r.status_code == 409 + + def test_create_failure_rolls_back_partial_import( + self, client, monkeypatch, workspace + ): + """A mid-create DB error rolls back already-created tasks (all-or-nothing).""" + _connect(client, monkeypatch) + _mock_get_issue( + monkeypatch, + { + 1: {"title": "One", "body": "a"}, + 2: {"title": "Two", "body": "b"}, + 3: {"title": "Three", "body": "c"}, + }, + ) + + import sqlite3 as _sqlite3 + + from codeframe.core import tasks as tasks_mod + + real_create = tasks_mod.create + calls = {"n": 0} + + def flaky_create(*args, **kwargs): + calls["n"] += 1 + if calls["n"] == 3: # fail on the third create + raise _sqlite3.OperationalError("database is locked") + return real_create(*args, **kwargs) + + monkeypatch.setattr( + "codeframe.ui.routers.github_integrations_v2.tasks.create", flaky_create + ) + # The server raises a 500; TestClient re-raises it. The endpoint rolls + # back the already-created tasks before the error propagates. + with pytest.raises(_sqlite3.OperationalError): + client.post( + "/api/v2/integrations/github/import", + json={"issue_numbers": [1, 2, 3]}, + ) + # The two tasks created before the failure must have been rolled back. + assert tasks_mod.list_tasks(workspace) == [] + + def test_missing_issue_number_maps_to_404(self, client, monkeypatch, workspace): + """A stale/typo'd issue number is a client error (404), not a 502.""" + _connect(client, monkeypatch) + from codeframe.core.github_issues_service import IssueNotFoundError + from codeframe.ui.routers import github_integrations_v2 + + async def missing(pat, repo, number, **kwargs): + raise IssueNotFoundError(f"Issue #{number} was not found in '{repo}'.") + + monkeypatch.setattr(github_integrations_v2, "get_issue", missing) + r = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [9999]} + ) + assert r.status_code == 404 + from codeframe.core import tasks + + assert tasks.list_tasks(workspace) == [] + + def test_fetch_failure_creates_no_tasks(self, client, monkeypatch, workspace): + """A mid-batch fetch error aborts cleanly — no partial tasks created.""" + _connect(client, monkeypatch) + + from codeframe.core.github_connect_service import GitHubConnectError + from codeframe.ui.routers import github_integrations_v2 + + async def flaky_get_issue(pat, repo, number, **kwargs): + if number == 2: + raise GitHubConnectError("issue 2 is inaccessible") + return { + "number": number, + "title": f"Issue {number}", + "body": "x", + "labels": [], + "html_url": f"https://github.com/acme/app/issues/{number}", + } + + monkeypatch.setattr( + github_integrations_v2, "get_issue", flaky_get_issue + ) + r = client.post( + "/api/v2/integrations/github/import", + json={"issue_numbers": [1, 2, 3]}, + ) + assert r.status_code == 502 + # The earlier valid issue (#1) must NOT have been created. + from codeframe.core import tasks + + assert tasks.list_tasks(workspace) == [] + + def test_same_issue_number_different_repo_is_not_skipped( + self, client, monkeypatch, workspace + ): + """After reconnecting to another repo, the same issue number imports.""" + _connect(client, monkeypatch, repo="acme/app") + _mock_get_issue(monkeypatch, {12: {"title": "acme 12", "body": "x"}}) + client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [12]} + ) + + # Reconnect the same workspace to a different repo and import its #12. + _connect(client, monkeypatch, repo="other/app") + _mock_get_issue(monkeypatch, {12: {"title": "other 12", "body": "y"}}) + r = client.post( + "/api/v2/integrations/github/import", json={"issue_numbers": [12]} + ) + assert r.json()["total_created"] == 1 + assert r.json()["skipped"] == [] + + from codeframe.core import tasks + + titles = {t.title for t in tasks.list_tasks(workspace)} + assert {"acme 12", "other 12"} <= titles + + diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index 1ddd055c..57458c80 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1161,3 +1161,292 @@ def test_rate_limit_on_real_endpoint(self, rate_limited_client): data = response.json() assert "blockers" in data assert "total" in data + + +# ============================================================================ +# Tasks v2 — GitHub traceability + auto-close (issue #565) +# ============================================================================ + + +class TestTasksV2GitHubTraceability: + """TaskResponse exposes GitHub linkage; PATCH toggles auto-close; DONE closes.""" + + def test_response_includes_github_fields(self, test_client): + from codeframe.core import tasks + + task = tasks.create( + test_client.workspace, + title="Imported", + github_issue_number=42, + external_url="https://github.com/acme/app/issues/42", + ) + r = test_client.get(f"/api/v2/tasks/{task.id}") + assert r.status_code == 200 + data = r.json() + assert data["github_issue_number"] == 42 + assert data["external_url"] == "https://github.com/acme/app/issues/42" + assert data["auto_close_github_issue"] is False + + def test_plain_task_has_null_github_fields(self, test_client_with_task): + r = test_client_with_task.get(f"/api/v2/tasks/{test_client_with_task.task.id}") + data = r.json() + assert data["github_issue_number"] is None + assert data["external_url"] is None + assert data["auto_close_github_issue"] is False + + def test_patch_sets_auto_close(self, test_client): + from codeframe.core import tasks + + task = tasks.create( + test_client.workspace, title="Imported", github_issue_number=7 + ) + r = test_client.patch( + f"/api/v2/tasks/{task.id}", json={"auto_close_github_issue": True} + ) + assert r.status_code == 200 + assert r.json()["auto_close_github_issue"] is True + assert tasks.get(test_client.workspace, task.id).auto_close_github_issue is True + + def test_done_via_http_reaches_core_autoclose(self, test_client, monkeypatch): + """PATCH status=DONE flows through to the core auto-close dispatch (#565). + + The detailed dispatch behavior (connection resolution, opt-out, failure + isolation) is covered by the core unit tests; here we only assert the + HTTP DONE path reaches it. Auto-close lives in core so the CLI/agent + completion paths get the same behavior. + """ + from codeframe.core import tasks + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + from codeframe.core.state_machine import TaskStatus + + calls = [] + monkeypatch.setattr( + tasks, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + save_github_integration_config( + test_client.workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + ) + + task = tasks.create( + test_client.workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + external_url="https://github.com/acme/app/issues/99", + auto_close_github_issue=True, + ) + r = test_client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) + assert r.status_code == 200 + assert calls == [("acme/app", 99)] + assert tasks.get(test_client.workspace, task.id).status == TaskStatus.DONE + + def test_combined_done_and_optin_in_one_request_closes( + self, test_client, monkeypatch + ): + """A single PATCH setting both status=DONE and auto_close=true closes. + + The flag must be persisted before the DONE transition so the core + dispatch observes the opt-in (#565 — codex review). + """ + from codeframe.core import tasks + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + from codeframe.core.state_machine import TaskStatus + + calls = [] + monkeypatch.setattr( + tasks, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + save_github_integration_config( + test_client.workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + ) + + task = tasks.create( + test_client.workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=77, + external_url="https://github.com/acme/app/issues/77", + auto_close_github_issue=False, + ) + # Opt in AND complete in the same request. + r = test_client.patch( + f"/api/v2/tasks/{task.id}", + json={"status": "DONE", "auto_close_github_issue": True}, + ) + assert r.status_code == 200 + assert calls == [("acme/app", 77)] + + def test_optin_on_already_done_task_closes_now(self, test_client, monkeypatch): + """Enabling auto-close on an already-DONE task closes the issue now.""" + from codeframe.core import tasks + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + from codeframe.core.state_machine import TaskStatus + + calls = [] + monkeypatch.setattr( + tasks, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + save_github_integration_config( + test_client.workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + ) + + task = tasks.create( + test_client.workspace, + title="Already done", + status=TaskStatus.DONE, + github_issue_number=88, + external_url="https://github.com/acme/app/issues/88", + auto_close_github_issue=False, + ) + r = test_client.patch( + f"/api/v2/tasks/{task.id}", + json={"auto_close_github_issue": True}, + ) + assert r.status_code == 200 + assert calls == [("acme/app", 88)] + + def test_combined_done_and_optout_does_not_close(self, test_client, monkeypatch): + """A single PATCH that completes AND opts out must NOT close the issue.""" + from codeframe.core import tasks + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + from codeframe.core.state_machine import TaskStatus + + calls = [] + monkeypatch.setattr( + tasks, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + save_github_integration_config( + test_client.workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + ) + + task = tasks.create( + test_client.workspace, + title="Opted in", + status=TaskStatus.IN_PROGRESS, + github_issue_number=55, + external_url="https://github.com/acme/app/issues/55", + auto_close_github_issue=True, + ) + # Complete AND opt out in one request — the issue must stay open. + r = test_client.patch( + f"/api/v2/tasks/{task.id}", + json={"status": "DONE", "auto_close_github_issue": False}, + ) + assert r.status_code == 200 + assert calls == [] + after = tasks.get(test_client.workspace, task.id) + assert after.status == TaskStatus.DONE + assert after.auto_close_github_issue is False + + def test_reopen_with_optin_does_not_close(self, test_client, monkeypatch): + """A combined reopen (DONE->READY) + opt-in must NOT close the issue.""" + from codeframe.core import tasks + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + from codeframe.core.state_machine import TaskStatus + + calls = [] + monkeypatch.setattr( + tasks, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + save_github_integration_config( + test_client.workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + ) + + task = tasks.create( + test_client.workspace, + title="Done import", + status=TaskStatus.DONE, + github_issue_number=88, + external_url="https://github.com/acme/app/issues/88", + auto_close_github_issue=False, + ) + # Reopen AND opt in within one request — the issue must stay open. + r = test_client.patch( + f"/api/v2/tasks/{task.id}", + json={"status": "READY", "auto_close_github_issue": True}, + ) + assert r.status_code == 200 + assert calls == [] + assert tasks.get(test_client.workspace, task.id).status == TaskStatus.READY + + def test_optout_on_done_task_does_not_close(self, test_client, monkeypatch): + """Disabling auto-close on a DONE task must not trigger a close.""" + from codeframe.core import tasks + from codeframe.core.state_machine import TaskStatus + + calls = [] + monkeypatch.setattr( + tasks, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + + task = tasks.create( + test_client.workspace, + title="Already done", + status=TaskStatus.DONE, + github_issue_number=88, + external_url="https://github.com/acme/app/issues/88", + auto_close_github_issue=True, + ) + r = test_client.patch( + f"/api/v2/tasks/{task.id}", + json={"auto_close_github_issue": False}, + ) + assert r.status_code == 200 + assert calls == [] + + def test_rejected_transition_does_not_persist_auto_close(self, test_client): + """An invalid transition must not leave the auto-close flag mutated.""" + from codeframe.core import tasks + from codeframe.core.state_machine import TaskStatus + + task = tasks.create( + test_client.workspace, + title="Imported", + status=TaskStatus.BACKLOG, + github_issue_number=5, + external_url="https://github.com/acme/app/issues/5", + auto_close_github_issue=False, + ) + # BACKLOG -> DONE is not an allowed transition; the request is rejected + # and the auto_close flag must remain unchanged (no hidden side effect). + r = test_client.patch( + f"/api/v2/tasks/{task.id}", + json={"status": "DONE", "auto_close_github_issue": True}, + ) + assert r.status_code == 400 + assert ( + tasks.get(test_client.workspace, task.id).auto_close_github_issue is False + ) diff --git a/web-ui/src/__tests__/components/tasks/GitHubIssueBadge.test.tsx b/web-ui/src/__tests__/components/tasks/GitHubIssueBadge.test.tsx new file mode 100644 index 00000000..8f4d65d3 --- /dev/null +++ b/web-ui/src/__tests__/components/tasks/GitHubIssueBadge.test.tsx @@ -0,0 +1,33 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; + +import { GitHubIssueBadge } from '@/components/tasks/GitHubIssueBadge'; + +describe('GitHubIssueBadge', () => { + it('renders the issue number and links to the issue', () => { + render( + + ); + const link = screen.getByRole('link', { + name: /imported from github issue #42/i, + }); + expect(link).toHaveAttribute('href', 'https://github.com/acme/app/issues/42'); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', expect.stringContaining('noopener')); + expect(screen.getByText(/Imported from GitHub #42/)).toBeInTheDocument(); + }); + + it('does not propagate clicks to parent handlers', () => { + const parentClick = jest.fn(); + render( +
+ +
+ ); + screen.getByRole('link').click(); + expect(parentClick).not.toHaveBeenCalled(); + }); +}); diff --git a/web-ui/src/__tests__/components/tasks/GitHubIssueImportModal.test.tsx b/web-ui/src/__tests__/components/tasks/GitHubIssueImportModal.test.tsx index 5533e24c..2d21d17f 100644 --- a/web-ui/src/__tests__/components/tasks/GitHubIssueImportModal.test.tsx +++ b/web-ui/src/__tests__/components/tasks/GitHubIssueImportModal.test.tsx @@ -178,4 +178,25 @@ describe('GitHubIssueImportModal', () => { const lastCallKey = mockUseSWR.mock.calls.at(-1)?.[0]; expect(lastCallKey).toBeNull(); }); + + it('renders an inline import error and keeps the modal usable', () => { + mockSWRByPage({ 1: [issue(1, 'x')] }, 1); + render( + + ); + expect(screen.getByRole('alert')).toHaveTextContent( + 'No GitHub repository is connected.' + ); + // The issue row is still rendered (selection preserved for a retry). + expect(screen.getByText('x')).toBeInTheDocument(); + }); + + it('shows an importing progress label on the action button', () => { + mockSWRByPage({ 1: [issue(1, 'x')] }, 1); + render(); + expect( + screen.getByRole('button', { name: /importing/i }) + ).toBeInTheDocument(); + }); + }); diff --git a/web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx b/web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx new file mode 100644 index 00000000..39fb7493 --- /dev/null +++ b/web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx @@ -0,0 +1,167 @@ +import React from 'react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import useSWR from 'swr'; + +import { TaskBoardView } from '@/components/tasks/TaskBoardView'; +import { integrationsApi } from '@/lib/api'; +import type { GitHubIntegrationStatus, GitHubIssue } from '@/types'; + +// Focused test for the #565 import execution wiring: clicking the stubbed +// modal's import trigger calls integrationsApi.importIssues and surfaces a +// summary banner on success. + +jest.mock('swr'); +jest.mock('next/navigation', () => ({ useRouter: () => ({ push: jest.fn() }) })); +jest.mock('next/link', () => { + const MockLink = ({ href, children }: { href: string; children: React.ReactNode }) => ( + {children} + ); + MockLink.displayName = 'MockLink'; + return MockLink; +}); +jest.mock('@/hooks/useRequirementsLookup', () => ({ + useRequirementsLookup: () => ({ requirementsMap: new Map(), isLoading: false }), +})); +jest.mock('@/lib/api', () => ({ + tasksApi: { getAll: jest.fn() }, + prdApi: { getAll: jest.fn() }, + costsApi: { getTopTasks: jest.fn() }, + integrationsApi: { getStatus: jest.fn(), getIssues: jest.fn(), importIssues: jest.fn() }, +})); + +jest.mock('@/components/tasks/TaskBoardContent', () => ({ + TaskBoardContent: () =>
, +})); +jest.mock('@/components/tasks/TaskDetailModal', () => ({ TaskDetailModal: () => null })); +jest.mock('@/components/tasks/TaskFilters', () => ({ TaskFilters: () =>
})); +jest.mock('@/components/tasks/BatchActionsBar', () => ({ BatchActionsBar: () =>
})); +jest.mock('@/components/tasks/BulkActionConfirmDialog', () => ({ + BulkActionConfirmDialog: () => null, +})); + +// Stub the import modal: expose onImport via a trigger button and reflect the +// `importing` flag so the test can assert progress state. +jest.mock('@/components/tasks/GitHubIssueImportModal', () => ({ + GitHubIssueImportModal: ({ + open, + importing, + importError, + onImport, + }: { + open: boolean; + importing: boolean; + importError: string | null; + onImport: (issues: GitHubIssue[]) => void; + }) => + open ? ( +
+ {importing && importing-flag} + {importError &&
{importError}
} + +
+ ) : null, +})); + +const mockUseSWR = useSWR as jest.MockedFunction; +const mockMutate = jest.fn(); + +const CONNECTED: GitHubIntegrationStatus = { + connected: true, + repo: 'acme/app', + owner_login: 'acme', + owner_avatar_url: '', +}; + +function setupSWR() { + mockUseSWR.mockImplementation((key) => { + const k = String(key); + if (k.includes('/integrations/github/status')) { + return { data: CONNECTED, error: undefined, isLoading: false, mutate: jest.fn() } as never; + } + if (k.includes('/api/v2/tasks')) { + return { + data: { tasks: [], total: 0 }, + error: undefined, + isLoading: false, + mutate: mockMutate, + } as never; + } + return { data: undefined, error: undefined, isLoading: false, mutate: jest.fn() } as never; + }); +} + +describe('TaskBoardView — GitHub import execution (#565)', () => { + beforeEach(() => { + jest.clearAllMocks(); + setupSWR(); + }); + + it('imports selected issues and shows a summary banner', async () => { + (integrationsApi.importIssues as jest.Mock).mockResolvedValue({ + created: [{ task_id: 't1', issue_number: 12, title: 'Fix login' }], + skipped: [], + total_created: 1, + }); + + render(); + fireEvent.click(screen.getByRole('button', { name: /import from github/i })); + fireEvent.click(screen.getByText('trigger-import')); + + await waitFor(() => + expect(integrationsApi.importIssues).toHaveBeenCalledWith('/ws', [12]) + ); + await waitFor(() => + expect(screen.getByRole('status')).toHaveTextContent(/1 task created/i) + ); + expect(mockMutate).toHaveBeenCalled(); + }); + + it('reports skipped duplicates in the summary', async () => { + (integrationsApi.importIssues as jest.Mock).mockResolvedValue({ + created: [], + skipped: [12], + total_created: 0, + }); + + render(); + fireEvent.click(screen.getByRole('button', { name: /import from github/i })); + fireEvent.click(screen.getByText('trigger-import')); + + await waitFor(() => + expect(screen.getByRole('status')).toHaveTextContent( + /0 tasks created · 1 skipped \(already imported\)/i + ) + ); + }); + + it('surfaces an error banner when the import fails', async () => { + (integrationsApi.importIssues as jest.Mock).mockRejectedValue({ + detail: 'No GitHub repository is connected.', + }); + + render(); + fireEvent.click(screen.getByRole('button', { name: /import from github/i })); + fireEvent.click(screen.getByText('trigger-import')); + + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent( + /No GitHub repository is connected/i + ) + ); + }); +}); diff --git a/web-ui/src/__tests__/components/tasks/TaskDetailModal.test.tsx b/web-ui/src/__tests__/components/tasks/TaskDetailModal.test.tsx index bcf60518..61313ffe 100644 --- a/web-ui/src/__tests__/components/tasks/TaskDetailModal.test.tsx +++ b/web-ui/src/__tests__/components/tasks/TaskDetailModal.test.tsx @@ -38,6 +38,7 @@ jest.mock('@/lib/api', () => ({ getOne: jest.fn(), getAll: jest.fn(), updateStatus: jest.fn(), + updateGitHubSettings: jest.fn(), }, })); @@ -159,3 +160,47 @@ describe('TaskDetailModal last changed timestamp', () => { expect(screen.queryByText(/last changed/i)).not.toBeInTheDocument(); }); }); + +describe('TaskDetailModal — GitHub traceability (#565)', () => { + beforeEach(() => jest.clearAllMocks()); + + it('shows the GitHub badge for imported tasks', async () => { + renderModal({ + github_issue_number: 42, + external_url: 'https://github.com/acme/app/issues/42', + }); + await waitFor(() => expect(screen.getByText('Test Task')).toBeInTheDocument()); + const link = screen.getByRole('link', { name: /imported from github issue #42/i }); + expect(link).toHaveAttribute('href', 'https://github.com/acme/app/issues/42'); + }); + + it('does not show the badge for non-imported tasks', async () => { + renderModal({ github_issue_number: undefined, external_url: undefined }); + await waitFor(() => expect(screen.getByText('Test Task')).toBeInTheDocument()); + expect( + screen.queryByRole('checkbox', { + name: /close github issue when task is marked done/i, + }) + ).not.toBeInTheDocument(); + }); + + it('persists the auto-close toggle via the API', async () => { + (tasksApi.updateGitHubSettings as jest.Mock).mockResolvedValue({}); + renderModal({ + github_issue_number: 42, + external_url: 'https://github.com/acme/app/issues/42', + auto_close_github_issue: false, + }); + await waitFor(() => expect(screen.getByText('Test Task')).toBeInTheDocument()); + + const checkbox = screen.getByRole('checkbox', { + name: /close github issue when task is marked done/i, + }); + expect(checkbox).not.toBeChecked(); + checkbox.click(); + + await waitFor(() => + expect(tasksApi.updateGitHubSettings).toHaveBeenCalledWith('/ws', 'task-1', true) + ); + }); +}); diff --git a/web-ui/src/components/tasks/GitHubIssueBadge.tsx b/web-ui/src/components/tasks/GitHubIssueBadge.tsx new file mode 100644 index 00000000..1a8491fb --- /dev/null +++ b/web-ui/src/components/tasks/GitHubIssueBadge.tsx @@ -0,0 +1,28 @@ +'use client'; + +import { GithubIcon } from '@hugeicons/react'; + +interface GitHubIssueBadgeProps { + issueNumber: number; + issueUrl: string; +} + +/** + * A small badge linking an imported task back to its source GitHub issue + * (issue #565). Opens the issue in a new tab. + */ +export function GitHubIssueBadge({ issueNumber, issueUrl }: GitHubIssueBadgeProps) { + return ( + e.stopPropagation()} + aria-label={`Imported from GitHub issue #${issueNumber}`} + className="inline-flex items-center gap-1.5 rounded-md border bg-muted/40 px-2 py-0.5 text-xs font-medium text-muted-foreground transition-colors hover:bg-muted hover:text-foreground focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring" + > + + Imported from GitHub #{issueNumber} + + ); +} diff --git a/web-ui/src/components/tasks/GitHubIssueImportModal.tsx b/web-ui/src/components/tasks/GitHubIssueImportModal.tsx index 9ab0b9bd..4074511a 100644 --- a/web-ui/src/components/tasks/GitHubIssueImportModal.tsx +++ b/web-ui/src/components/tasks/GitHubIssueImportModal.tsx @@ -29,6 +29,10 @@ interface GitHubIssueImportModalProps { workspacePath: string; /** Connected repo slug ("owner/repo") for the header, when known. */ repo?: string | null; + /** True while the parent is executing the import (#565) — shows progress. */ + importing?: boolean; + /** Error message from a failed import (#565) — rendered inline in the modal. */ + importError?: string | null; onClose: () => void; /** Called with the chosen issues when the user confirms the import. */ onImport: (selectedIssues: GitHubIssue[]) => void; @@ -40,6 +44,8 @@ export function GitHubIssueImportModal({ open, workspacePath, repo, + importing = false, + importError = null, onClose, onImport, }: GitHubIssueImportModalProps) { @@ -266,6 +272,17 @@ export function GitHubIssueImportModal({ })}
+ {/* Import error (kept in-modal so the selection is preserved) */} + {importError && ( +
+ + {importError} +
+ )} + {/* Footer: pagination + actions */}
@@ -297,12 +314,14 @@ export function GitHubIssueImportModal({
diff --git a/web-ui/src/components/tasks/TaskBoardView.tsx b/web-ui/src/components/tasks/TaskBoardView.tsx index 40600718..7f4a951c 100644 --- a/web-ui/src/components/tasks/TaskBoardView.tsx +++ b/web-ui/src/components/tasks/TaskBoardView.tsx @@ -95,14 +95,57 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { // ─── Detail modal state ──────────────────────────────────────── const [detailTaskId, setDetailTaskId] = useState(null); - // ─── GitHub issue import modal (issue #564) ──────────────────── + // ─── GitHub issue import modal (issues #564 / #565) ──────────── const [importModalOpen, setImportModalOpen] = useState(false); - - // The actual import flow lands in #565; for now selecting + confirming - // simply closes the modal (the browser/multi-select is the #564 deliverable). - const handleImportIssues = useCallback((_selected: GitHubIssue[]) => { - setImportModalOpen(false); - }, []); + const [isImporting, setIsImporting] = useState(false); + const [importSummary, setImportSummary] = useState(null); + const [importError, setImportError] = useState(null); + + // Execute the import (#565): create tasks from the selected issues, then + // refresh the board so the new tasks (with their GitHub badges) appear. + const handleImportIssues = useCallback( + async (selectedIssues: GitHubIssue[]) => { + if (selectedIssues.length === 0) { + setImportModalOpen(false); + return; + } + setIsImporting(true); + setActionError(null); + setImportSummary(null); + setImportError(null); + let imported = false; + try { + const numbers = selectedIssues.map((i) => i.number); + const result = await integrationsApi.importIssues(workspacePath, numbers); + imported = true; + setImportModalOpen(false); + const parts = [ + `${result.total_created} task${result.total_created !== 1 ? 's' : ''} created`, + ]; + if (result.skipped.length > 0) { + parts.push( + `${result.skipped.length} skipped (already imported)` + ); + } + setImportSummary(parts.join(' · ')); + } catch (err) { + const apiErr = err as ApiError; + // Keep the modal open and show the error inline there, so the user sees + // it (a board-level banner would sit behind the dialog) and keeps their + // selection for a retry. + setImportError(apiErr.detail || 'Failed to import issues from GitHub'); + } finally { + setIsImporting(false); + } + // Refresh the board AFTER the import resolves. A refresh failure is not an + // import failure — the tasks were already created — so it must not flip + // the success summary into an error (SWR will revalidate again later). + if (imported) { + mutate(); + } + }, + [workspacePath, mutate] + ); // ─── Error state for actions ─────────────────────────────────── const [actionError, setActionError] = useState(null); @@ -398,6 +441,23 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { />
+ {/* Import success summary banner (#565) */} + {importSummary && ( +
+ Imported from GitHub: {importSummary} + +
+ )} + {/* Action error banner */} {actionError && (
@@ -486,7 +546,12 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { open={importModalOpen} workspacePath={workspacePath} repo={ghStatus?.repo} - onClose={() => setImportModalOpen(false)} + importing={isImporting} + importError={importError} + onClose={() => { + setImportModalOpen(false); + setImportError(null); + }} onImport={handleImportIssues} /> diff --git a/web-ui/src/components/tasks/TaskDetailModal.tsx b/web-ui/src/components/tasks/TaskDetailModal.tsx index 7a6bca62..86d2a0b6 100644 --- a/web-ui/src/components/tasks/TaskDetailModal.tsx +++ b/web-ui/src/components/tasks/TaskDetailModal.tsx @@ -27,6 +27,8 @@ import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/comp import { STATUS_INFO } from '@/lib/taskStatusInfo'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; +import { Checkbox } from '@/components/ui/checkbox'; +import { GitHubIssueBadge } from './GitHubIssueBadge'; import useSWR from 'swr'; import { tasksApi } from '@/lib/api'; import { useRequirementsLookup } from '@/hooks/useRequirementsLookup'; @@ -128,6 +130,21 @@ export function TaskDetailModal({ } }; + // Toggle "close GitHub issue when DONE" for imported tasks (#565). Optimistic: + // flip locally, then persist; revert on failure. + const handleToggleAutoClose = async (checked: boolean) => { + if (!task) return; + const previous = task.auto_close_github_issue ?? false; + setTask({ ...task, auto_close_github_issue: checked }); + try { + await tasksApi.updateGitHubSettings(workspacePath, task.id, checked); + } catch (err) { + const apiErr = err as ApiError; + setError(apiErr.detail || 'Failed to update GitHub setting'); + setTask({ ...task, auto_close_github_issue: previous }); + } + }; + return ( { if (!isOpen) onClose(); }}> @@ -209,6 +226,24 @@ export function TaskDetailModal({ )}
+ {/* GitHub issue linkage (#565) — badge + auto-close toggle */} + {task.github_issue_number != null && task.external_url && ( +
+ + +
+ )} + {/* Dependencies — full list with status highlights and navigation */} {task.depends_on.length > 0 && (
diff --git a/web-ui/src/lib/api.ts b/web-ui/src/lib/api.ts index 1d798977..a61b6bb1 100644 --- a/web-ui/src/lib/api.ts +++ b/web-ui/src/lib/api.ts @@ -78,6 +78,7 @@ import type { GitHubIntegrationStatus, GitHubIssuesResponse, GitHubIssuesParams, + GitHubImportResponse, } from '@/types'; // FastAPI validation error format @@ -258,6 +259,22 @@ export const tasksApi = { return response.data; }, + /** + * Toggle whether the linked GitHub issue closes when the task is DONE (#565) + */ + updateGitHubSettings: async ( + workspacePath: string, + taskId: string, + autoClose: boolean + ): Promise => { + const response = await api.patch( + `/api/v2/tasks/${taskId}`, + { auto_close_github_issue: autoClose }, + { params: { workspace_path: workspacePath } } + ); + return response.data; + }, + /** * Start execution of a single task */ @@ -1038,6 +1055,20 @@ export const integrationsApi = { ); return response.data; }, + + // Import selected GitHub issues as tasks (issue #565). + importIssues: async ( + workspacePath: string, + issueNumbers: number[] + ): Promise => { + const response = await api.post( + '/api/v2/integrations/github/import', + { issue_numbers: issueNumbers }, + { params: { workspace_path: workspacePath } } + ); + return response.data; + }, + }; // Cost analytics API (issues #557, #558) diff --git a/web-ui/src/types/index.ts b/web-ui/src/types/index.ts index 53a5fa09..5c21764d 100644 --- a/web-ui/src/types/index.ts +++ b/web-ui/src/types/index.ts @@ -84,6 +84,10 @@ export interface Task { estimated_hours?: number; created_at?: string; updated_at?: string; + // GitHub issue traceability (issue #565) + github_issue_number?: number | null; + external_url?: string | null; + auto_close_github_issue?: boolean; } export interface TaskListResponse { @@ -700,6 +704,19 @@ export interface GitHubIssuesParams { label?: string; } +// GitHub issue import execution (issue #565) +export interface ImportedTaskSummary { + task_id: string; + issue_number: number; + title: string; +} + +export interface GitHubImportResponse { + created: ImportedTaskSummary[]; + skipped: number[]; + total_created: number; +} + // Cost analytics types (issue #557) export interface DailyCostPoint { date: string; // ISO YYYY-MM-DD