Conversation
…nt epic state - MetadataManager.updateMetadata: re-throw errors after logging per project guidelines - start.ts: replace dynamic imports of prompt.js with static import - start.ts: revert parsed.type to 'issue' when epic child data fetch fails - start.ts: replace process.exit(1) with throw for testability - ignite.ts: remove unnecessary `as string` cast on issueType - Update tests to match new behavior
Complexity AssessmentClassification: SIMPLE Metrics:
Reasoning: While the refactoring involves signature propagation across multiple callers (src/cli.ts, src/commands/start.ts, src/commands/ignite.ts, src/utils/list-children.ts) and test rewrites, this is a mechanical refactoring with an established provider pattern precedent. The architectural decisions (return type mapping, IssueTracker parameter passing) follow existing patterns in the codebase and involve no breaking changes or data migrations. |
Combined Analysis & Plan - Issue #631Executive Summary
Implementation OverviewHigh-Level Execution Phases
Quick Stats
Complete Analysis and Implementation Details (click to expand)Research FindingsProblem Space
Codebase Research
Affected Files
Signature Propagation ChainCallers:
Implementation PlanAutomated Test Cases to CreateTest File: The Click to expand test structure (15 lines)describe('fetchChildIssues', () => {
// Create mock IssueTracker with getChildIssues method
// Test: should delegate to issueTracker.getChildIssues()
// Test: should return empty array when API fails (fault tolerance via Promise.allSettled)
// Test: should pass repo parameter to issueTracker.getChildIssues()
// Remove: provider-specific tests (GitHub parseInt, Linear apiToken, unsupported provider)
// -- those concerns now live in the provider implementations
})
describe('assembleChildrenData', () => {
// Update all tests: replace `settings` param with mock `issueTracker`
// Remove: mocking of getSubIssues/IssueTrackerFactory.getProviderName
// Add: mock issueTracker.getChildIssues() instead
})Provider test files: Provider-specific Click to expand provider test structure (30 lines)// In src/lib/GitHubService.test.ts - add describe block:
describe('getChildIssues', () => {
// Test: should call getSubIssues with parsed integer and repo
// Test: should return empty array for non-numeric identifier (e.g., "not-a-number")
// Test: should return results from getSubIssues mapped to {id, title, url, state}
})
// In src/lib/LinearService.test.ts - add describe block:
describe('getChildIssues', () => {
// Test: should call getLinearChildIssues with identifier
// Test: should pass apiToken from config when available
// Test: should pass undefined options when no apiToken configured
})
// In src/lib/providers/jira/JiraIssueTracker.ts - add tests
// (No existing test file; add describe block to a new JiraIssueTracker.test.ts
// OR add to existing test infrastructure if there's a shared pattern)
describe('JiraIssueTracker', () => {
describe('getChildIssues', () => {
// Test: should query JQL "parent = KEY" via client.searchIssues()
// Test: should normalize identifier to uppercase before querying
// Test: should map Jira issues to {id: key, title: summary, url, state: status.name.toLowerCase()}
// Test: should return empty array when no children found
// Test: should construct URL using config.host + "/browse/" + key
})
})Files to Modify1.
|
Implementation CompleteSummaryRefactored Changes Made
Validation Results
Code Review Fixes AppliedJQL Injection Protection (Medium Severity)Added Jira key format validation ( Promise.allSettled Simplification (Low Severity)Replaced Additional Tests
|
…er abstraction Replace hardcoded per-provider switch statement in fetchChildIssues() with delegation to IssueTracker.getChildIssues() interface method. Each provider (GitHub, Linear, Jira) now implements getChildIssues(), enabling Jira child issue support and ensuring future providers get child issue fetching for free. - Add getChildIssues() to IssueTracker interface - Implement in GitHubService, LinearService, JiraIssueTracker - Add JQL key format validation to prevent injection - Simplify fetchChildIssues error handling (try/catch vs Promise.allSettled) - Update all callers to pass IssueTracker instead of settings - Rewrite list-children tests to use mock IssueTracker - Add provider-specific getChildIssues tests (13 new tests) Fixes #631
acreeger
force-pushed
the
refactor/issue-631__fetch-child-issues-provider
branch
from
c9a7c98 to
fb1c564
Compare
1 participant
Fixes #631
Refactor fetchChildIssues to use IssueTracker provider abstraction instead of hardcoded per-provider utilities
Problem
fetchChildIssues()insrc/utils/list-children.tsbypasses the provider abstraction pattern by using hardcoded per-provider utility functions with a switch statement:This means:
JiraIssueManagementProvider) already has a workinggetChildIssues()method, showing the capability exists but isn't exposed through the right abstractionImpact
il list --childrendoesn't work for Jira projectsil startcan't find child issues for Jira epicsfetchChildIssueDetails()(used for swarm mode epic metadata) also fails for Jira since it callsfetchChildIssues()internallyProposed Solution
Refactor
fetchChildIssues()to delegate to theIssueTrackerinterface (or theIssueManagementProviderabstraction) instead of calling provider-specific utility functions directly. This would:Options
getChildIssues()to theIssueTrackerinterface — Each provider implements it,fetchChildIssuesjust callsissueTracker.getChildIssues(parentId)IssueManagementProvider.getChildIssues()— Already implemented for all three providers, but couples CLI utils to the MCP layerOption 1 is cleaner architecturally.
Files to Change
src/lib/IssueTracker.ts— AddgetChildIssues()to interfacesrc/lib/GitHubService.ts— Implement using existinggetSubIssues()src/lib/providers/jira/JiraIssueTracker.ts— Implement using JQL querygetLinearChildIssues()src/utils/list-children.ts— RefactorfetchChildIssues()to use the interfaceThis PR was created automatically by iloom.