From 4f9ab7c9cc9e618bc9a7bb0f0e2621c3c14cfce9 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 06:50:13 -0700 Subject: [PATCH 01/20] feat(tasks): GitHub-issue traceability fields on v2 tasks (#565) Add external_url + auto_close_github_issue columns to the v2 tasks table (both _init_database and _ensure_schema_upgrades), persist github_issue_number through create(), and add get_by_github_issue_number() (duplicate-import protection) and update_auto_close() helpers. --- codeframe/core/tasks.py | 106 ++++++++++++++++++-- codeframe/core/workspace.py | 12 +++ tests/core/test_task_github_traceability.py | 105 +++++++++++++++++++ 3 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 tests/core/test_task_github_traceability.py diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index 6834f011..2d674211 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -61,6 +61,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 +81,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 +118,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 +144,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 +167,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 +182,87 @@ def get(workspace: Workspace, task_id: str) -> Optional[Task]: return _row_to_task(row) +def get_by_github_issue_number( + workspace: Workspace, github_issue_number: int +) -> Optional[Task]: + """Get a task previously imported from a given GitHub issue number. + + Used for duplicate-import protection (issue #565): if a task already exists + for an issue number in this workspace, the import is skipped rather than + creating a second task. + + Args: + workspace: Workspace to query + github_issue_number: GitHub issue number to look up + + Returns: + The matching Task if one exists, otherwise None + """ + conn = get_db_connection(workspace) + 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 github_issue_number = ? + LIMIT 1 + """, + (workspace.id, github_issue_number), + ) + row = cursor.fetchone() + 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 +284,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 +295,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 @@ -753,7 +842,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 +881,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..f80560ef 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(""" @@ -522,6 +526,14 @@ 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() # Ensure runs table exists before creating dependent tables (run_logs, diagnostic_reports) cursor.execute( diff --git a/tests/core/test_task_github_traceability.py b/tests/core/test_task_github_traceability.py new file mode 100644 index 00000000..051142f1 --- /dev/null +++ b/tests/core/test_task_github_traceability.py @@ -0,0 +1,105 @@ +"""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.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 TestGetByGithubIssueNumber: + def test_returns_matching_task(self, workspace): + created = tasks.create( + workspace, title="Imported", github_issue_number=123 + ) + found = tasks.get_by_github_issue_number(workspace, 123) + 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_github_issue_number(workspace, 555) is None + + def test_scoped_to_workspace(self, workspace, tmp_path): + """A different workspace must not see this workspace's imported issue.""" + tasks.create(workspace, title="Imported", github_issue_number=10) + other_path = tmp_path / "other" + other_path.mkdir() + other = create_or_load_workspace(other_path) + assert tasks.get_by_github_issue_number(other, 10) is None + + +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) From aa526d01dca71b9a4534a23ffaf0509eb993b385 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 07:05:02 -0700 Subject: [PATCH 02/20] feat(github): import issues as tasks + auto-close on DONE (#565) Backend for executing the GitHub issue import: - core/github_issues_service: get_issue() + close_issue() - POST /api/v2/integrations/github/import (dedupe via github_issue_number, labels footer in description) and PATCH /issues/{number}/close - tasks_v2: expose github_issue_number/external_url/auto_close_github_issue on TaskResponse, accept auto_close toggle on PATCH, fire fire-and-forget issue close on the DONE transition (failures swallowed) - tests disable the shared rate limiter to avoid ip:testclient bucket bleed --- codeframe/core/github_issues_service.py | 136 +++++++++++++++ .../ui/routers/github_integrations_v2.py | 136 ++++++++++++++- codeframe/ui/routers/tasks_v2.py | 144 +++++++++++----- tests/core/test_github_issues_service.py | 129 ++++++++++++++ tests/ui/test_github_integrations_v2.py | 159 ++++++++++++++++++ tests/ui/test_v2_routers_integration.py | 154 +++++++++++++++++ 6 files changed, 818 insertions(+), 40 deletions(-) diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index ec26ff2a..5026dac7 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -190,6 +190,142 @@ 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.") + + _raise_for_status(resp.status_code, context="get issue") + + raw = resp.json() + if not isinstance(raw, dict): + raw = {} + 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, + 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. + 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: + try: + cresp = await client.post( + f"{base}/comments", json={"body": comment}, headers=headers + ) + except httpx.HTTPError as exc: + logger.warning("GitHub issue comment failed: %s", type(exc).__name__) + raise GitHubConnectError("Could not reach GitHub. Try again later.") + _raise_for_status(cresp.status_code, context="issue comment") + + 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") + return True + finally: + if own_client: + await client.aclose() + + async def _search_issues( client: httpx.AsyncClient, headers: dict[str, str], diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index 1bf52752..dfbbe0d4 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -29,12 +29,13 @@ parse_repo, validate_connection, ) -from codeframe.core.github_issues_service import list_issues +from codeframe.core.github_issues_service import close_issue, 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 +89,29 @@ 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 + + +class CloseIssueResponse(BaseModel): + success: bool + issue_number: 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 @@ -338,3 +362,113 @@ 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 ``github_issue_number``. + """ + repo, pat = _require_connection(workspace, manager) + + created: list[ImportedTaskSummary] = [] + skipped: list[int] = [] + + for number in body.issue_numbers: + existing = tasks.get_by_github_issue_number(workspace, number) + if existing is not None: + skipped.append(number) + continue + + try: + issue = await get_issue(pat, repo, number) + except GitHubConnectError as e: + raise _map_github_error(e) + + description = issue["body"] or "" + if issue["labels"]: + footer = "**Labels:** " + ", ".join(issue["labels"]) + description = f"{description}\n\n{footer}" if description else footer + + task = tasks.create( + workspace, + title=issue["title"], + description=description, + github_issue_number=number, + external_url=issue["html_url"], + ) + created.append( + ImportedTaskSummary( + task_id=task.id, issue_number=number, title=task.title + ) + ) + + return ImportResponse( + created=created, skipped=skipped, total_created=len(created) + ) + + +@router.patch("/issues/{number}/close", response_model=CloseIssueResponse) +@rate_limit_standard() +async def close_github_issue( + request: Request, + number: int, + workspace: Workspace = Depends(get_v2_workspace), + manager: CredentialManager = Depends(get_credential_manager), +) -> CloseIssueResponse: + """Close a GitHub issue (issue #565), posting a completion comment.""" + repo, pat = _require_connection(workspace, manager) + + try: + await close_issue(pat, repo, number, comment="Completed via CodeFRAME") + except GitHubConnectError as e: + raise _map_github_error(e) + + return CloseIssueResponse(success=True, issue_number=number) diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index de97e41b..33d4d011 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -25,6 +25,10 @@ from codeframe.core.workspace import Workspace from codeframe.lib.rate_limiter import rate_limit_ai, rate_limit_standard from codeframe.core import runtime, tasks, conductor, streaming +from codeframe.core.credentials import CredentialManager, CredentialProvider +from codeframe.core.github_connect_service import GitHubConnectError +from codeframe.core.github_integration_config import load_github_integration_config +from codeframe.core.github_issues_service import close_issue from codeframe.core.runtime import RunStatus from codeframe.core.state_machine import TaskStatus from codeframe.ui.dependencies import get_v2_workspace @@ -35,6 +39,52 @@ router = APIRouter(prefix="/api/v2/tasks", tags=["tasks-v2"]) +def get_credential_manager() -> CredentialManager: + """Dependency: machine-wide CredentialManager (overridden in tests). + + Shares the ``CredentialProvider.GIT_GITHUB`` slot used by the Integrations + settings tab (#555/#563) — used here to resolve the PAT for GitHub + issue auto-close on task completion (#565). + """ + return CredentialManager() + + +async def _auto_close_github_issue( + workspace: Workspace, + manager: CredentialManager, + github_issue_number: int, +) -> None: + """Close the linked GitHub issue when its task is marked DONE (#565). + + Runs as a fire-and-forget background task. Resolves the repo from the + per-workspace integration config and the PAT from the credential store; a + missing connection (or any GitHub error) is logged and swallowed so the + task transition is never affected. + """ + try: + cfg = load_github_integration_config(workspace) + pat = manager.get_credential(CredentialProvider.GIT_GITHUB) + if cfg is None or not pat: + logger.info( + "Skipping GitHub auto-close for issue #%s: no connection.", + github_issue_number, + ) + return + await close_issue( + pat, cfg["repo"], github_issue_number, comment="Completed via CodeFRAME" + ) + except GitHubConnectError as e: + logger.warning( + "Auto-close of GitHub issue #%s failed: %s", github_issue_number, e + ) + except Exception as e: # noqa: BLE001 - must never break the task transition + logger.warning( + "Unexpected error auto-closing GitHub issue #%s: %s", + github_issue_number, + e, + ) + + # ============================================================================ # Request/Response Models # ============================================================================ @@ -145,6 +195,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 +235,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 +286,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 +318,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) @@ -275,7 +327,9 @@ async def update_task( request: Request, task_id: str, body: UpdateTaskRequest, + background_tasks: BackgroundTasks, workspace: Workspace = Depends(get_v2_workspace), + manager: CredentialManager = Depends(get_credential_manager), ) -> TaskResponse: """Update a task's title, description, priority, or status. @@ -296,11 +350,13 @@ async def update_task( - 400: Invalid status or status transition """ try: + transitioned_to_done = False # Handle status update separately if provided if body.status: try: new_status = TaskStatus(body.status.upper()) tasks.update_status(workspace, task_id, new_status) + transitioned_to_done = new_status == TaskStatus.DONE except ValueError as e: if "Invalid status" in str(e) or "not a valid" in str(e).lower(): raise HTTPException( @@ -326,18 +382,28 @@ 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, - ) + # Persist the GitHub auto-close preference if provided (#565). + if body.auto_close_github_issue is not None: + task = tasks.update_auto_close( + workspace, task_id, body.auto_close_github_issue + ) + + # On a DONE transition, fire-and-forget the GitHub issue close when the + # task opted in and is linked to an issue (#565). Failures are swallowed + # inside the background task — the transition has already succeeded. + if ( + transitioned_to_done + and task.auto_close_github_issue + and task.github_issue_number is not None + ): + background_tasks.add_task( + _auto_close_github_issue, + workspace, + manager, + task.github_issue_number, + ) + + return TaskResponse.from_task(task) except ValueError as e: error_msg = str(e) diff --git a/tests/core/test_github_issues_service.py b/tests/core/test_github_issues_service.py index 8265d207..04763fad 100644 --- a/tests/core/test_github_issues_service.py +++ b/tests/core/test_github_issues_service.py @@ -223,3 +223,132 @@ 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 close_issue, get_issue # noqa: E402 + + +class TestGetIssue: + @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_maps_to_connect_error(self): + 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) + + +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_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) diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index f633f463..b9763fe9 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -25,6 +25,29 @@ VALID_PAT = "ghp_validtoken1234567890" +@pytest.fixture(autouse=True) +def _disable_rate_limiting(monkeypatch): + """Disable rate limiting around each test. + + The TestClient hits a fixed ``ip:testclient`` bucket, so AI-rate-limited + endpoints (connect/import) accumulate across the whole module and would + eventually 429. Disabling the limiter and resetting the cached config + + limiter singleton keeps each test isolated. + """ + from codeframe.config.rate_limits import _reset_rate_limit_config + from codeframe.core.config import reset_global_config + from codeframe.lib.rate_limiter import reset_rate_limiter + + monkeypatch.setenv("RATE_LIMIT_ENABLED", "false") + _reset_rate_limit_config() + reset_global_config() + reset_rate_limiter() + yield + _reset_rate_limit_config() + reset_global_config() + reset_rate_limiter() + + @pytest.fixture def workspace(): temp_dir = Path(tempfile.mkdtemp()) @@ -362,3 +385,139 @@ 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) + + +def _mock_close_issue(monkeypatch, *, calls=None, exc=None): + from codeframe.ui.routers import github_integrations_v2 + + async def fake(pat, repo, number, **kwargs): + if calls is not None: + calls.append({"repo": repo, "number": number, **kwargs}) + if exc is not None: + raise exc + return True + + monkeypatch.setattr(github_integrations_v2, "close_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_github_issue_number(workspace, 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" + + 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_github_issue_number(workspace, 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 + + +class TestCloseIssueEndpoint: + def test_requires_connection(self, client): + r = client.patch("/api/v2/integrations/github/issues/5/close") + assert r.status_code == 409 + + def test_closes_issue(self, client, monkeypatch): + _connect(client, monkeypatch) + calls = [] + _mock_close_issue(monkeypatch, calls=calls) + r = client.patch("/api/v2/integrations/github/issues/5/close") + assert r.status_code == 200 + assert r.json() == {"success": True, "issue_number": 5} + assert calls[0]["number"] == 5 + + def test_invalid_token_maps_to_401(self, client, monkeypatch): + _connect(client, monkeypatch) + from codeframe.core.github_connect_service import InvalidTokenError + + _mock_close_issue(monkeypatch, exc=InvalidTokenError("bad")) + r = client.patch("/api/v2/integrations/github/issues/5/close") + assert r.status_code == 401 diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index 1ddd055c..cec89168 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1161,3 +1161,157 @@ 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 _build_client(self, workspace): + """A tasks_v2 client with an isolated credential manager holding a PAT.""" + import tempfile as _tf + from pathlib import Path as _Path + + from codeframe.core.credentials import ( + CredentialManager, + CredentialProvider, + CredentialStore, + ) + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + from codeframe.ui.dependencies import get_v2_workspace + from codeframe.ui.routers import tasks_v2 + + save_github_integration_config( + workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + ) + store = CredentialStore(storage_dir=_Path(_tf.mkdtemp())) + store._keyring_available = False + manager = CredentialManager.__new__(CredentialManager) + manager._store = store + manager.set_credential(CredentialProvider.GIT_GITHUB, "ghp_token") + + app = FastAPI() + app.include_router(tasks_v2.router) + app.dependency_overrides[get_v2_workspace] = lambda: workspace + app.dependency_overrides[tasks_v2.get_credential_manager] = lambda: manager + client = TestClient(app) + client.workspace = workspace + return client + + def test_done_transition_triggers_auto_close(self, test_workspace, monkeypatch): + from codeframe.core import tasks + from codeframe.core.state_machine import TaskStatus + from codeframe.ui.routers import tasks_v2 + + calls = [] + + async def fake_close(pat, repo, number, **kwargs): + calls.append((repo, number)) + return True + + monkeypatch.setattr(tasks_v2, "close_issue", fake_close) + + client = self._build_client(test_workspace) + task = tasks.create( + test_workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + auto_close_github_issue=True, + ) + r = client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) + assert r.status_code == 200 + assert calls == [("acme/app", 99)] + + def test_done_without_auto_close_does_not_close(self, test_workspace, monkeypatch): + from codeframe.core import tasks + from codeframe.core.state_machine import TaskStatus + from codeframe.ui.routers import tasks_v2 + + calls = [] + + async def fake_close(pat, repo, number, **kwargs): + calls.append(number) + return True + + monkeypatch.setattr(tasks_v2, "close_issue", fake_close) + + client = self._build_client(test_workspace) + task = tasks.create( + test_workspace, + title="Imported, no auto-close", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + auto_close_github_issue=False, + ) + r = client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) + assert r.status_code == 200 + assert calls == [] + + def test_done_auto_close_failure_does_not_break_transition( + self, test_workspace, monkeypatch + ): + from codeframe.core import tasks + from codeframe.core.github_connect_service import GitHubConnectError + from codeframe.core.state_machine import TaskStatus + from codeframe.ui.routers import tasks_v2 + + async def boom(pat, repo, number, **kwargs): + raise GitHubConnectError("GitHub down") + + monkeypatch.setattr(tasks_v2, "close_issue", boom) + + client = self._build_client(test_workspace) + task = tasks.create( + test_workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + auto_close_github_issue=True, + ) + r = client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) + assert r.status_code == 200 + assert tasks.get(test_workspace, task.id).status == TaskStatus.DONE From ebe66031e1a73ccb58b9d91f68db35d3ca5c47ab Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 07:11:08 -0700 Subject: [PATCH 03/20] feat(web-ui): execute GitHub issue import + task traceability (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - integrationsApi.importIssues/closeIssue + tasksApi.updateGitHubSettings - GitHubIssueBadge component (links a task to its source issue) - TaskBoardView: real import flow with in-progress state + summary banner (N created · M skipped) and board refresh - TaskDetailModal: GitHub badge + 'close issue when DONE' checkbox (optimistic) - Task type extended with github_issue_number/external_url/auto_close_github_issue --- .../tasks/GitHubIssueBadge.test.tsx | 33 ++++ .../tasks/TaskBoardView.import565.test.tsx | 164 ++++++++++++++++++ .../components/tasks/TaskDetailModal.test.tsx | 45 +++++ .../src/components/tasks/GitHubIssueBadge.tsx | 28 +++ .../tasks/GitHubIssueImportModal.tsx | 11 +- web-ui/src/components/tasks/TaskBoardView.tsx | 63 ++++++- .../src/components/tasks/TaskDetailModal.tsx | 35 ++++ web-ui/src/lib/api.ts | 43 +++++ web-ui/src/types/index.ts | 17 ++ 9 files changed, 429 insertions(+), 10 deletions(-) create mode 100644 web-ui/src/__tests__/components/tasks/GitHubIssueBadge.test.tsx create mode 100644 web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx create mode 100644 web-ui/src/components/tasks/GitHubIssueBadge.tsx 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/TaskBoardView.import565.test.tsx b/web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx new file mode 100644 index 00000000..878537d6 --- /dev/null +++ b/web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx @@ -0,0 +1,164 @@ +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, + onImport, + }: { + open: boolean; + importing: boolean; + onImport: (issues: GitHubIssue[]) => void; + }) => + open ? ( +
+ {importing && importing-flag} + +
+ ) : 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..eb6c01e7 100644 --- a/web-ui/src/components/tasks/GitHubIssueImportModal.tsx +++ b/web-ui/src/components/tasks/GitHubIssueImportModal.tsx @@ -29,6 +29,8 @@ 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; onClose: () => void; /** Called with the chosen issues when the user confirms the import. */ onImport: (selectedIssues: GitHubIssue[]) => void; @@ -40,6 +42,7 @@ export function GitHubIssueImportModal({ open, workspacePath, repo, + importing = false, onClose, onImport, }: GitHubIssueImportModalProps) { @@ -297,12 +300,14 @@ export function GitHubIssueImportModal({
diff --git a/web-ui/src/components/tasks/TaskBoardView.tsx b/web-ui/src/components/tasks/TaskBoardView.tsx index 40600718..4635c820 100644 --- a/web-ui/src/components/tasks/TaskBoardView.tsx +++ b/web-ui/src/components/tasks/TaskBoardView.tsx @@ -95,14 +95,45 @@ 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); + + // 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); + try { + const numbers = selectedIssues.map((i) => i.number); + const result = await integrationsApi.importIssues(workspacePath, numbers); + 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(' · ')); + await mutate(); + } catch (err) { + const apiErr = err as ApiError; + setActionError(apiErr.detail || 'Failed to import issues from GitHub'); + } finally { + setIsImporting(false); + } + }, + [workspacePath, mutate] + ); // ─── Error state for actions ─────────────────────────────────── const [actionError, setActionError] = useState(null); @@ -398,6 +429,23 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { />
+ {/* Import success summary banner (#565) */} + {importSummary && ( +
+ Imported from GitHub: {importSummary} + +
+ )} + {/* Action error banner */} {actionError && (
@@ -486,6 +534,7 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { open={importModalOpen} workspacePath={workspacePath} repo={ghStatus?.repo} + importing={isImporting} onClose={() => setImportModalOpen(false)} 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..8fe8c3ab 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,32 @@ 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; + }, + + // Close a GitHub issue (issue #565). + closeIssue: async ( + workspacePath: string, + issueNumber: number + ): Promise<{ success: boolean; issue_number: number }> => { + const response = await api.patch<{ success: boolean; issue_number: number }>( + `/api/v2/integrations/github/issues/${issueNumber}/close`, + {}, + { 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 From 245be38c2cbe495beb216546324d5f0604b12910 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 07:47:21 -0700 Subject: [PATCH 04/20] fix(github): repo-aware import dedupe + core-level auto-close (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address cross-family (codex) review: - P1: move GitHub issue auto-close from the HTTP PATCH path into core tasks.update_status (the single DONE chokepoint), so agent/batch (via runtime.complete_run) and CLI completions also close the linked issue. Fire-and-forget on a daemon thread, fully guarded — mirrors the existing blockers/conductor webhook dispatch pattern. - P2: de-duplicate imports on the full issue URL (get_by_external_url) instead of the bare issue number, so the same number in a different connected repo is still importable. Import now fetches then de-dupes by html_url. --- codeframe/core/tasks.py | 86 +++++++++++-- .../ui/routers/github_integrations_v2.py | 11 +- codeframe/ui/routers/tasks_v2.py | 73 +---------- tests/core/test_task_github_traceability.py | 114 +++++++++++++++++- tests/ui/test_github_integrations_v2.py | 33 ++++- tests/ui/test_v2_routers_integration.py | 114 ++++-------------- 6 files changed, 247 insertions(+), 184 deletions(-) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index 2d674211..dcf9943e 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -5,8 +5,11 @@ 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 @@ -16,6 +19,8 @@ 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.""" @@ -182,18 +187,19 @@ def get(workspace: Workspace, task_id: str) -> Optional[Task]: return _row_to_task(row) -def get_by_github_issue_number( - workspace: Workspace, github_issue_number: int +def get_by_external_url( + workspace: Workspace, external_url: str ) -> Optional[Task]: - """Get a task previously imported from a given GitHub issue number. + """Get a task previously imported from a given external (issue) URL. - Used for duplicate-import protection (issue #565): if a task already exists - for an issue number in this workspace, the import is skipped rather than - creating a second task. + 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 - github_issue_number: GitHub issue number to look up + external_url: The issue's ``html_url`` to look up Returns: The matching Task if one exists, otherwise None @@ -204,10 +210,10 @@ def get_by_github_issue_number( """ 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 github_issue_number = ? + WHERE workspace_id = ? AND external_url = ? LIMIT 1 """, - (workspace.id, github_issue_number), + (workspace.id, external_url), ) row = cursor.fetchone() conn.close() @@ -376,9 +382,71 @@ 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 _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. Resolves the repo from the per-workspace integration + config and the PAT from the machine-wide credential store. + """ + if not task.auto_close_github_issue or task.github_issue_number is None: + return + try: + from codeframe.core.credentials import CredentialManager, CredentialProvider + from codeframe.core.github_integration_config import ( + load_github_integration_config, + ) + + cfg = load_github_integration_config(workspace) + pat = CredentialManager().get_credential(CredentialProvider.GIT_GITHUB) + if cfg is None or not pat: + logger.info( + "Skipping GitHub auto-close for issue #%s: no connection.", + task.github_issue_number, + ) + return + _close_issue_background(pat, cfg["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, + ) + + +def _close_issue_background(pat: str, repo: str, issue_number: int) -> None: + """Fire-and-forget the GitHub issue close on a daemon thread (#565).""" + + def _run() -> None: + try: + from codeframe.core.github_issues_service import close_issue + + asyncio.run( + close_issue( + pat, repo, issue_number, comment="Completed via CodeFRAME" + ) + ) + except Exception: # noqa: BLE001 - background best-effort + logger.warning( + "GitHub auto-close of issue #%s failed", issue_number, exc_info=True + ) + + threading.Thread( + target=_run, daemon=True, name=f"gh-autoclose-{issue_number}" + ).start() + + def update( workspace: Workspace, task_id: str, diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index dfbbe0d4..3c6b9ad9 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -422,16 +422,17 @@ async def import_issues( skipped: list[int] = [] for number in body.issue_numbers: - existing = tasks.get_by_github_issue_number(workspace, number) - if existing is not None: - skipped.append(number) - continue - try: issue = await get_issue(pat, repo, number) except GitHubConnectError as e: raise _map_github_error(e) + # De-dupe on the full issue URL (repo-aware): the same issue number in a + # different repo is a distinct issue and must still be importable. + if tasks.get_by_external_url(workspace, issue["html_url"]) is not None: + skipped.append(number) + continue + description = issue["body"] or "" if issue["labels"]: footer = "**Labels:** " + ", ".join(issue["labels"]) diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index 33d4d011..11349073 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -25,10 +25,6 @@ from codeframe.core.workspace import Workspace from codeframe.lib.rate_limiter import rate_limit_ai, rate_limit_standard from codeframe.core import runtime, tasks, conductor, streaming -from codeframe.core.credentials import CredentialManager, CredentialProvider -from codeframe.core.github_connect_service import GitHubConnectError -from codeframe.core.github_integration_config import load_github_integration_config -from codeframe.core.github_issues_service import close_issue from codeframe.core.runtime import RunStatus from codeframe.core.state_machine import TaskStatus from codeframe.ui.dependencies import get_v2_workspace @@ -39,52 +35,6 @@ router = APIRouter(prefix="/api/v2/tasks", tags=["tasks-v2"]) -def get_credential_manager() -> CredentialManager: - """Dependency: machine-wide CredentialManager (overridden in tests). - - Shares the ``CredentialProvider.GIT_GITHUB`` slot used by the Integrations - settings tab (#555/#563) — used here to resolve the PAT for GitHub - issue auto-close on task completion (#565). - """ - return CredentialManager() - - -async def _auto_close_github_issue( - workspace: Workspace, - manager: CredentialManager, - github_issue_number: int, -) -> None: - """Close the linked GitHub issue when its task is marked DONE (#565). - - Runs as a fire-and-forget background task. Resolves the repo from the - per-workspace integration config and the PAT from the credential store; a - missing connection (or any GitHub error) is logged and swallowed so the - task transition is never affected. - """ - try: - cfg = load_github_integration_config(workspace) - pat = manager.get_credential(CredentialProvider.GIT_GITHUB) - if cfg is None or not pat: - logger.info( - "Skipping GitHub auto-close for issue #%s: no connection.", - github_issue_number, - ) - return - await close_issue( - pat, cfg["repo"], github_issue_number, comment="Completed via CodeFRAME" - ) - except GitHubConnectError as e: - logger.warning( - "Auto-close of GitHub issue #%s failed: %s", github_issue_number, e - ) - except Exception as e: # noqa: BLE001 - must never break the task transition - logger.warning( - "Unexpected error auto-closing GitHub issue #%s: %s", - github_issue_number, - e, - ) - - # ============================================================================ # Request/Response Models # ============================================================================ @@ -327,9 +277,7 @@ async def update_task( request: Request, task_id: str, body: UpdateTaskRequest, - background_tasks: BackgroundTasks, workspace: Workspace = Depends(get_v2_workspace), - manager: CredentialManager = Depends(get_credential_manager), ) -> TaskResponse: """Update a task's title, description, priority, or status. @@ -350,13 +298,11 @@ async def update_task( - 400: Invalid status or status transition """ try: - transitioned_to_done = False # Handle status update separately if provided if body.status: try: new_status = TaskStatus(body.status.upper()) tasks.update_status(workspace, task_id, new_status) - transitioned_to_done = new_status == TaskStatus.DONE except ValueError as e: if "Invalid status" in str(e) or "not a valid" in str(e).lower(): raise HTTPException( @@ -382,27 +328,14 @@ async def update_task( priority=body.priority, ) - # Persist the GitHub auto-close preference if provided (#565). + # Persist the GitHub auto-close preference if provided (#565). The + # actual issue close fires from core on the DONE transition + # (tasks.update_status), so it applies to CLI/agent completions too. if body.auto_close_github_issue is not None: task = tasks.update_auto_close( workspace, task_id, body.auto_close_github_issue ) - # On a DONE transition, fire-and-forget the GitHub issue close when the - # task opted in and is linked to an issue (#565). Failures are swallowed - # inside the background task — the transition has already succeeded. - if ( - transitioned_to_done - and task.auto_close_github_issue - and task.github_issue_number is not None - ): - background_tasks.add_task( - _auto_close_github_issue, - workspace, - manager, - task.github_issue_number, - ) - return TaskResponse.from_task(task) except ValueError as e: diff --git a/tests/core/test_task_github_traceability.py b/tests/core/test_task_github_traceability.py index 051142f1..556d193b 100644 --- a/tests/core/test_task_github_traceability.py +++ b/tests/core/test_task_github_traceability.py @@ -9,6 +9,7 @@ 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 @@ -65,26 +66,127 @@ def test_github_fields_present_in_list(self, workspace): assert by_id[imported.id].external_url.endswith("/issues/99") -class TestGetByGithubIssueNumber: +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 + workspace, title="Imported", github_issue_number=123, external_url=url ) - found = tasks.get_by_github_issue_number(workspace, 123) + 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_github_issue_number(workspace, 555) is None + 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.""" - tasks.create(workspace, title="Imported", github_issue_number=10) + 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_github_issue_number(other, 10) is None + 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).""" + + def test_done_dispatches_when_opted_in(self, workspace, 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)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + from codeframe.core.github_integration_config import ( + save_github_integration_config, + ) + + save_github_integration_config( + workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + ) + + 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_does_not_dispatch_when_not_opted_in(self, workspace, 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)), + ) + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + + task = tasks.create( + workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + auto_close_github_issue=False, + ) + tasks.update_status(workspace, task.id, TaskStatus.DONE) + assert calls == [] + + def test_done_skips_when_not_connected(self, workspace, 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)), + ) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + # No github config saved and no PAT → nothing dispatched. + task = tasks.create( + workspace, + title="Imported", + status=TaskStatus.IN_PROGRESS, + github_issue_number=99, + auto_close_github_issue=True, + ) + tasks.update_status(workspace, task.id, TaskStatus.DONE) + assert calls == [] class TestUpdateAutoClose: diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index b9763fe9..85ea4ead 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -456,11 +456,14 @@ def test_imports_create_tasks(self, client, monkeypatch, workspace): # Tasks really exist in the workspace with the right linkage. from codeframe.core import tasks - t12 = tasks.get_by_github_issue_number(workspace, 12) + 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) @@ -473,7 +476,9 @@ def test_labels_appended_to_description(self, client, monkeypatch, workspace): ) from codeframe.core import tasks - t = tasks.get_by_github_issue_number(workspace, 5) + 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): @@ -499,6 +504,30 @@ def test_duplicate_import_is_skipped(self, client, monkeypatch, workspace): ] assert len(matching) == 1 + 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 + class TestCloseIssueEndpoint: def test_requires_connection(self, client): diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index cec89168..e8ad3b94 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1207,111 +1207,41 @@ def test_patch_sets_auto_close(self, test_client): assert r.json()["auto_close_github_issue"] is True assert tasks.get(test_client.workspace, task.id).auto_close_github_issue is True - def _build_client(self, workspace): - """A tasks_v2 client with an isolated credential manager holding a PAT.""" - import tempfile as _tf - from pathlib import Path as _Path - - from codeframe.core.credentials import ( - CredentialManager, - CredentialProvider, - CredentialStore, - ) + 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.ui.dependencies import get_v2_workspace - from codeframe.ui.routers import tasks_v2 - - save_github_integration_config( - workspace, - {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, - ) - store = CredentialStore(storage_dir=_Path(_tf.mkdtemp())) - store._keyring_available = False - manager = CredentialManager.__new__(CredentialManager) - manager._store = store - manager.set_credential(CredentialProvider.GIT_GITHUB, "ghp_token") - - app = FastAPI() - app.include_router(tasks_v2.router) - app.dependency_overrides[get_v2_workspace] = lambda: workspace - app.dependency_overrides[tasks_v2.get_credential_manager] = lambda: manager - client = TestClient(app) - client.workspace = workspace - return client - - def test_done_transition_triggers_auto_close(self, test_workspace, monkeypatch): - from codeframe.core import tasks from codeframe.core.state_machine import TaskStatus - from codeframe.ui.routers import tasks_v2 calls = [] - - async def fake_close(pat, repo, number, **kwargs): - calls.append((repo, number)) - return True - - monkeypatch.setattr(tasks_v2, "close_issue", fake_close) - - client = self._build_client(test_workspace) - task = tasks.create( - test_workspace, - title="Imported", - status=TaskStatus.IN_PROGRESS, - github_issue_number=99, - auto_close_github_issue=True, + monkeypatch.setattr( + tasks, + "_close_issue_background", + lambda pat, repo, number: calls.append((repo, number)), ) - r = client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) - assert r.status_code == 200 - assert calls == [("acme/app", 99)] - - def test_done_without_auto_close_does_not_close(self, test_workspace, monkeypatch): - from codeframe.core import tasks - from codeframe.core.state_machine import TaskStatus - from codeframe.ui.routers import tasks_v2 - - calls = [] - - async def fake_close(pat, repo, number, **kwargs): - calls.append(number) - return True - - monkeypatch.setattr(tasks_v2, "close_issue", fake_close) - - client = self._build_client(test_workspace) - task = tasks.create( - test_workspace, - title="Imported, no auto-close", - status=TaskStatus.IN_PROGRESS, - github_issue_number=99, - auto_close_github_issue=False, + monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") + save_github_integration_config( + test_client.workspace, + {"repo": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, ) - r = client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) - assert r.status_code == 200 - assert calls == [] - def test_done_auto_close_failure_does_not_break_transition( - self, test_workspace, monkeypatch - ): - from codeframe.core import tasks - from codeframe.core.github_connect_service import GitHubConnectError - from codeframe.core.state_machine import TaskStatus - from codeframe.ui.routers import tasks_v2 - - async def boom(pat, repo, number, **kwargs): - raise GitHubConnectError("GitHub down") - - monkeypatch.setattr(tasks_v2, "close_issue", boom) - - client = self._build_client(test_workspace) task = tasks.create( - test_workspace, + 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 = client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) + r = test_client.patch(f"/api/v2/tasks/{task.id}", json={"status": "DONE"}) assert r.status_code == 200 - assert tasks.get(test_workspace, task.id).status == TaskStatus.DONE + assert calls == [("acme/app", 99)] + assert tasks.get(test_client.workspace, task.id).status == TaskStatus.DONE From b333b0385bf28d39ca0c3e5313bf786e15b9cf9e Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 07:51:47 -0700 Subject: [PATCH 05/20] fix(github): close issue on combined DONE+optin PATCH; non-daemon close (#565) Address second codex review pass: - Persist auto_close_github_issue before the status transition in update_task so a single PATCH {status: DONE, auto_close_github_issue: true} closes the issue (the core DONE dispatch now reads the fresh flag). - Run the auto-close thread non-daemon so a short-lived CLI process waits for the GitHub close at exit (bounded by the ~15s client timeout) instead of abandoning it. The server response still returns without joining. --- codeframe/core/tasks.py | 12 +++++-- codeframe/ui/routers/tasks_v2.py | 18 +++++------ tests/ui/test_v2_routers_integration.py | 42 +++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index dcf9943e..46c702d1 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -426,7 +426,15 @@ def _dispatch_github_autoclose(workspace: Workspace, task: Task) -> None: def _close_issue_background(pat: str, repo: str, issue_number: int) -> None: - """Fire-and-forget the GitHub issue close on a daemon thread (#565).""" + """Close the linked GitHub issue off the caller's path (#565). + + The close runs on a separate thread so it never blocks the task transition + (the FastAPI response returns immediately; the agent/CLI continues). The + thread is intentionally **non-daemon**: unlike a best-effort notification, + leaving the issue open is a real failure, so a short-lived CLI process must + wait for the close at interpreter exit rather than abandoning it. The wait + is bounded by the GitHub client timeout (~15s). + """ def _run() -> None: try: @@ -443,7 +451,7 @@ def _run() -> None: ) threading.Thread( - target=_run, daemon=True, name=f"gh-autoclose-{issue_number}" + target=_run, daemon=False, name=f"gh-autoclose-{issue_number}" ).start() diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index 11349073..0b26db56 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -298,6 +298,13 @@ async def update_task( - 400: Invalid status or status transition """ try: + # Persist the GitHub auto-close preference BEFORE any status change, so a + # combined request ({status: "DONE", auto_close_github_issue: true}) + # closes the issue: the DONE transition (in core) reads the freshly-saved + # flag rather than the stale value. (#565) + if body.auto_close_github_issue is not None: + tasks.update_auto_close(workspace, task_id, body.auto_close_github_issue) + # Handle status update separately if provided if body.status: try: @@ -319,7 +326,8 @@ async def update_task( detail=api_error("Invalid status transition", ErrorCodes.INVALID_STATE, str(e)), ) - # Update other fields + # Update other fields (re-reads current state, including the auto-close + # flag persisted above, so the returned task reflects every change). task = tasks.update( workspace, task_id, @@ -328,14 +336,6 @@ async def update_task( priority=body.priority, ) - # Persist the GitHub auto-close preference if provided (#565). The - # actual issue close fires from core on the DONE transition - # (tasks.update_status), so it applies to CLI/agent completions too. - if body.auto_close_github_issue is not None: - task = tasks.update_auto_close( - workspace, task_id, body.auto_close_github_issue - ) - return TaskResponse.from_task(task) except ValueError as e: diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index e8ad3b94..42d6c6ef 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1245,3 +1245,45 @@ def test_done_via_http_reaches_core_autoclose(self, test_client, monkeypatch): 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)] From c940a4a62752fc0587c527795b085f6a14501dfc Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 08:00:30 -0700 Subject: [PATCH 06/20] fix(github): source-repo auto-close, transactional import, no PATCH side-effects (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third codex review pass: - P1: auto-close now targets the task's source repo parsed from external_url, not the workspace's current connection — completing an old imported task after reconnecting to a different repo no longer closes the wrong issue. - P2: import is two-phase (fetch+dedupe all, then create) so a mid-batch fetch failure leaves no partial tasks behind. - P2: update_task pre-validates the status transition before any mutation, so a rejected PATCH no longer persists the auto-close flag (and illegal transitions now return a proper 400). --- codeframe/core/tasks.py | 43 ++++++++--- .../ui/routers/github_integrations_v2.py | 18 ++++- codeframe/ui/routers/tasks_v2.py | 68 +++++++++++------ tests/core/test_task_github_traceability.py | 74 +++++++++++++------ tests/ui/test_github_integrations_v2.py | 31 ++++++++ tests/ui/test_v2_routers_integration.py | 24 ++++++ 6 files changed, 198 insertions(+), 60 deletions(-) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index 46c702d1..1d9d353d 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -14,6 +14,7 @@ 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 @@ -392,31 +393,55 @@ def update_status( 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 _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. Resolves the repo from the per-workspace integration - config and the PAT from the machine-wide credential store. + 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 - from codeframe.core.github_integration_config import ( - load_github_integration_config, - ) - cfg = load_github_integration_config(workspace) pat = CredentialManager().get_credential(CredentialProvider.GIT_GITHUB) - if cfg is None or not pat: + if not pat: logger.info( - "Skipping GitHub auto-close for issue #%s: no connection.", + "Skipping GitHub auto-close for issue #%s: no stored PAT.", task.github_issue_number, ) return - _close_issue_background(pat, cfg["repo"], task.github_issue_number) + _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", diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index 3c6b9ad9..7e8ec775 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -414,25 +414,35 @@ async def import_issues( 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 ``github_issue_number``. + 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) - created: list[ImportedTaskSummary] = [] skipped: list[int] = [] + to_create: list[tuple[int, dict]] = [] + # 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 GitHubConnectError as e: raise _map_github_error(e) - # De-dupe on the full issue URL (repo-aware): the same issue number in a - # different repo is a distinct issue and must still be importable. if tasks.get_by_external_url(workspace, issue["html_url"]) is not None: skipped.append(number) continue + to_create.append((number, issue)) + # Phase 2 — create the tasks. No awaits here, so nothing can fail partway. + created: list[ImportedTaskSummary] = [] + for number, issue in to_create: description = issue["body"] or "" if issue["labels"]: footer = "**Labels:** " + ", ".join(issue["labels"]) diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index 0b26db56..49cfdb6e 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -26,7 +26,7 @@ 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 TaskStatus, can_transition from codeframe.ui.dependencies import get_v2_workspace from codeframe.ui.response_models import api_error, ErrorCodes @@ -298,36 +298,56 @@ async def update_task( - 400: Invalid status or status transition """ try: - # Persist the GitHub auto-close preference BEFORE any status change, so a - # combined request ({status: "DONE", auto_close_github_issue: true}) - # closes the issue: the DONE transition (in core) reads the freshly-saved - # flag rather than the stale value. (#565) - if body.auto_close_github_issue is not None: - tasks.update_auto_close(workspace, task_id, body.auto_close_github_issue) - - # 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 — notably a persisted auto-close flag that + # would later close the issue unexpectedly. (#565) + 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( + f"Invalid status: {body.status}", + ErrorCodes.VALIDATION_ERROR, + f"Valid values: {[s.value for s in TaskStatus]}", + ), + ) + 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}", + ), + ) + if not can_transition(current.status, new_status): raise HTTPException( status_code=400, - detail=api_error("Invalid status transition", ErrorCodes.INVALID_STATE, str(e)), + detail=api_error( + "Invalid status transition", + ErrorCodes.INVALID_STATE, + f"Cannot transition from {current.status.value} to " + f"{new_status.value}", + ), ) - # Update other fields (re-reads current state, including the auto-close - # flag persisted above, so the returned task reflects every change). + # Validations passed — persist the auto-close preference first so a + # combined {status: DONE, auto_close_github_issue: true} request closes + # the issue (the DONE transition in core reads the freshly-saved flag). + if body.auto_close_github_issue is not None: + tasks.update_auto_close(workspace, task_id, body.auto_close_github_issue) + + # Apply the already-validated status transition. + if new_status is not None: + tasks.update_status(workspace, task_id, new_status) + + # 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, diff --git a/tests/core/test_task_github_traceability.py b/tests/core/test_task_github_traceability.py index 556d193b..fba54be7 100644 --- a/tests/core/test_task_github_traceability.py +++ b/tests/core/test_task_github_traceability.py @@ -116,7 +116,8 @@ def test_scoped_to_workspace(self, workspace, tmp_path): class TestAutoCloseDispatch: """update_status fires the GitHub auto-close on DONE for all callers (#565).""" - def test_done_dispatches_when_opted_in(self, workspace, monkeypatch): + @staticmethod + def _record_calls(monkeypatch): from codeframe.core import tasks as tasks_mod calls = [] @@ -125,36 +126,51 @@ def test_done_dispatches_when_opted_in(self, workspace, monkeypatch): "_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) + 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": "acme/app", "owner_login": "acme", "owner_avatar_url": ""}, + {"repo": "newowner/newrepo", "owner_login": "newowner", + "owner_avatar_url": ""}, ) - task = tasks.create( workspace, - title="Imported", + title="From old repo", status=TaskStatus.IN_PROGRESS, - github_issue_number=99, - external_url="https://github.com/acme/app/issues/99", + 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", 99)] + assert calls == [("acme/app", 42)] def test_done_does_not_dispatch_when_not_opted_in(self, workspace, 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)), - ) + calls = self._record_calls(monkeypatch) monkeypatch.setenv("GITHUB_TOKEN", "ghp_token") task = tasks.create( @@ -162,27 +178,39 @@ def test_done_does_not_dispatch_when_not_opted_in(self, workspace, monkeypatch): 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_not_connected(self, workspace, monkeypatch): - from codeframe.core import tasks as tasks_mod + 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 == [] - 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( - tasks_mod, - "_close_issue_background", - lambda pat, repo, number: calls.append((repo, number)), + "codeframe.core.credentials.CredentialManager.get_credential", + lambda self, provider, name=None: None, ) - monkeypatch.delenv("GITHUB_TOKEN", raising=False) - # No github config saved and no PAT → nothing dispatched. 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) diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index 85ea4ead..a70f2ced 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -504,6 +504,37 @@ def test_duplicate_import_is_skipped(self, client, monkeypatch, workspace): ] assert len(matching) == 1 + 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 ): diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index 42d6c6ef..5302d21b 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1287,3 +1287,27 @@ def test_combined_done_and_optin_in_one_request_closes( ) assert r.status_code == 200 assert calls == [("acme/app", 77)] + + 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 + ) From 7da22fec00ff781834839bbe83aec16929882e85 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 08:07:03 -0700 Subject: [PATCH 07/20] fix(github): dedupe payload, reject PRs, drop unused close endpoint (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth codex review pass: - De-dupe issue numbers within a single /import payload so a repeated number creates one task (was: one per occurrence). - Reject pull-request numbers in get_issue (NotAnIssueError -> 422), matching list_issues which already excludes PRs — a PR can no longer be imported as a task. - Remove the unused standalone PATCH /issues/{n}/close endpoint + its frontend client. It was dead code and, lacking task context, would close the wrong repo's issue after a reconnect. Auto-close runs through the core dispatch, which correctly targets each task's source repo (external_url). --- codeframe/core/github_issues_service.py | 14 ++++ .../ui/routers/github_integrations_v2.py | 44 +++++------ tests/core/test_github_issues_service.py | 25 ++++++- tests/ui/test_github_integrations_v2.py | 74 ++++++++++--------- web-ui/src/lib/api.ts | 12 --- 5 files changed, 96 insertions(+), 73 deletions(-) diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index 5026dac7..0f63f2cd 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -37,6 +37,14 @@ _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. + """ + # 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"') @@ -247,6 +255,12 @@ async def 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)) diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index 7e8ec775..34b8f26d 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -29,7 +29,11 @@ parse_repo, validate_connection, ) -from codeframe.core.github_issues_service import close_issue, get_issue, list_issues +from codeframe.core.github_issues_service import ( + NotAnIssueError, + get_issue, + list_issues, +) from codeframe.core.github_integration_config import ( clear_github_integration_config, load_github_integration_config, @@ -107,11 +111,6 @@ class ImportResponse(BaseModel): total_created: int -class CloseIssueResponse(BaseModel): - success: bool - issue_number: 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 @@ -426,18 +425,30 @@ async def import_issues( 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 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 GitHubConnectError as e: raise _map_github_error(e) - if tasks.get_by_external_url(workspace, issue["html_url"]) is not None: + 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. No awaits here, so nothing can fail partway. @@ -464,22 +475,3 @@ async def import_issues( return ImportResponse( created=created, skipped=skipped, total_created=len(created) ) - - -@router.patch("/issues/{number}/close", response_model=CloseIssueResponse) -@rate_limit_standard() -async def close_github_issue( - request: Request, - number: int, - workspace: Workspace = Depends(get_v2_workspace), - manager: CredentialManager = Depends(get_credential_manager), -) -> CloseIssueResponse: - """Close a GitHub issue (issue #565), posting a completion comment.""" - repo, pat = _require_connection(workspace, manager) - - try: - await close_issue(pat, repo, number, comment="Completed via CodeFRAME") - except GitHubConnectError as e: - raise _map_github_error(e) - - return CloseIssueResponse(success=True, issue_number=number) diff --git a/tests/core/test_github_issues_service.py b/tests/core/test_github_issues_service.py index 04763fad..43e49264 100644 --- a/tests/core/test_github_issues_service.py +++ b/tests/core/test_github_issues_service.py @@ -230,10 +230,33 @@ def handler(request: httpx.Request) -> httpx.Response: # ───────────────────────────────────────────────────────────────────────────── from codeframe.core.github_connect_service import GitHubConnectError # noqa: E402 -from codeframe.core.github_issues_service import close_issue, get_issue # noqa: E402 +from codeframe.core.github_issues_service import ( # noqa: E402 + 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: diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index a70f2ced..76955051 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -413,19 +413,6 @@ async def fake(pat, repo, number, **kwargs): monkeypatch.setattr(github_integrations_v2, "get_issue", fake) -def _mock_close_issue(monkeypatch, *, calls=None, exc=None): - from codeframe.ui.routers import github_integrations_v2 - - async def fake(pat, repo, number, **kwargs): - if calls is not None: - calls.append({"repo": repo, "number": number, **kwargs}) - if exc is not None: - raise exc - return True - - monkeypatch.setattr(github_integrations_v2, "close_issue", fake) - - class TestImport: def test_requires_connection(self, client): r = client.post( @@ -504,6 +491,46 @@ def test_duplicate_import_is_skipped(self, client, monkeypatch, workspace): ] 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_fetch_failure_creates_no_tasks(self, client, monkeypatch, workspace): """A mid-batch fetch error aborts cleanly — no partial tasks created.""" _connect(client, monkeypatch) @@ -560,24 +587,3 @@ def test_same_issue_number_different_repo_is_not_skipped( assert {"acme 12", "other 12"} <= titles -class TestCloseIssueEndpoint: - def test_requires_connection(self, client): - r = client.patch("/api/v2/integrations/github/issues/5/close") - assert r.status_code == 409 - - def test_closes_issue(self, client, monkeypatch): - _connect(client, monkeypatch) - calls = [] - _mock_close_issue(monkeypatch, calls=calls) - r = client.patch("/api/v2/integrations/github/issues/5/close") - assert r.status_code == 200 - assert r.json() == {"success": True, "issue_number": 5} - assert calls[0]["number"] == 5 - - def test_invalid_token_maps_to_401(self, client, monkeypatch): - _connect(client, monkeypatch) - from codeframe.core.github_connect_service import InvalidTokenError - - _mock_close_issue(monkeypatch, exc=InvalidTokenError("bad")) - r = client.patch("/api/v2/integrations/github/issues/5/close") - assert r.status_code == 401 diff --git a/web-ui/src/lib/api.ts b/web-ui/src/lib/api.ts index 8fe8c3ab..a61b6bb1 100644 --- a/web-ui/src/lib/api.ts +++ b/web-ui/src/lib/api.ts @@ -1069,18 +1069,6 @@ export const integrationsApi = { return response.data; }, - // Close a GitHub issue (issue #565). - closeIssue: async ( - workspacePath: string, - issueNumber: number - ): Promise<{ success: boolean; issue_number: number }> => { - const response = await api.patch<{ success: boolean; issue_number: number }>( - `/api/v2/integrations/github/issues/${issueNumber}/close`, - {}, - { params: { workspace_path: workspacePath } } - ); - return response.data; - }, }; // Cost analytics API (issues #557, #558) From a123a8c6bfa9077d32f2be013ac47f595414d096 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 08:18:37 -0700 Subject: [PATCH 08/20] fix(github): context-aware auto-close + atomic import dedupe (#565) Fifth codex review pass: - Auto-close is now context-aware (mirrors WebhookNotificationService): in the server it schedules on the running event loop (no thread to join at shutdown); only in sync/CLI contexts does it use a non-daemon thread so the close completes before the process exits. Bounded by a 10s timeout so a hung GitHub call never stalls a CLI for long. Resolves the daemon-vs-completion tension from the prior pass. - Add a UNIQUE index on tasks(workspace_id, external_url) so duplicate-import protection is atomic: concurrent imports (double-submit / two tabs) can no longer both create a task for the same issue. The import catches the IntegrityError and records the loser as skipped. --- codeframe/core/github_issues_service.py | 5 +- codeframe/core/tasks.py | 71 ++++++++++++------- codeframe/core/workspace.py | 13 ++++ .../ui/routers/github_integrations_v2.py | 24 +++++-- tests/core/test_task_github_traceability.py | 17 +++++ 5 files changed, 97 insertions(+), 33 deletions(-) diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index 0f63f2cd..911727a3 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -285,6 +285,7 @@ async def close_issue( 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). @@ -294,6 +295,8 @@ async def close_issue( 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. @@ -310,7 +313,7 @@ async def close_issue( own_client = client is None if own_client: - client = httpx.AsyncClient(timeout=_TIMEOUT) + client = httpx.AsyncClient(timeout=timeout) try: headers = _headers(pat) base = f"{GITHUB_API_BASE}/repos/{owner}/{name}/issues/{number}" diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index 1d9d353d..bb825c81 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -450,34 +450,55 @@ def _dispatch_github_autoclose(workspace: Workspace, task: Task) -> None: ) -def _close_issue_background(pat: str, repo: str, issue_number: int) -> None: - """Close the linked GitHub issue off the caller's path (#565). - - The close runs on a separate thread so it never blocks the task transition - (the FastAPI response returns immediately; the agent/CLI continues). The - thread is intentionally **non-daemon**: unlike a best-effort notification, - leaving the issue open is a real failure, so a short-lived CLI process must - wait for the close at interpreter exit rather than abandoning it. The wait - is bounded by the GitHub client timeout (~15s). - """ +# 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 - def _run() -> None: - try: - from codeframe.core.github_issues_service import close_issue - asyncio.run( - close_issue( - pat, repo, issue_number, comment="Completed via CodeFRAME" - ) - ) - except Exception: # noqa: BLE001 - background best-effort - logger.warning( - "GitHub auto-close of issue #%s failed", issue_number, exc_info=True - ) +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 + ) - threading.Thread( - target=_run, daemon=False, name=f"gh-autoclose-{issue_number}" - ).start() + +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( diff --git a/codeframe/core/workspace.py b/codeframe/core/workspace.py index f80560ef..adf372aa 100644 --- a/codeframe/core/workspace.py +++ b/codeframe/core/workspace.py @@ -370,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)") @@ -534,6 +541,12 @@ def _ensure_schema_upgrades(db_path: Path) -> None: "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 34b8f26d..32b85a7a 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 @@ -459,13 +460,22 @@ async def import_issues( footer = "**Labels:** " + ", ".join(issue["labels"]) description = f"{description}\n\n{footer}" if description else footer - task = tasks.create( - workspace, - title=issue["title"], - description=description, - github_issue_number=number, - external_url=issue["html_url"], - ) + 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.append( ImportedTaskSummary( task_id=task.id, issue_number=number, title=task.title diff --git a/tests/core/test_task_github_traceability.py b/tests/core/test_task_github_traceability.py index fba54be7..8383ad9d 100644 --- a/tests/core/test_task_github_traceability.py +++ b/tests/core/test_task_github_traceability.py @@ -66,6 +66,23 @@ def test_github_fields_present_in_list(self, workspace): 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" From 721779b2c06368eaf7904a32bcef85e384758905 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 08:24:31 -0700 Subject: [PATCH 09/20] fix(github): best-effort close comment + close on late opt-in (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sixth codex review pass: - close_issue: posting the completion comment is now best-effort. A locked issue or comment-disabled repo no longer aborts before the close — the close (the operation that matters) always proceeds. - update_task: enabling auto-close on a task that is ALREADY DONE now closes the issue immediately (new tasks.autoclose_if_done helper), instead of silently doing nothing until some future re-completion. --- codeframe/core/github_issues_service.py | 15 ++++-- codeframe/core/tasks.py | 12 +++++ codeframe/ui/routers/tasks_v2.py | 9 +++- tests/core/test_github_issues_service.py | 18 +++++++ tests/ui/test_v2_routers_integration.py | 63 ++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index 911727a3..e8344841 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -319,14 +319,23 @@ async def close_issue( 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", type(exc).__name__) - raise GitHubConnectError("Could not reach GitHub. Try again later.") - _raise_for_status(cresp.status_code, context="issue comment") + logger.warning( + "GitHub issue comment failed (%s); closing anyway.", + type(exc).__name__, + ) try: resp = await client.patch( diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index bb825c81..b31eca7e 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -411,6 +411,18 @@ def _repo_from_issue_url(url: Optional[str]) -> Optional[str]: 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). diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index 49cfdb6e..31ec3aec 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -340,7 +340,14 @@ async def update_task( # combined {status: DONE, auto_close_github_issue: true} request closes # the issue (the DONE transition in core reads the freshly-saved flag). if body.auto_close_github_issue is not None: - tasks.update_auto_close(workspace, task_id, body.auto_close_github_issue) + updated = tasks.update_auto_close( + workspace, task_id, body.auto_close_github_issue + ) + # If the user opts in on a task that is ALREADY done (no transition + # will occur below), close the issue now — otherwise it would stay + # open forever. No-op when the task isn't DONE or isn't opted in. + if body.auto_close_github_issue: + tasks.autoclose_if_done(workspace, updated) # Apply the already-validated status transition. if new_status is not None: diff --git a/tests/core/test_github_issues_service.py b/tests/core/test_github_issues_service.py index 43e49264..fee6a828 100644 --- a/tests/core/test_github_issues_service.py +++ b/tests/core/test_github_issues_service.py @@ -355,6 +355,24 @@ def handler(request: httpx.Request) -> httpx.Response: ("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 = [] diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index 5302d21b..b46fc2bb 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1288,6 +1288,69 @@ def test_combined_done_and_optin_in_one_request_closes( 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_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 a55198c9c9edc63878c779b343be684498947c98 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 08:36:32 -0700 Subject: [PATCH 10/20] fix(github): don't auto-close when reopening a DONE task in same PATCH (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seventh codex review pass: - The late opt-in close (for already-DONE tasks) now only fires when the PATCH is NOT also changing status. A combined {status: READY, auto_close: true} request reopens the task without closing the issue — previously it closed the issue while reopening the task (opposite of intent). Rebuttal (out of scope): auto-close resolves the PAT from the single machine-wide GIT_GITHUB credential (the #555/#563 model). After reconnecting a workspace to a different repo, closing an older imported task whose repo needs a different PAT may 403/404 (logged, best-effort). Per-repo credential storage is a broader change than #565; documented as a Known Limitation. --- codeframe/ui/routers/tasks_v2.py | 10 ++++--- tests/ui/test_v2_routers_integration.py | 37 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index 31ec3aec..03c7d821 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -343,10 +343,12 @@ async def update_task( updated = tasks.update_auto_close( workspace, task_id, body.auto_close_github_issue ) - # If the user opts in on a task that is ALREADY done (no transition - # will occur below), close the issue now — otherwise it would stay - # open forever. No-op when the task isn't DONE or isn't opted in. - if body.auto_close_github_issue: + # If the user opts in on a task that is ALREADY done, close the issue + # now — otherwise it would stay open forever. Only when this request + # is NOT also changing status: a request that transitions the task + # (e.g. DONE -> READY to reopen it) must not close the issue; the + # DONE-transition path below handles the close when appropriate. + if body.auto_close_github_issue and new_status is None: tasks.autoclose_if_done(workspace, updated) # Apply the already-validated status transition. diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index b46fc2bb..4240623e 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1323,6 +1323,43 @@ def test_optin_on_already_done_task_closes_now(self, test_client, monkeypatch): assert r.status_code == 200 assert calls == [("acme/app", 88)] + 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 e38fdd54ea9488bb6ac7f328b236fb8e17cd61b7 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 08:47:26 -0700 Subject: [PATCH 11/20] fix(github): 404 issue->client error; refresh failure not import failure (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eighth codex review pass: - get_issue now raises IssueNotFoundError on a 404 (distinct from upstream GitHubConnectError), and the import maps it to 404 instead of 502 — a stale/typo'd issue number is reported as a client error, not a server failure. - TaskBoardView: the post-import board refresh (mutate) runs after the import resolves and outside its try/catch, so a transient /api/v2/tasks refresh error no longer flips a successful import into a 'Failed to import' banner. --- codeframe/core/github_issues_service.py | 13 +++++++++++++ codeframe/ui/routers/github_integrations_v2.py | 7 +++++++ tests/core/test_github_issues_service.py | 5 +++-- tests/ui/test_github_integrations_v2.py | 18 ++++++++++++++++++ web-ui/src/components/tasks/TaskBoardView.tsx | 9 ++++++++- 5 files changed, 49 insertions(+), 3 deletions(-) diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index e8344841..068e36e3 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -45,6 +45,15 @@ class NotAnIssueError(Exception): 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"') @@ -250,6 +259,10 @@ async def get_issue( logger.warning("GitHub get issue failed: %s", type(exc).__name__) raise GitHubConnectError("Could not reach GitHub. Try again later.") + # A 404 means the issue number doesn't exist (stale/typo'd payload) — + # a client error, distinct from an upstream GitHub failure. + if resp.status_code == 404: + raise IssueNotFoundError(f"Issue #{number} was not found in '{repo}'.") _raise_for_status(resp.status_code, context="get issue") raw = resp.json() diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index 32b85a7a..08090d04 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -31,6 +31,7 @@ validate_connection, ) from codeframe.core.github_issues_service import ( + IssueNotFoundError, NotAnIssueError, get_issue, list_issues, @@ -440,6 +441,12 @@ async def import_issues( 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) diff --git a/tests/core/test_github_issues_service.py b/tests/core/test_github_issues_service.py index fee6a828..2361e218 100644 --- a/tests/core/test_github_issues_service.py +++ b/tests/core/test_github_issues_service.py @@ -231,6 +231,7 @@ def handler(request: httpx.Request) -> httpx.Response: 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, @@ -310,12 +311,12 @@ def handler(request: httpx.Request) -> httpx.Response: await get_issue(VALID_PAT, "acme/app", 1, client=client) @pytest.mark.asyncio - async def test_404_maps_to_connect_error(self): + async def test_404_maps_to_issue_not_found(self): 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): + with pytest.raises(IssueNotFoundError): await get_issue(VALID_PAT, "acme/app", 999, client=client) diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index 76955051..cd34074e 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -531,6 +531,24 @@ async def pr_get_issue(pat, repo, number, **kwargs): assert tasks.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) diff --git a/web-ui/src/components/tasks/TaskBoardView.tsx b/web-ui/src/components/tasks/TaskBoardView.tsx index 4635c820..a1576618 100644 --- a/web-ui/src/components/tasks/TaskBoardView.tsx +++ b/web-ui/src/components/tasks/TaskBoardView.tsx @@ -111,9 +111,11 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { setIsImporting(true); setActionError(null); setImportSummary(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`, @@ -124,13 +126,18 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { ); } setImportSummary(parts.join(' · ')); - await mutate(); } catch (err) { const apiErr = err as ApiError; setActionError(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] ); From 2c5379bde09a778d568dba437eafed738c97b31f Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 08:59:38 -0700 Subject: [PATCH 12/20] fix(github): disambiguate 404, atomic-safe PATCH ordering (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ninth codex review pass: - get_issue: a 404 on the issue endpoint now probes the repo to distinguish a genuinely missing issue (-> IssueNotFoundError -> 404) from a broken integration (renamed/deleted repo or rotated token -> connect/auth error), so a real auth/repo failure is no longer reported to the user as a stale issue number. The probe only runs on the 404 path. - update_task: apply the status transition BEFORE persisting the auto-close flag, so a concurrently-rejected transition (now a 409) leaves no persisted side effect. The explicit late-opt-in close now fires on a newly-enabled flag (false->true) when the task is DONE — double-fire-safe with the core DONE-transition close. --- codeframe/core/github_issues_service.py | 25 +++++++++++- codeframe/ui/routers/tasks_v2.py | 48 +++++++++++++++++------- tests/core/test_github_issues_service.py | 31 ++++++++++++++- 3 files changed, 87 insertions(+), 17 deletions(-) diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index 068e36e3..16780225 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -259,9 +259,30 @@ async def get_issue( logger.warning("GitHub get issue failed: %s", type(exc).__name__) raise GitHubConnectError("Could not reach GitHub. Try again later.") - # A 404 means the issue number doesn't exist (stale/typo'd payload) — - # a client error, distinct from an upstream GitHub failure. + # 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." + ) + # Repo is reachable → the issue itself genuinely does not exist. raise IssueNotFoundError(f"Issue #{number} was not found in '{repo}'.") _raise_for_status(resp.status_code, context="get issue") diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index 03c7d821..05ccd9f0 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, can_transition +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 @@ -336,25 +340,41 @@ async def update_task( ), ) - # Validations passed — persist the auto-close preference first so a - # combined {status: DONE, auto_close_github_issue: true} request closes - # the issue (the DONE transition in core reads the freshly-saved flag). + # Apply the (pre-validated) status transition FIRST so the auto-close + # flag is never persisted ahead of a transition that could still be + # rejected by a concurrent state change — leaving no side effect on a + # failed request. When the task was already opted in, core closes the + # issue on this DONE transition. + if new_status is not None: + try: + tasks.update_status(workspace, task_id, new_status) + except InvalidTransitionError: + # Pre-validation passed, so this means the task state changed + # concurrently between the check and the apply. Nothing has been + # persisted yet — report a conflict. + raise HTTPException( + status_code=409, + detail=api_error( + "Task state changed concurrently; please retry.", + ErrorCodes.CONFLICT, + ), + ) + + # Persist the auto-close preference AFTER the status change has + # succeeded. When the flag is newly enabled (false -> true) and the task + # is now DONE, close the issue explicitly: core only auto-closes on the + # DONE transition for tasks that were ALREADY opted in, so a freshly + # enabled flag (late opt-in, or a combined {status: DONE, opt-in}) + # otherwise wouldn't fire. if body.auto_close_github_issue is not None: + before = tasks.get(workspace, task_id) + was_opted_in = before.auto_close_github_issue if before else False updated = tasks.update_auto_close( workspace, task_id, body.auto_close_github_issue ) - # If the user opts in on a task that is ALREADY done, close the issue - # now — otherwise it would stay open forever. Only when this request - # is NOT also changing status: a request that transitions the task - # (e.g. DONE -> READY to reopen it) must not close the issue; the - # DONE-transition path below handles the close when appropriate. - if body.auto_close_github_issue and new_status is None: + if body.auto_close_github_issue and not was_opted_in: tasks.autoclose_if_done(workspace, updated) - # Apply the already-validated status transition. - if new_status is not None: - tasks.update_status(workspace, task_id, new_status) - # Update remaining fields; re-reads current state so the returned task # reflects every change (including the auto-close flag above). task = tasks.update( diff --git a/tests/core/test_github_issues_service.py b/tests/core/test_github_issues_service.py index 2361e218..c038dd29 100644 --- a/tests/core/test_github_issues_service.py +++ b/tests/core/test_github_issues_service.py @@ -311,14 +311,43 @@ def handler(request: httpx.Request) -> httpx.Response: await get_issue(VALID_PAT, "acme/app", 1, client=client) @pytest.mark.asyncio - async def test_404_maps_to_issue_not_found(self): + 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_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 From 5a9b5f4c6a9e7eac24fc65395949427475d92e75 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 09:10:48 -0700 Subject: [PATCH 13/20] fix(github): roll back partial imports; malformed repo -> 409 (#565) Tenth codex review pass: - Import phase 2 now rolls back already-created tasks if a create fails with an unexpected DB error (e.g. OperationalError on a locked DB), preserving the all-or-nothing contract. (The URL-unique index still makes a retry idempotent.) - A malformed saved repo slug (parse_repo ValueError) now returns 409 from /import, consistent with the browse endpoint, instead of a 500. --- .../ui/routers/github_integrations_v2.py | 76 ++++++++++++------- tests/ui/test_github_integrations_v2.py | 54 +++++++++++++ 2 files changed, 103 insertions(+), 27 deletions(-) diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index 08090d04..de258ab3 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -434,6 +434,13 @@ async def import_issues( 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. @@ -459,35 +466,50 @@ async def import_issues( seen_urls.add(url) to_create.append((number, issue)) - # Phase 2 — create the tasks. No awaits here, so nothing can fail partway. + # 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] = [] - 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.append( - ImportedTaskSummary( - task_id=task.id, issue_number=number, title=task.title + 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 return ImportResponse( created=created, skipped=skipped, total_created=len(created) diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index cd34074e..00e46e07 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -531,6 +531,60 @@ async def pr_get_issue(pat, repo, number, **kwargs): assert tasks.list_tasks(workspace) == [] + 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 3d4945ed89716fc2117211b97b92a2ea4ae937a3 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 09:16:21 -0700 Subject: [PATCH 14/20] fix(github): honor opt-out on combined DONE PATCH (persist-first+rollback) (#565) Eleventh codex review pass: - Persist the auto-close flag BEFORE the status transition again, so the core DONE dispatch reads the value requested in THIS PATCH: {status: DONE, opt-out} no longer closes the issue (and {status: DONE, opt-in} still does). - Restore the pass-9 atomicity guarantee differently: if the transition is rejected by a concurrent change, roll the flag back to its original value (409), so a failed request leaves no side effect. - Late opt-in (false->true on an already-DONE task, no transition) still closes explicitly; double-fire and re-close are avoided by gating on (newly-enabled AND no transition in this request). --- codeframe/ui/routers/tasks_v2.py | 82 ++++++++++++++----------- tests/ui/test_v2_routers_integration.py | 39 ++++++++++++ 2 files changed, 85 insertions(+), 36 deletions(-) diff --git a/codeframe/ui/routers/tasks_v2.py b/codeframe/ui/routers/tasks_v2.py index 05ccd9f0..8a35461e 100644 --- a/codeframe/ui/routers/tasks_v2.py +++ b/codeframe/ui/routers/tasks_v2.py @@ -302,10 +302,22 @@ async def update_task( - 400: Invalid status or status transition """ try: - # 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 — notably a persisted auto-close flag that - # would later close the issue unexpectedly. (#565) + # 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: @@ -319,16 +331,6 @@ async def update_task( f"Valid values: {[s.value for s in TaskStatus]}", ), ) - 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}", - ), - ) if not can_transition(current.status, new_status): raise HTTPException( status_code=400, @@ -340,18 +342,31 @@ async def update_task( ), ) - # Apply the (pre-validated) status transition FIRST so the auto-close - # flag is never persisted ahead of a transition that could still be - # rejected by a concurrent state change — leaving no side effect on a - # failed request. When the task was already opted in, core closes the - # issue on this DONE transition. + # 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 this means the task state changed - # concurrently between the check and the apply. Nothing has been - # persisted yet — report a conflict. + # 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( @@ -360,20 +375,15 @@ async def update_task( ), ) - # Persist the auto-close preference AFTER the status change has - # succeeded. When the flag is newly enabled (false -> true) and the task - # is now DONE, close the issue explicitly: core only auto-closes on the - # DONE transition for tasks that were ALREADY opted in, so a freshly - # enabled flag (late opt-in, or a combined {status: DONE, opt-in}) - # otherwise wouldn't fire. - if body.auto_close_github_issue is not None: - before = tasks.get(workspace, task_id) - was_opted_in = before.auto_close_github_issue if before else False - updated = tasks.update_auto_close( - workspace, task_id, body.auto_close_github_issue - ) - if body.auto_close_github_issue and not was_opted_in: - tasks.autoclose_if_done(workspace, updated) + # 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 remaining fields; re-reads current state so the returned task # reflects every change (including the auto-close flag above). diff --git a/tests/ui/test_v2_routers_integration.py b/tests/ui/test_v2_routers_integration.py index 4240623e..57458c80 100644 --- a/tests/ui/test_v2_routers_integration.py +++ b/tests/ui/test_v2_routers_integration.py @@ -1323,6 +1323,45 @@ def test_optin_on_already_done_task_closes_now(self, test_client, monkeypatch): 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 52e24749b7e3c0d391df9398b863af3164471481 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 09:26:35 -0700 Subject: [PATCH 15/20] fix(github): in-modal import errors + invalidate issue cache (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twelfth codex review pass: - Import failures now render inline INSIDE the import modal (new importError prop) and keep it open, so the error is visible and the selection is preserved for a retry — previously a board-level banner sat hidden behind the open dialog. - After a successful import, the 60s issue-browse cache for the repo is invalidated, so reopening the modal no longer offers just-imported issues as selectable (they would now be skipped as duplicates). --- .../ui/routers/github_integrations_v2.py | 15 +++++++++++++ tests/ui/test_github_integrations_v2.py | 22 +++++++++++++++++++ .../tasks/GitHubIssueImportModal.test.tsx | 21 ++++++++++++++++++ .../tasks/TaskBoardView.import565.test.tsx | 3 +++ .../tasks/GitHubIssueImportModal.tsx | 14 ++++++++++++ web-ui/src/components/tasks/TaskBoardView.tsx | 13 +++++++++-- 6 files changed, 86 insertions(+), 2 deletions(-) diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index de258ab3..29d74083 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -136,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( @@ -511,6 +522,10 @@ async def import_issues( logger.warning("Failed to roll back imported task %s", task_id) raise + # Invalidate the browse cache so a re-open reflects the new duplicates. + if created: + _issue_cache_invalidate(repo) + return ImportResponse( created=created, skipped=skipped, total_created=len(created) ) diff --git a/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index 00e46e07..a4c3bf75 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -531,6 +531,28 @@ async def pr_get_issue(pat, repo, number, **kwargs): 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_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) 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 index 878537d6..39fb7493 100644 --- a/web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx +++ b/web-ui/src/__tests__/components/tasks/TaskBoardView.import565.test.tsx @@ -45,15 +45,18 @@ 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}
}
+ {/* Import error (kept in-modal so the selection is preserved) */} + {importError && ( +
+ + {importError} +
+ )} + {/* Footer: pagination + actions */}
diff --git a/web-ui/src/components/tasks/TaskBoardView.tsx b/web-ui/src/components/tasks/TaskBoardView.tsx index a1576618..7f4a951c 100644 --- a/web-ui/src/components/tasks/TaskBoardView.tsx +++ b/web-ui/src/components/tasks/TaskBoardView.tsx @@ -99,6 +99,7 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { const [importModalOpen, setImportModalOpen] = useState(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. @@ -111,6 +112,7 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { setIsImporting(true); setActionError(null); setImportSummary(null); + setImportError(null); let imported = false; try { const numbers = selectedIssues.map((i) => i.number); @@ -128,7 +130,10 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { setImportSummary(parts.join(' · ')); } catch (err) { const apiErr = err as ApiError; - setActionError(apiErr.detail || 'Failed to import issues from GitHub'); + // 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); } @@ -542,7 +547,11 @@ export function TaskBoardView({ workspacePath }: TaskBoardViewProps) { workspacePath={workspacePath} repo={ghStatus?.repo} importing={isImporting} - onClose={() => setImportModalOpen(false)} + importError={importError} + onClose={() => { + setImportModalOpen(false); + setImportError(null); + }} onImport={handleImportIssues} /> From 48082e5763c53b295d7358513281920d1f5b10cf Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 09:31:17 -0700 Subject: [PATCH 16/20] fix(github): invalidate browse cache on skipped-only imports (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thirteenth codex review pass: - Always invalidate the repo's issue-browse cache after an import, including all-skipped imports (issues created by another tab/process), so the modal never offers already-imported issues as selectable within the 60s TTL. Rebuttal (P2, daemon thread — re-raised across passes 3/5/13): the auto-close runs on a non-daemon thread in sync/CLI contexts BY DESIGN. For a CLI process to RELIABLY close the issue it must wait for the close; abandoning it (daemon=True) is a silent feature failure (the pass-3 finding). The wait is bounded by the 10s client timeout and is zero in the common case (fast GitHub completes before the command exits). Documented as a Known Limitation rather than reverted. --- .../ui/routers/github_integrations_v2.py | 7 ++-- tests/ui/test_github_integrations_v2.py | 33 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/codeframe/ui/routers/github_integrations_v2.py b/codeframe/ui/routers/github_integrations_v2.py index 29d74083..d2e62b2e 100644 --- a/codeframe/ui/routers/github_integrations_v2.py +++ b/codeframe/ui/routers/github_integrations_v2.py @@ -522,9 +522,10 @@ async def import_issues( logger.warning("Failed to roll back imported task %s", task_id) raise - # Invalidate the browse cache so a re-open reflects the new duplicates. - if created: - _issue_cache_invalidate(repo) + # 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/tests/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index a4c3bf75..f930276e 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -553,6 +553,39 @@ def test_import_invalidates_issue_cache(self, client, monkeypatch, workspace): 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 b229206da2587638a34abf761a4846ca270be602 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 09:35:33 -0700 Subject: [PATCH 17/20] fix(github): correct HTTP status edge cases (probe degradation, redirects) (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourteenth codex review pass: - get_issue 404 repo-probe: a degraded probe response (429/5xx) now raises GitHubConnectError instead of IssueNotFoundError, so a GitHub outage during import isn't misreported as a stale issue number. - close_issue: a 3xx redirect (moved/renamed/transferred repo) is now treated as a failure — httpx doesn't follow redirects, so the PATCH wasn't applied and the issue is still open; we no longer report a silent success. --- codeframe/core/github_issues_service.py | 24 +++++++++++++++++++++++- tests/core/test_github_issues_service.py | 24 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/codeframe/core/github_issues_service.py b/codeframe/core/github_issues_service.py index 16780225..6dc837d1 100644 --- a/codeframe/core/github_issues_service.py +++ b/codeframe/core/github_issues_service.py @@ -282,7 +282,20 @@ async def get_issue( raise GitHubConnectError( f"Repository '{repo}' is no longer accessible." ) - # Repo is reachable → the issue itself genuinely does not exist. + 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") @@ -380,6 +393,15 @@ async def close_issue( 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: diff --git a/tests/core/test_github_issues_service.py b/tests/core/test_github_issues_service.py index c038dd29..dc56cb1e 100644 --- a/tests/core/test_github_issues_service.py +++ b/tests/core/test_github_issues_service.py @@ -335,6 +335,19 @@ def handler(request: httpx.Request) -> httpx.Response: 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.""" @@ -423,3 +436,14 @@ def handler(request: httpx.Request) -> httpx.Response: 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) From 70a26304300027d53a1ceaf4c25d65baa9aca141 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 10:16:59 -0700 Subject: [PATCH 18/20] test: disable global rate limiter suite-wide to fix CI flakiness (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 AI bucket (connect/import = 20/min) is shared across the whole test suite and exhausts in the full v2 run — 429ing /connect and surfacing as spurious 409s in the GitHub import tests (green in isolation, red in CI). Disable the global default limiter once in the root conftest, BEFORE any router import, so every route imports unwrapped. Verified with the exact CI invocation (pytest tests/ --ignore=tests/e2e -m v2): 2949 passed. Rate-limit-specific tests build their own explicit Limiter and are unaffected; the per-file workaround is removed. --- tests/conftest.py | 11 +++++++++++ tests/ui/test_github_integrations_v2.py | 23 ----------------------- 2 files changed, 11 insertions(+), 23 deletions(-) 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/ui/test_github_integrations_v2.py b/tests/ui/test_github_integrations_v2.py index f930276e..3015b35c 100644 --- a/tests/ui/test_github_integrations_v2.py +++ b/tests/ui/test_github_integrations_v2.py @@ -25,29 +25,6 @@ VALID_PAT = "ghp_validtoken1234567890" -@pytest.fixture(autouse=True) -def _disable_rate_limiting(monkeypatch): - """Disable rate limiting around each test. - - The TestClient hits a fixed ``ip:testclient`` bucket, so AI-rate-limited - endpoints (connect/import) accumulate across the whole module and would - eventually 429. Disabling the limiter and resetting the cached config + - limiter singleton keeps each test isolated. - """ - from codeframe.config.rate_limits import _reset_rate_limit_config - from codeframe.core.config import reset_global_config - from codeframe.lib.rate_limiter import reset_rate_limiter - - monkeypatch.setenv("RATE_LIMIT_ENABLED", "false") - _reset_rate_limit_config() - reset_global_config() - reset_rate_limiter() - yield - _reset_rate_limit_config() - reset_global_config() - reset_rate_limiter() - - @pytest.fixture def workspace(): temp_dir = Path(tempfile.mkdtemp()) From 61baa352681cafaeae61acd7820cfb90dab3df29 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 10:19:15 -0700 Subject: [PATCH 19/20] refactor(github): try/finally in get_by_external_url + clarify test (#565) Address advisory review feedback: - Wrap the get_by_external_url DB connection in try/finally so it can't leak on a query error (matches the create/update_auto_close pattern). - Comment the GITHUB_TOKEN -> CredentialManager.get_credential resolution in the auto-close dispatch tests so the test intent is explicit. (Other advisory notes declined: in-place mutation in update_auto_close matches every sibling update_* in the module; a TODO comment on the context-aware dispatch is disallowed by project convention and the tradeoff is documented in the PR body.) --- codeframe/core/tasks.py | 26 +++++++++++---------- tests/core/test_task_github_traceability.py | 3 +++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index b31eca7e..0db062b1 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -206,18 +206,20 @@ def get_by_external_url( The matching Task if one exists, otherwise None """ conn = get_db_connection(workspace) - 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() - conn.close() + 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 diff --git a/tests/core/test_task_github_traceability.py b/tests/core/test_task_github_traceability.py index 8383ad9d..71b6f2df 100644 --- a/tests/core/test_task_github_traceability.py +++ b/tests/core/test_task_github_traceability.py @@ -147,6 +147,9 @@ def _record_calls(monkeypatch): 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( From a15fa9c3f18e5a7a9834353db3be7d70fd6c7dbb Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 4 Jun 2026 10:31:25 -0700 Subject: [PATCH 20/20] docs: mark Phase 5.5 (#565) GitHub issue import complete Update CLAUDE.md Current Focus: #565 (import execution + task traceability + auto-close) is implemented and merged-ready; document the endpoint behavior, URL-keyed dedup + unique index, core-level auto-close targeting the source repo, and the single-PAT known limitation. --- CLAUDE.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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.