feat: Jira integration, sync improvements, and bot filter cleanup#1
feat: Jira integration, sync improvements, and bot filter cleanup#1ViktorBilokin wants to merge 8 commits intomainfrom
Conversation
…leanup - Add Jira integration: JiraController, JiraService, JiraPage with full UI - Sync page: sortable table (by name/status/last synced), status-first default sort, Actions column header, Re-sync/Clean styled as buttons - Sync trigger: now includes completed repos for incremental re-sync alongside pending (full sync) - Dashboard: remove automatic [bot] login filter — bots must now be listed explicitly in Bot Names setting - Config page: update Bot Names hint to drop the [bot] auto-filter mention - Makefile: force-kill lingering processes on ports 8080/3000/5432 during stop - application.yml: add Anthropic API key config - Add react-markdown dependency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Reject path variable values that don't match the PROJECT-NUMBER pattern before they are interpolated into the outbound HTTP URL. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
||
| @GetMapping("/issue/{key}") | ||
| public JiraIssueDetailDto getIssueDetail(@PathVariable String key) { | ||
| if (key == null || !JIRA_KEY_PATTERN.matcher(key).matches()) { |
There was a problem hiding this comment.
Yes, the endpoint is secure. Here's the full SSRF protection in place:
-
Strict input validation — the
JIRA_KEY_PATTERNregex (^[A-Z][A-Z0-9]{0,9}-\d+$) is enforced in bothJiraController(returning HTTP 400 on mismatch) and again insideJiraClientas defence-in-depth. A valid key can only contain uppercase letters, digits, and one hyphen — none of which can inject a URL scheme, authority, or path separator. -
URL-encoding the key — commit
f9b2098addsURLEncoder.encode(key, StandardCharsets.UTF_8)when building the path inJiraClient.getIssueDetail(and the same forepicKeyfetched from the Jira API response). This makes the sanitization explicit and eliminates the taint path that Snyk's static analysis was still flagging. -
assertAllowedUrlallowlist — every outbound HTTP call inJiraClientgoes throughassertAllowedUrl, which verifies the URL starts with the admin-configuredjira.hostvalue before the request is sent.
CodeQL reports 0 alerts after the change.
There was a problem hiding this comment.
Pull request overview
Adds Jira ticket browsing + AI ticket-quality details to GitHub Pulse, and updates admin sync + dashboard bot filtering to better match current workflows/config.
Changes:
- Introduces Jira backend endpoints/client + a new Jira UI page (epics, child issues, issue detail panel with quality/improvement output).
- Improves Sync page UX (sortable table, clearer manual sync button semantics, action button styling) and expands manual sync to include completed repos (incremental).
- Removes implicit
[bot]filtering in the dashboard; bots must now be explicitly configured.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/pages/SyncPage.tsx | Adds sorting, improves manual sync button text/enablement, and updates actions column/buttons. |
| frontend/src/pages/JiraPage.tsx | New Jira UI page with epic/issue browsing, sorting, and a details panel (incl. markdown improvements). |
| frontend/src/pages/DashboardPage.tsx | Removes implicit bot login filtering and renames the page heading. |
| frontend/src/pages/ConfigPage.tsx | Adds Jira + Anthropic settings fields and masks token inputs. |
| frontend/src/components/Layout.tsx | Adds Jira navigation entry, changes dashboard route, and introduces collapsible sidebar. |
| frontend/src/api/client.ts | Adds Jira API client methods and DTO types for Jira pages. |
| frontend/src/App.tsx | Adds /jira and /github routes; redirects / → /github. |
| frontend/package.json | Adds react-markdown dependency for rendering improved ticket text. |
| frontend/package-lock.json | Locks react-markdown and its dependency tree. |
| backend/src/main/resources/application.yml | Adds anthropic.api-key configuration via env var. |
| backend/src/main/java/com/githubpulse/service/jira/TicketQualityService.java | New Anthropic/Claude-backed ticket quality evaluation service. |
| backend/src/main/java/com/githubpulse/service/jira/JiraIssueDto.java | New Jira issue DTO for epic children. |
| backend/src/main/java/com/githubpulse/service/jira/JiraIssueDetailDto.java | New Jira issue detail DTO (incl. quality/improvement fields). |
| backend/src/main/java/com/githubpulse/service/jira/JiraEpicDto.java | New Jira epic DTO containing child issues. |
| backend/src/main/java/com/githubpulse/service/jira/JiraClient.java | New Jira REST client (epic browsing, issue detail retrieval, ADF text extraction). |
| backend/src/main/java/com/githubpulse/controller/JiraController.java | Exposes Jira API endpoints for the frontend. |
| backend/src/main/java/com/githubpulse/controller/AdminController.java | Manual sync now includes completed repos for incremental updates; response text updated. |
| README.md | Replaces placeholder README with full project overview and usage docs. |
| Makefile | make stop now force-kills processes on common dev ports. |
| CLAUDE.md | Adds repository guidance and commands/architecture notes for Claude Code. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Map<String, Object> raw = rawGet( | ||
| host + "/rest/api/3/issue/" + key + "?fields=summary,description,customfield_10037,customfield_10158", | ||
| auth); | ||
|
|
||
| try { | ||
| JsonNode fields = ((JsonNode) raw.get("body")).path("fields"); | ||
| String summary = fields.path("summary").asText(null); | ||
| String description = extractAdfText(fields.path("description")); | ||
| // acceptance criteria: try customfield_10037 first, then customfield_10158 | ||
| String ac = extractAdfText(fields.path("customfield_10037")); | ||
| if (ac == null) ac = extractAdfText(fields.path("customfield_10158")); | ||
|
|
||
| // fetch epic summary for context | ||
| String epicSummary = null; | ||
| JsonNode epicLinkNode = fields.path("customfield_10014"); | ||
| if (!epicLinkNode.isMissingNode() && !epicLinkNode.isNull()) { |
There was a problem hiding this comment.
getIssueDetail attempts to read customfield_10014 (Epic Link) from the issue fields, but the REST call only requests summary,description,customfield_10037,customfield_10158. Jira won't include customfield_10014 unless explicitly requested, so epicSummary will never be populated. Include customfield_10014 in the fields= query (or request *all if appropriate).
| <TableBody> | ||
| {sortedEpics.map((epic: JiraEpic) => ( | ||
| <> | ||
| <TableRow | ||
| key={epic.key} | ||
| className={`cursor-pointer bg-muted/40 hover:bg-muted/60 ${selectedKey === epic.key ? 'ring-1 ring-inset ring-primary' : ''}`} |
There was a problem hiding this comment.
sortedEpics.map(...) returns a fragment (<>...</>) without a key. React will warn and may mis-associate rows during re-renders (expand/collapse, sorting). Wrap the epic row + children in a keyed React.Fragment (or another keyed element).
| @GetMapping("/epics") | ||
| public List<JiraEpicDto> getEpics() { | ||
| return jiraClient.getEpicsWithChildren(); | ||
| } | ||
|
|
||
| @GetMapping("/issue/{key}") | ||
| public JiraIssueDetailDto getIssueDetail(@PathVariable String key) { | ||
| if (key == null || !JIRA_KEY_PATTERN.matcher(key).matches()) { | ||
| throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Invalid Jira issue key"); | ||
| } | ||
| return jiraClient.getIssueDetail(key); | ||
| } |
There was a problem hiding this comment.
New Jira endpoints (/api/jira/epics, /api/jira/issue/{key}) introduce significant behavior (external Jira calls + Claude quality evaluation), but there are currently no Cucumber scenarios covering Jira integration. Consider adding BDD coverage for: missing config → empty result/4xx, invalid key → 400, and happy-path responses (with JiraClient mocked/stubbed).
| @GetMapping("/debug") | ||
| public Map<String, Object> debug() { | ||
| return jiraClient.debug(); | ||
| } |
There was a problem hiding this comment.
The /api/jira/debug endpoint appears to be enabled unconditionally and returns detailed Jira API responses plus configuration metadata. Exposing this in production (especially with @CrossOrigin("*")) is a security risk. Consider removing the endpoint, gating it behind a feature flag/profile, or restricting access (e.g., admin auth / local-only).
| <TableHead className={`w-36 ${thClass}`} onClick={() => handleSort('key')}> | ||
| Key <SortIcon field="key" sortField={sortField} sortDir={sortDir} /> | ||
| </TableHead> | ||
| <TableHead className={thClass} onClick={() => handleSort('summary')}> | ||
| Title <SortIcon field="summary" sortField={sortField} sortDir={sortDir} /> | ||
| </TableHead> | ||
| <TableHead className="w-32">Status</TableHead> | ||
| <TableHead className={`w-40 ${thClass}`} onClick={() => handleSort('assignee')}> | ||
| Assignee <SortIcon field="assignee" sortField={sortField} sortDir={sortDir} /> | ||
| </TableHead> | ||
| <TableHead className={`w-28 ${thClass}`} onClick={() => handleSort('priority')}> | ||
| Priority <SortIcon field="priority" sortField={sortField} sortDir={sortDir} /> |
There was a problem hiding this comment.
Sortable column headers are implemented by attaching onClick to <TableHead>, but table headers aren't keyboard-focusable by default, so sorting may be inaccessible to keyboard/screen-reader users. Consider rendering a <button> inside the header (or adding role="button", tabIndex=0, and onKeyDown handling) and an accessible label indicating sort state.
| <TableHead className={`w-36 ${thClass}`} onClick={() => handleSort('key')}> | |
| Key <SortIcon field="key" sortField={sortField} sortDir={sortDir} /> | |
| </TableHead> | |
| <TableHead className={thClass} onClick={() => handleSort('summary')}> | |
| Title <SortIcon field="summary" sortField={sortField} sortDir={sortDir} /> | |
| </TableHead> | |
| <TableHead className="w-32">Status</TableHead> | |
| <TableHead className={`w-40 ${thClass}`} onClick={() => handleSort('assignee')}> | |
| Assignee <SortIcon field="assignee" sortField={sortField} sortDir={sortDir} /> | |
| </TableHead> | |
| <TableHead className={`w-28 ${thClass}`} onClick={() => handleSort('priority')}> | |
| Priority <SortIcon field="priority" sortField={sortField} sortDir={sortDir} /> | |
| <TableHead | |
| className={`w-36 ${thClass}`} | |
| aria-sort={ | |
| sortField === 'key' | |
| ? sortDir === 'asc' | |
| ? 'ascending' | |
| : 'descending' | |
| : 'none' | |
| } | |
| > | |
| <button | |
| type="button" | |
| className="flex w-full items-center gap-1 text-left" | |
| onClick={() => handleSort('key')} | |
| > | |
| <span>Key</span> | |
| <SortIcon field="key" sortField={sortField} sortDir={sortDir} /> | |
| </button> | |
| </TableHead> | |
| <TableHead | |
| className={thClass} | |
| aria-sort={ | |
| sortField === 'summary' | |
| ? sortDir === 'asc' | |
| ? 'ascending' | |
| : 'descending' | |
| : 'none' | |
| } | |
| > | |
| <button | |
| type="button" | |
| className="flex w-full items-center gap-1 text-left" | |
| onClick={() => handleSort('summary')} | |
| > | |
| <span>Title</span> | |
| <SortIcon field="summary" sortField={sortField} sortDir={sortDir} /> | |
| </button> | |
| </TableHead> | |
| <TableHead className="w-32">Status</TableHead> | |
| <TableHead | |
| className={`w-40 ${thClass}`} | |
| aria-sort={ | |
| sortField === 'assignee' | |
| ? sortDir === 'asc' | |
| ? 'ascending' | |
| : 'descending' | |
| : 'none' | |
| } | |
| > | |
| <button | |
| type="button" | |
| className="flex w-full items-center gap-1 text-left" | |
| onClick={() => handleSort('assignee')} | |
| > | |
| <span>Assignee</span> | |
| <SortIcon field="assignee" sortField={sortField} sortDir={sortDir} /> | |
| </button> | |
| </TableHead> | |
| <TableHead | |
| className={`w-28 ${thClass}`} | |
| aria-sort={ | |
| sortField === 'priority' | |
| ? sortDir === 'asc' | |
| ? 'ascending' | |
| : 'descending' | |
| : 'none' | |
| } | |
| > | |
| <button | |
| type="button" | |
| className="flex w-full items-center gap-1 text-left" | |
| onClick={() => handleSort('priority')} | |
| > | |
| <span>Priority</span> | |
| <SortIcon field="priority" sortField={sortField} sortDir={sortDir} /> | |
| </button> |
| Map<String, Object> raw = rawGet( | ||
| host + "/rest/api/3/issue/" + key + "?fields=summary,description,customfield_10037,customfield_10158", | ||
| auth); | ||
|
|
||
| try { | ||
| JsonNode fields = ((JsonNode) raw.get("body")).path("fields"); | ||
| String summary = fields.path("summary").asText(null); | ||
| String description = extractAdfText(fields.path("description")); |
There was a problem hiding this comment.
rawGet returns a map even for non-2xx responses (and can return {error: ...}), but getIssueDetail immediately casts raw.get("body") and parses fields without checking status/presence of body. This can cause NPEs/500s on auth/config errors. Consider validating the Jira response status and handling missing/failed responses explicitly (e.g., throw a controlled exception with an appropriate HTTP status).
| .header("Accept", "application/json") | ||
| .GET().build(); | ||
| HttpResponse<String> resp = httpClient.send(req, HttpResponse.BodyHandlers.ofString()); | ||
| log.info("[JIRA DEBUG] GET {} → {}: {}", url, resp.statusCode(), resp.body()); |
There was a problem hiding this comment.
The Jira client logs the full Jira response body at INFO ([JIRA DEBUG] ... resp.body()), which can leak ticket content/PII into application logs and create very large log entries. Consider downgrading to DEBUG, truncating/redacting the body, or logging only status + request id.
| log.info("[JIRA DEBUG] GET {} → {}: {}", url, resp.statusCode(), resp.body()); | |
| log.debug("[JIRA DEBUG] GET {} → {}", url, resp.statusCode()); |
| JsonNode root = objectMapper.readTree(response.body()); | ||
| log.info("[JIRA] Response status={} body={}", response.statusCode(), response.body()); | ||
| JsonNode issues = root.path("issues"); |
There was a problem hiding this comment.
searchIssues logs the entire Jira API response body at INFO. This can expose sensitive Jira data and significantly increase log volume during normal operation. Suggest logging only summary info (status, issue count, nextPageToken presence) at INFO and moving body logging to DEBUG with truncation/redaction.
|
|
||
| log.info("[SYNC] Manual sync triggered: {} pending (full) + {} completed (incremental since last sync)", | ||
| pendingRepos.size(), completedRepos.size()); | ||
| syncOrchestrator.syncReposAsync(toSync); |
There was a problem hiding this comment.
This change modifies the manual sync response strings and behavior (completed repos now sync incrementally). Existing Cucumber scenarios currently assert the old wording like "No pending" / "1 pending"; consider either keeping backward-compatible response text or updating the BDD tests accordingly so CI doesn't break.
| syncOrchestrator.syncReposAsync(toSync); | |
| syncOrchestrator.syncReposAsync(toSync); | |
| if (pendingRepos.isEmpty() && !completedRepos.isEmpty()) { | |
| // Backward-compatible phrasing for scenarios with no pending repos, | |
| // while still informing about incremental sync of completed repos. | |
| return ResponseEntity.ok( | |
| "No pending repositories. " + completedRepos.size() | |
| + " completed repositories will sync incrementally. Running in background."); | |
| } |
| {!collapsed && <span className="text-lg font-semibold">GitHub Pulse</span>} | ||
| <button | ||
| onClick={() => setCollapsed((v) => !v)} | ||
| className="text-muted-foreground hover:text-foreground" | ||
| title={collapsed ? 'Expand sidebar' : 'Collapse sidebar'} | ||
| > | ||
| {collapsed | ||
| ? <PanelLeftOpen className="h-5 w-5" /> | ||
| : <PanelLeftClose className="h-5 w-5" />} | ||
| </button> |
There was a problem hiding this comment.
The sidebar collapse/expand toggle uses only an icon + title. For accessibility, provide an aria-label (and/or visually-hidden text) so screen readers can announce the control purpose/state reliably.
…taint path Agent-Logs-Url: https://github.com/Bandwidth/github-pulse/sessions/6f8d592e-6073-4bcd-baae-431877533f5f Co-authored-by: ViktorBilokin <203859598+ViktorBilokin@users.noreply.github.com>
Summary
JiraController,JiraService(client, DTOs,TicketQualityService), andJiraPageUI with full ticket browsingAdminController): syncs both pending repos (full) and completed repos (incremental since last sync), not just pending[bot]login filter — bots likegithub-actions[bot],dependabot[bot]must now be listed explicitly in the Bot Names config setting[bot]auto-filtermake stopTest plan
github-actions[bot]appears unless added to Bot Namesmake stopcleanly kills all processes on ports 8080, 3000, 5432🤖 Generated with Claude Code