Skip to content

feat(integrations): GitHub issue browser + multi-select import UI (#564)#608

Merged
frankbria merged 3 commits into
mainfrom
feat/564-github-issues-browser
Jun 2, 2026
Merged

feat(integrations): GitHub issue browser + multi-select import UI (#564)#608
frankbria merged 3 commits into
mainfrom
feat/564-github-issues-browser

Conversation

@frankbria
Copy link
Copy Markdown
Owner

@frankbria frankbria commented Jun 2, 2026

Closes #564.

What

Phase 5.5 — the GitHub issue browser that lets users select open issues from a connected repo before importing. The actual import action is out of scope here (lands in #565); selecting + confirming currently just closes the modal.

Plan adaptation

The Traycer plan on the issue targeted a superseded architecture (a new integrations_v2.py router reading GITHUB_TOKEN/GITHUB_REPO env vars via get_global_config()). The shipped #563 integration is different, so this was rewired to the real code:

  • Extends the existing github_integrations_v2.py router (prefix already /api/v2/integrations/github) instead of adding a new one.
  • PAT comes from CredentialManager (CredentialProvider.GIT_GITHUB); repo from .codeframe/github_integration.json (load_github_integration_config).
  • Reuses the existing /status endpoint + integrationsApi.getStatus to gate the button.

Backend

  • core/github_issues_service.py — headless list_issues():
    • Open issues only; pull requests filtered out (the /issues endpoint returns PRs too).
    • Label filter via labels=; free-text search routes to /search/issues with repo: + is:issue + is:open and reads total_count.
    • Pagination total from the Link header's rel="last" page.
    • Typed error mapping reusing [Phase 5.5] GitHub Issues import: repository connection (PAT auth) #563's error classes (401→InvalidTokenError, 403→InsufficientScopeError, 410 issues-disabled → empty).
  • GET /api/v2/integrations/github/issues?page&per_page&search&label on the existing router:
    • 409 when no repo is connected; per_page clamped to 1..100.
    • 60s in-process TTL cache keyed by the full query to avoid GitHub rate-limit pressure during paging.
    • PAT never returned.

Frontend

  • GitHubIssueImportModal — paginated list (25/page), debounced (300ms) title search, label filter, multi-select that persists across pages, select-all-on-page (indeterminate state), selected-count badge, skeleton loading, error banner. Import Selected disabled until ≥1 selected.
  • integrationsApi.getIssues + GitHubIssue/GitHubIssuesResponse types.
  • TaskBoardViewImport from GitHub button shown only when a repo is connected; opens the modal.

Drive-by fix

.gitignore's unanchored tasks/ rule also matched web-ui/src/components/tasks/, silently ignoring new source files there. Anchored to /tasks/.

Acceptance criteria

  • Issue list renders with pagination
  • Search and label filter work
  • Multi-select state persists across pages
  • [Import Selected] enabled only when ≥ 1 issue selected
  • npm test passes

Testing

  • Backend: tests/core/test_github_issues_service.py (9), tests/ui/test_github_integrations_v2.py (+8). uv run ruff check . clean.
  • Frontend: GitHubIssueImportModal.test.tsx (9), TaskBoardView.githubImport.test.tsx (2). Full suite 945 passing; npm run build succeeds.

Known limitations

Summary by CodeRabbit

  • New Features
    • Added GitHub issue browsing UI and import modal: paginated open-issue list, debounced title search, label filtering, per-page select-all, persistent multi-page selection, and an "Import Selected" action (modal shown only when GitHub is connected).
  • API
    • New backend endpoint to list open GitHub issues with caching, pagination, and error mappings.
  • Tests
    • Added unit and UI tests covering service, API, modal, task board flow, caching, and auth handling.
  • Documentation / Chores
    • Updated docs, .gitignore, and added an HTTP client dependency and a test icon mock.

Adds the issue-import browser that lets users select open issues from a
connected GitHub repo (the import action itself lands in #565).

Backend:
- core/github_issues_service.py: headless list_issues() — open issues only,
  PRs filtered out, label filter, /search/issues fallback for free-text,
  Link-header pagination, typed error mapping (401/403/410).
- GET /api/v2/integrations/github/issues on the existing #563 router, with a
  60s in-process TTL cache and a 409 when no repo is connected.

Frontend:
- GitHubIssueImportModal: paginated list, debounced search, label filter,
  multi-select that persists across pages, select-all-on-page (indeterminate),
  selected-count badge, Import Selected gated on >=1 selection.
- integrationsApi.getIssues + GitHub issue types.
- TaskBoardView: 'Import from GitHub' button gated on connection status.

Also fixes an unanchored .gitignore 'tasks/' rule that silently ignored
web-ui/src/components/tasks/ source files.

Tests: 9 service + 8 router (backend), 9 modal + 2 board (frontend). All green.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 091e7609-9bcf-494c-9ce4-18b38c9e6aa4

📥 Commits

Reviewing files that changed from the base of the PR and between 80012d3 and 016ebfc.

📒 Files selected for processing (1)
  • CLAUDE.md

Walkthrough

Adds a complete GitHub issue browser: backend service to list/search issues, a TTL-cached HTTP endpoint, frontend types and API client, a React modal for paginated multi-select import, TaskBoardView wiring, tests, and small repo/config updates.

Changes

GitHub Issue Browser Import Feature

Layer / File(s) Summary
Backend core service: list/search issues from GitHub
codeframe/core/github_issues_service.py, tests/core/test_github_issues_service.py
list_issues() validates owner/repo, builds auth headers, routes between REST /repos/{owner}/{repo}/issues and /search/issues based on search, normalizes results to GitHubIssue, filters pull requests, computes pagination totals from Link header or total_count, and maps HTTP/network errors to typed connector exceptions. Async pytest suite covers routing, params, filtering, pagination, and error mapping.
Backend HTTP endpoint with caching and error mapping
codeframe/ui/routers/github_integrations_v2.py, tests/ui/test_github_integrations_v2.py
GET /api/v2/integrations/github/issues validates workspace repo metadata and stored PAT (409 if missing), clamps per_page to 1–100, uses a 60s monotonic TTL cache keyed by repo+params, calls list_issues() on miss, maps service exceptions to HTTP statuses (401/403/502), returns Pydantic GitHubIssuesResponse, and is covered by endpoint tests including cache behavior and error mapping.
Frontend types and API client for issue fetching
web-ui/src/types/index.ts, web-ui/src/lib/api.ts
Adds GitHubIssue, GitHubIssuesResponse, GitHubIssuesParams TypeScript interfaces and integrationsApi.getIssues(workspacePath, params?) to call the backend issues endpoint with pagination, search, and label filters.
Frontend modal component: browse, filter, multi-select issues
web-ui/src/components/tasks/GitHubIssueImportModal.tsx, web-ui/src/__tests__/components/tasks/GitHubIssueImportModal.test.tsx
GitHubIssueImportModal (client) manages page, debounced title search, label filter, and selection Map to persist multi-page selections; fetches via SWR when open; renders select-all (indeterminate), selected-count badge, loading/error/empty states, per-row metadata and compact relative age, and Import/Cancel actions. RTL tests verify rendering, selection behavior, pagination persistence, error display, and SWR suspension when closed.
Frontend integration: TaskBoardView button and modal wiring
web-ui/src/components/tasks/TaskBoardView.tsx, web-ui/src/__tests__/components/tasks/TaskBoardView.githubImport.test.tsx
TaskBoardView fetches GitHub integration status via SWR, derives githubConnected to show/hide the "Import from GitHub" button (with GithubIcon), manages importModalOpen, and mounts GitHubIssueImportModal with workspacePath and repo. Tests verify visibility and modal opening.
Repo config and mocks
.gitignore, pyproject.toml, web-ui/__mocks__/@hugeicons/react.js
Anchors /tasks/ in .gitignore, adds httpx>=0.27.0 to project dependencies, and adds GithubIcon to the Jest mock used by the UI.

Sequence Diagram

sequenceDiagram
  participant User
  participant Modal
  participant SWR_integAPI
  participant BackendRouter
  participant list_issues
  participant GitHubAPI
  User->>Modal: open modal (workspace, repo)
  Modal->>SWR_integAPI: useSWR(key={workspace, page, per_page, search, label})
  SWR_integAPI->>BackendRouter: GET /api/v2/integrations/github/issues?workspace_path=...
  BackendRouter->>list_issues: list_issues(pat, repo, page, per_page, search, label)
  list_issues->>GitHubAPI: GET /search/issues or /repos/{owner}/{repo}/issues
  GitHubAPI-->>list_issues: issues[], Link header / total_count
  list_issues-->>BackendRouter: (items[], total)
  BackendRouter-->>SWR_integAPI: {items, total}
  SWR_integAPI-->>Modal: data / isLoading / error
  User->>Modal: select items / click Import Selected
  Modal-->>User: onImport(selectedIssues[])
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped to GitHub, nose a-gleam,
Fetched open issues, one, two, and three—
Checkboxes ticked, pages kept in store,
Import clicked, the tasks hop ashore.
A little rabbit cheer for PR glory!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the main feature: adding a GitHub issue browser UI and multi-select import capability.
Linked Issues check ✅ Passed The PR implementation fully addresses all coding requirements from #564: paginated issue list, search/label filtering, multi-select persistence, proper gating, backend endpoint with caching, and typed responses.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the GitHub issue browser feature. The .gitignore fix is a minor drive-by fix mentioned in objectives, and CLAUDE.md documentation update is appropriate.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/564-github-issues-browser

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

Code Review — Phase 5.5 GitHub Issue Browser (564): Well-scoped implementation, architecture compliant. Two items to fix before merge: (1) Add workspace path to issue cache key to prevent cross-workspace data leakage; (2) Strip double-quotes from label param to prevent GitHub search qualifier manipulation. Full review details to follow.

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted test

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

test plain string

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@codeframe/core/github_issues_service.py`:
- Line 25: The module codeframe/core/github_issues_service.py imports httpx but
httpx is not declared in project dependencies; add "httpx" to the project's
dependency list (e.g., under [project] dependencies in pyproject.toml or the
appropriate [project.optional-dependencies] group if intended optional) so
installs include it, then update the lockfile (poetry lock / pip-tools /
pip-compile) and CI/setup workflows to install the new dependency; locate the
missing import by checking the import line in github_issues_service.py to verify
the package name.

In `@web-ui/src/components/tasks/GitHubIssueImportModal.tsx`:
- Around line 314-331: The formatAge function mangles phrases like "less than a
minute" and leaves "almost " intact; update formatAge to special-case the exact
"less than a minute" result (return "<1m" or "less than 1m") before doing other
replacements, strip both "about " and "almost " prefixes, and perform the unit
replacements using word-boundary or plural-aware patterns (e.g. replace
/months?/ -> "mo", /days?/ -> "d", /years?/ -> "y", /hours?/ -> "h", /minutes?/
-> "m") so you don't accidentally alter words inside other phrases; keep these
changes inside the formatAge function that calls formatDistanceToNow.

In `@web-ui/src/components/tasks/TaskBoardView.tsx`:
- Line 13: The import of GithubIcon from '`@hugeicons/react`' in TaskBoardView.tsx
is incorrect because '`@hugeicons/react`' exports a renderer (HugeiconsIcon) while
individual icons live in separate packs; update the TaskBoardView.tsx usage to
import the actual icon (e.g., GithubIcon) from the icon pack (such as
'`@hugeicons/core-free-icons`') and render it via the HugeiconsIcon renderer using
the icon prop (or alternatively import and use the renderer and pass the core
icon to its icon prop where GithubIcon is currently expected), ensuring
Cancel01Icon and Task01Icon imports are similarly validated or replaced with the
correct source.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d84bf8d0-3a24-4e2b-acd2-c1aef953d78f

📥 Commits

Reviewing files that changed from the base of the PR and between 91ec698 and a313118.

📒 Files selected for processing (12)
  • .gitignore
  • codeframe/core/github_issues_service.py
  • codeframe/ui/routers/github_integrations_v2.py
  • tests/core/test_github_issues_service.py
  • tests/ui/test_github_integrations_v2.py
  • web-ui/__mocks__/@hugeicons/react.js
  • web-ui/src/__tests__/components/tasks/GitHubIssueImportModal.test.tsx
  • web-ui/src/__tests__/components/tasks/TaskBoardView.githubImport.test.tsx
  • web-ui/src/components/tasks/GitHubIssueImportModal.tsx
  • web-ui/src/components/tasks/TaskBoardView.tsx
  • web-ui/src/lib/api.ts
  • web-ui/src/types/index.ts

Comment thread codeframe/core/github_issues_service.py
Comment thread web-ui/src/components/tasks/GitHubIssueImportModal.tsx Outdated
import { BulkActionConfirmDialog, type BulkActionType } from './BulkActionConfirmDialog';
import { Cancel01Icon, Task01Icon } from '@hugeicons/react';
import { GitHubIssueImportModal } from './GitHubIssueImportModal';
import { Cancel01Icon, Task01Icon, GithubIcon } from '@hugeicons/react';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1) Show prior usage of any Github*Icon in the repo (is this an established import?)
rg -nP 'Github\w*Icon' -g '!**/__mocks__/**'
echo "----- hugeicons mock -----"
cat web-ui/__mocks__/@hugeicons/react.js
echo "----- installed export names (if available) -----"
fd -t f -p 'node_modules/@hugeicons/react' --exec sh -c 'grep -l "GithubIcon" "$1" 2>/dev/null' _ {} \; 2>/dev/null | head

Repository: frankbria/codeframe

Length of output: 3616


🌐 Web query:

@hugeicons/react 0.3.0 GithubIcon export name

💡 Result:

In the @hugeicons/react library, individual icons are not exported directly from the @hugeicons/react package. Instead, @hugeicons/react serves as a rendering library that provides the HugeiconsIcon component, while the icons themselves are exported from separate icon packages, such as @hugeicons/core-free-icons [1][2][3]. To use the GitHub icon, you should install the icon pack and import it from that package: 1. Install the required packages: npm install @hugeicons/react @hugeicons/core-free-icons [4][5]. 2. Import the icon and the renderer: import { HugeiconsIcon } from '@hugeicons/react'; import { GithubIcon } from '@hugeicons/core-free-icons'; 3. Use the component: Note that older versions of the library (such as the deprecated hugeicons-react package) followed a different structure, but the current official approach requires importing icons from the dedicated icon packs [4][3][5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

node - <<'NODE'
const path = require('path');

function tryRequire(mod) {
  try {
    const pkg = require(path.join(mod, 'package.json'));
    const m = require(mod);
    const keys = m && typeof m === 'object' ? Object.keys(m) : [];
    const githubKeys = keys.filter(k => /github/i.test(k));
    console.log(`== ${mod} ==`);
    console.log(`version: ${pkg.version}`);
    console.log(`export keys: ${githubKeys.length ? githubKeys.join(', ') : '(none matched /github/i)'}`);
    console.log(`has GithubIcon export: ${'GithubIcon' in m}`);
  } catch (e) {
    console.log(`== ${mod} ==`);
    console.log(String(e));
  }
}

tryRequire('`@hugeicons/react`');
tryRequire('`@hugeicons/core-free-icons`');
NODE

# Fallback: show if GithubIcon string exists in the installed package entrypoints
if [ -d "node_modules/@hugeicons/react" ]; then
  rg -n "GithubIcon" node_modules/@hugeicons/react --hidden || true
fi

Repository: frankbria/codeframe

Length of output: 333


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Check which hugeicons packages/versions are declared
ls -la web-ui | head
rg -n '"`@hugeicons/`' web-ui/package.json web-ui/package-lock.json web-ui/yarn.lock web-ui/pnpm-lock.yaml 2>/dev/null || true

# 2) Locate all Hugeicons/Github icon imports
rg -n 'from\s+[\'"]`@hugeicons/`(react|core-free-icons|[^\'"]+)[\'"]' web-ui/src --glob='*.{ts,tsx}' || true
rg -n 'GithubIcon' web-ui/src --glob='*.{ts,tsx}' || true
rg -n 'HugeiconsIcon' web-ui/src --glob='*.{ts,tsx}' || true

# 3) Check existing pattern for icons without numeric suffix
rg -n 'from\s+[\'"]`@hugeicons/react`[\'"]' web-ui/src/components --glob='*.{ts,tsx}' || true

Repository: frankbria/codeframe

Length of output: 830


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# package + lock (dependency versions / presence)
echo "== web-ui/package.json ==" && cat web-ui/package.json
echo "== web-ui/package-lock.json (hugeicons deps only) ==" 
rg -n '"`@hugeicons/`(react|core-free-icons)[^"]*"' web-ui/package-lock.json || true
rg -n '"`@hugeicons/`' web-ui/package-lock.json | head -n 80

# search import patterns
echo "== Imports from `@hugeicons/react` in web-ui/src ==" 
rg -n "from\s+['\"]`@hugeicons/react`['\"]" web-ui/src --glob='*.{ts,tsx}' || true

echo "== GithubIcon usage in web-ui/src (any import/source) ==" 
rg -n "GithubIcon" web-ui/src --glob='*.{ts,tsx}' || true

echo "== HugeiconsIcon usage in web-ui/src (renderer pattern) ==" 
rg -n "HugeiconsIcon" web-ui/src --glob='*.{ts,tsx}' || true

Repository: frankbria/codeframe

Length of output: 8777


🏁 Script executed:

#!/bin/bash
set -euo pipefail

if [ -d "web-ui/node_modules/@hugeicons/react" ]; then
  echo "Found web-ui/node_modules/@hugeicons/react"
  node - <<'NODE'
  try {
    const m = require('./web-ui/node_modules/@hugeicons/react');
    const keys = Object.keys(m || {});
    console.log('version:', require('./web-ui/node_modules/@hugeicons/react/package.json').version);
    console.log('has GithubIcon:', 'GithubIcon' in m);
    console.log('github-like keys:', keys.filter(k => /github/i.test(k)).join(', ') || '(none)');
    console.log('type of GithubIcon:', typeof m.GithubIcon);
  } catch (e) {
    console.log(String(e));
  }
NODE

  echo "Searching installed package for 'GithubIcon'..."
  rg -n "GithubIcon" web-ui/node_modules/@hugeicons/react -S || true
else
  echo "web-ui/node_modules/@hugeicons/react not present in sandbox"
fi

echo "Also check whether any `@hugeicons/react` is present anywhere else..."
ls -ლა web-ui/node_modules/@hugeicons 2>/dev/null || true

Repository: frankbria/codeframe

Length of output: 191


🌐 Web query:

Find in @hugeicons/react@0.3.0 source/index exports: does it export GithubIcon? (use unpkg/jsdelivr/npm view or GitHub tag)

💡 Result:

The @hugeicons/react package (version 0.3.0) is a rendering library designed specifically for displaying icons [1][2]. It does not export individual icon components like GithubIcon directly from its root [1][2]. Instead, the library provides a single HugeiconsIcon component [1][2]. To display an icon, you must install a separate icon pack (such as @hugeicons/core-free-icons) and pass the desired icon object to the icon prop of the HugeiconsIcon component [1][3]. Example usage: import { HugeiconsIcon } from '@hugeicons/react'; import { GithubIcon } from '@hugeicons/core-free-icons'; function App { return ; }

Citations:


🌐 Web query:

unpkg @hugeicons/react@0.3.0 entry file exports HugeiconsIcon; does it contain export { GithubIcon } or named exports for icons?

💡 Result:

unpkg @hugeicons/react@0.3.0 (root entry) exports only the HugeiconsIcon React component (named export). It does not export individual icon components like GithubIcon from the entry point. Evidence: the package documentation describes @hugeicons/react as a “rendering library” that provides the HugeiconsIcon component, while the actual icons come from separate icon packages such as @hugeicons/core-free-icons / @hugeicons-pro/core-* [1][2]. Example usage imports HugeiconsIcon from @hugeicons/react and SearchIcon (an icon component) from @hugeicons/core-free-icons [1][2]. Therefore, a statement like export { GithubIcon } or other named exports for icons at the root entry is not part of the intended API. How to import a specific icon: import the icon component (e.g., GithubIcon) from the appropriate icon pack package, then pass it to HugeiconsIcon as the icon prop. [1][2] (Note: your question mentions unpkg @hugeicons/react@0.3.0 entry file exports HugeiconsIcon; that matches the known API pattern for this library, but I did not find evidence of any GithubIcon root export in the cited docs.)

Citations:


Fix GithubIcon import from @hugeicons/react

web-ui/src/components/tasks/TaskBoardView.tsx imports GithubIcon (along with other *Icon named exports) from @hugeicons/react. In @hugeicons/react@0.3.0, the package is a renderer that exports HugeiconsIcon, while individual icons like GithubIcon are provided by separate icon packs (e.g. @hugeicons/core-free-icons) and must be passed via the icon prop; relying on GithubIcon being a root named export will likely be undefined at runtime (tests can be masked by web-ui/__mocks__/@hugeicons/react.js).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/src/components/tasks/TaskBoardView.tsx` at line 13, The import of
GithubIcon from '`@hugeicons/react`' in TaskBoardView.tsx is incorrect because
'`@hugeicons/react`' exports a renderer (HugeiconsIcon) while individual icons
live in separate packs; update the TaskBoardView.tsx usage to import the actual
icon (e.g., GithubIcon) from the icon pack (such as
'`@hugeicons/core-free-icons`') and render it via the HugeiconsIcon renderer using
the icon prop (or alternatively import and use the renderer and pass the core
icon to its icon prop where GithubIcon is currently expected), ensuring
Cancel01Icon and Task01Icon imports are similarly validated or replaced with the
correct source.

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

Line 1

line 2 with no hash
line 3 also fine

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted stdin test

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted proc test

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

- Declare httpx as an explicit dependency in pyproject.toml. It was only
  available transitively (via anthropic/openai/etc.); this module imports it
  directly, so make the dependency explicit (Major).
- Fix formatAge mangling: 'less than a minute' became 'less than am' and
  'almost 2 years' became 'almost 2y'. Switch to formatDistanceToNowStrict
  (no about/almost/less-than prefixes) + clean unit abbreviation (Minor).
@frankbria
Copy link
Copy Markdown
Owner Author

Thanks @coderabbitai — triaged all three:

  1. httpx not in pyproject.toml (Major) — confirmed: it was only available transitively (anthropic/openai/e2b/mcp). Since this module imports it directly, declared httpx>=0.27.0 explicitly. Fixed in 80012d3.
  2. formatAge mangles 'less than a minute' (Minor) — confirmed ('less than a minute''less than am', 'almost 2 years''almost 2y'). Rewrote using formatDistanceToNowStrict (no about/almost/less-than prefixes) + a clean unit-abbreviation map. Verified across seconds→years and empty/invalid input. Fixed in 80012d3.
  3. GithubIcon import (Major) — verified as a valid @hugeicons/react export (the production npm run build resolves and renders it; added to the jest icon mock). No change needed.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 2, 2026

Tip

For best results, initiate chat on the files or code changes.

@frankbria: Thanks for the thorough triage — all three look resolved:

  1. httpx in pyproject.toml — good call making it an explicit dependency rather than relying on transitive resolution. That's the right fix.
  2. formatAge rewriteformatDistanceToNowStrict is the correct choice here; it produces clean, prefix-free output that an abbreviation map can handle reliably. Appreciate the verification across the full seconds→years range.
  3. GithubIcon — confirmed, the mock addition and production build validation are sufficient evidence. No change needed.

🐇 ✨

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

3 similar comments
@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

deleted

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

Follow-up Review - Phase 5.5 GitHub Issue Browser (#564)

This follows up on my earlier summary comment. The httpx explicit dependency (flagged by coderabbitai) was correctly added in 80012d3. Here is where things stand on the two items I flagged.

Item 1: Cache key workspace scope - closing this out

My earlier concern about cross-workspace cache leakage is less critical than I stated. Since the PAT is machine-wide (CredentialProvider.GIT_GITHUB), all workspaces share the same credentials, and the cache key already includes repo which disambiguates different repositories. The only edge case - two workspaces pointing at the same repo - would share a cache entry, but with the same PAT that is correct behavior. No action needed.

Item 2: Label injection in search qualifier - still present

In codeframe/core/github_issues_service.py around line 231:

qualifiers.append(f'label:"{label.strip()}"')

If the user-supplied label contains a double-quote, the assembled q string will break out of the quoted qualifier. In this single-user context the user can only corrupt their own search, but the fix is one line:

clean_label = label.strip().replace('"', '')
qualifiers.append(f'label:"{clean_label}"')

Low severity but trivial to fix before merge.

Additional observation: _headers is a private symbol imported across modules

github_issues_service.py imports _headers (underscore-prefixed) from github_connect_service. This is a minor coupling smell - if github_connect_service ever refactors its internals, this import breaks silently. Worth considering whether to expose it as a public helper or consolidate the shared HTTP utilities. Not a blocker.

Overall

Implementation is clean and architecture-compliant: core service is headless, router layer is a thin adapter, frontend modal handles multi-select state correctly across pages, and test coverage is solid (9+8 backend, 9+2 frontend). Recommended: address the label double-quote stripping before merge (five-minute fix), then this is good to go.

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

Code Review — Phase 5.5 GitHub Issue Browser (#564)

This is a well-scoped and well-tested implementation. Architecture choices align correctly with project conventions (headless core, thin router layer, reuse of #563's credential/config infrastructure). Findings organized by severity below.

Security / Correctness

Cache key missing workspace isolation (github_integrations_v2.py, lines 372–374)

The cache key is keyed only on query parameters, not on the workspace path:

cache_key = f"{repo}|{page}|{per_page}|{search}|{label}"

If the server process handles multiple workspaces connecting to the same repo, workspace A's cached response will be served to workspace B. The PAT is also excluded from the key, so two tokens with different effective permissions would receive the same cached data.

In the current single-user product model this is low-risk, but it is a latent bug as multi-workspace usage grows. Recommend adding workspace.path to the cache key.

Label search injection into GitHub query (github_issues_service.py, lines 230–231)

A label value containing a double-quote character (e.g. some" OR repo:attacker/repo label:"other) would produce a manipulated GitHub search qualifier. The server's own security boundary is unaffected (this only touches GitHub's search), but a user could broaden their own search beyond the intended repo scope. Consider stripping or rejecting double-quotes from the label value before interpolation.


Code Quality

Importing a private function from another module (github_issues_service.py, line 52)

_headers is imported from github_connect_service by its underscore-prefixed private name. This creates coupling that could silently break on refactor. The helper could be promoted to a public function in github_connect_service (e.g., build_auth_headers) to make the shared API explicit.

Pagination total slightly off after PR filtering (github_issues_service.py, lines 209–210)

_total_from_link_header falls back to items_len (the post-filter count) only when there is no Link header (single-page results). When a Link header is present it ignores items_len — so PR filtering has no effect on the total, which is actually correct for multi-page repos. However, on a single-page repo containing mixed issues and PRs, the fallback total will be understated. This is a very edge case and the PR description already documents the estimate limitation, but a comment on this line would prevent future confusion.

Unbounded cache growth (github_integrations_v2.py)

The module-level _ISSUE_CACHE dict grows without active eviction. Stale entries are removed lazily only on a cache-hit read. For typical usage the cache stays small, but if search terms vary widely the dict accumulates indefinitely. A simple bounded size or a periodic sweep would address this. Not urgent, but worth a TODO comment.


Frontend

formatAge string replacements are brittle (GitHubIssueImportModal.tsx, around line 1410)

The series of .replace() calls post-processing formatDistanceToNow output handles common cases but misses some date-fns output variants (e.g., "over 1 year", "almost 2 years", "less than a minute"). Using date-fns/formatDuration + intervalToDuration for controlled output, or accepting the full date-fns string as fallback, would be more robust against date-fns version upgrades.

Import button spinner conflates fetch and action state (GitHubIssueImportModal.tsx, around line 1397)

isLoading reflects the SWR data fetch, not the import action. When the user navigates to a new page (triggering a new SWR fetch) while the Import button has active selections, it momentarily shows a spinner implying the import itself is in progress. A separate isImporting state in the real import (#565) will fix this naturally.


What's Done Well

  • Architecture compliance: github_issues_service.py is fully headless — no FastAPI imports, rule Add Claude Code GitHub Workflow #1 respected.
  • PAT is never returned in any response path, consistent with the [Phase 5.5] GitHub Issues import: repository connection (PAT auth) #563 pattern.
  • Cache is correctly bypassed for the connection check (409 fires before cache read).
  • per_page clamping at the router layer (not delegated to the service layer) is the right boundary.
  • SWR key set to null when modal is closed avoids background fetches.
  • Selection state persisting across page changes via Map<number, GitHubIssue> is the right data structure.
  • Test coverage is thorough: error mapping, PR filtering, cache TTL, cross-page selection, PAT-never-echoed — all covered.
  • The .gitignore anchoring fix is correct and well-commented.

Summary

Two items worth addressing before merge:

  1. Add workspace path to the cache key — prevents cross-workspace data sharing.
  2. Strip or reject double-quotes from the label parameter — prevents GitHub search qualifier manipulation.

The remaining items are polish/future-work level. The PR is otherwise in good shape.

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

Code Review — GitHub Issue Browser (#564)

Overall: Well-executed PR. The architecture rules are followed cleanly: core is headless, the PAT never leaks into responses, typed errors are reused, and the 60s cache lives in the router layer where it belongs. Tests are comprehensive and meaningful. Three items below warrant attention before merge.


Issues

1. Unbounded _ISSUE_CACHE growth (memory leak)

_ISSUE_CACHE entries are only evicted on read, never on a background sweep. A long-running server making many distinct queries (different repos, search terms, labels) will accumulate stale entries indefinitely. Simplest fix — cap the dict or use functools.lru_cache with maxsize:

# github_integrations_v2.py
from functools import lru_cache

# Or simply add a max-size guard:
_ISSUE_CACHE_MAX_ENTRIES = 500

def _issue_cache_set(key: str, payload: Any) -> None:
    if len(_ISSUE_CACHE) >= _ISSUE_CACHE_MAX_ENTRIES:
        # Drop the oldest (insertion order in Python 3.7+)
        _ISSUE_CACHE.pop(next(iter(_ISSUE_CACHE)))
    _ISSUE_CACHE[key] = (time.monotonic() + _ISSUE_CACHE_TTL_SECONDS, payload)

2. Label filter input is not debounced

searchInput is debounced 300ms before driving a fetch, which is correct. But labelFilter fires a new SWR key immediately on every keystroke (setLabelFilter(e.target.value); setPage(1)). This hits the backend on each character typed. GitHub's search API has stricter rate limits (30 req/min authenticated), and while the 60s cache absorbs repeat queries, fast typists on the search+label path could still hammer it. The label input should mirror the debounce pattern the search box already uses.

3. GitHub 429 (rate-limit) not mapped to a distinct error

_raise_for_status catches >= 400 as a generic GitHubConnectError, which the router surfaces as 502. A 429 Too Many Requests from GitHub's /search/issues endpoint (which has a separate, tighter quota) should ideally be surfaced as a retriable signal rather than a generic gateway error — either a dedicated exception class or at minimum a distinguishable message. The search path is the higher-risk route here since its per-minute quota is ~3× tighter than the REST list endpoint.


Minor observations (non-blocking)

  • Imports inside test methods: from codeframe.core.github_connect_service import InvalidTokenError appears inside test_invalid_token_maps_to_401 and test_insufficient_scope_maps_to_403. Hoisting these to module level keeps the pattern consistent with the rest of the file.

  • body field absent from GitHubIssue: Fine for the browser-only scope of [Phase 5.5] GitHub Issues import: issue browser and multi-select UI #564, but [Phase 5.5] GitHub Issues import: execute import and task traceability #565's import will need issue body to populate task descriptions. Worth flagging now so the API contract can be extended in that PR without a breaking schema change.

  • handleImportIssues has an unused parameter: The _selected: GitHubIssue[] argument is intentionally a no-op stub (per the PR description), but the leading underscore convention (_selected) already signals this clearly. No change needed — just confirming it's intentional.


What's solid

  • Core service is genuinely headless — no FastAPI/HTTP types bleed into github_issues_service.py.
  • PR filter on the list endpoint (stripping pull_request-keyed items) is applied on both the REST and search paths.
  • Cache key uses the already-clamped per_page value, so a request for per_page=500 → clamped to 100 → cache key reflects 100. Correct.
  • test_pat_never_echoed asserting VALID_PAT not in r.text is a good security gate.
  • .gitignore /tasks/ anchor fix is a real bug fix — was silently dropping web-ui/src/components/tasks/ from git tracking.

The two cache issues (#1 and #2) are the ones I'd resolve before merging; #3 is a polish item. Otherwise this is merge-ready.

@frankbria frankbria merged commit 4010f38 into main Jun 2, 2026
11 checks passed
@frankbria frankbria deleted the feat/564-github-issues-browser branch June 2, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Phase 5.5] GitHub Issues import: issue browser and multi-select UI

1 participant