Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions src/lib/BranchNamingService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {
DefaultBranchNamingService,
SimpleBranchNameStrategy,
ClaudeBranchNameStrategy,
TemplateBranchNameStrategy,
slugify,
type BranchNameStrategy,
} from './BranchNamingService.js'

Expand Down Expand Up @@ -191,4 +193,97 @@ describe('BranchNamingService', () => {
expect(generateBranchName).toHaveBeenCalledWith('Default Model Test', 789, 'haiku')
})
})

describe('TemplateBranchNameStrategy', () => {
it('should substitute {ticketId} and {slug}', async () => {
const strategy = new TemplateBranchNameStrategy('{ticketId}-{slug}')
const branchName = await strategy.generate('PRINT-1234', 'Fix dependency bug')
expect(branchName).toBe('print-1234-fix-dependency-bug')
})

it('should handle Jira-style issue keys', async () => {
const strategy = new TemplateBranchNameStrategy('{ticketId}-{slug}')
const branchName = await strategy.generate('HB-42', 'Add dark mode toggle')
expect(branchName).toBe('hb-42-add-dark-mode-toggle')
})

it('should handle template with only ticketId', async () => {
const strategy = new TemplateBranchNameStrategy('{ticketId}')
const branchName = await strategy.generate('PROJ-99', 'Some title')
expect(branchName).toBe('proj-99')
})

it('should handle template with slashes', async () => {
const strategy = new TemplateBranchNameStrategy('feature/{ticketId}-{slug}')
const branchName = await strategy.generate('ENG-500', 'Update auth flow')
expect(branchName).toBe('feature/eng-500-update-auth-flow')
})

it('should truncate slug to 40 characters', async () => {
const strategy = new TemplateBranchNameStrategy('{ticketId}-{slug}')
const branchName = await strategy.generate(
'PRINT-1',
'This is a very long title that should definitely be truncated at some point'
)
const slug = branchName.replace('print-1-', '')
expect(slug.length).toBeLessThanOrEqual(40)
})

it('should remove trailing hyphens', async () => {
const strategy = new TemplateBranchNameStrategy('{ticketId}-{slug}')
const branchName = await strategy.generate('X-1', '!!!')
expect(branchName).toBe('x-1')
})
})

describe('slugify', () => {
it('should convert to lowercase and replace special chars', () => {
expect(slugify('Fix Bug #123')).toBe('fix-bug-123')
})

it('should respect maxLength', () => {
expect(slugify('a very long string here', 10).length).toBeLessThanOrEqual(10)
})

it('should trim leading and trailing hyphens', () => {
expect(slugify('---hello---')).toBe('hello')
})
})

describe('DefaultBranchNamingService with branchFormat', () => {
it('should use TemplateBranchNameStrategy when branchFormat is provided', async () => {
const service = new DefaultBranchNamingService({ useClaude: false })
const branchName = await service.generateBranchName({
issueNumber: 'PRINT-1234',
title: 'Fix deps bug',
branchFormat: '{ticketId}-{slug}',
})
expect(branchName).toBe('print-1234-fix-deps-bug')
})

it('should prefer explicit strategy over branchFormat', async () => {
class CustomStrategy implements BranchNameStrategy {
async generate(): Promise<string> {
return 'custom/branch'
}
}
const service = new DefaultBranchNamingService({ useClaude: false })
const branchName = await service.generateBranchName({
issueNumber: 'PRINT-1234',
title: 'Fix deps bug',
strategy: new CustomStrategy(),
branchFormat: '{ticketId}-{slug}',
})
expect(branchName).toBe('custom/branch')
})

it('should fall back to default strategy when no branchFormat', async () => {
const service = new DefaultBranchNamingService({ useClaude: false })
const branchName = await service.generateBranchName({
issueNumber: 123,
title: 'Test Issue',
})
expect(branchName).toBe('feat/issue-123__test-issue')
})
})
})
63 changes: 53 additions & 10 deletions src/lib/BranchNamingService.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
import type { BranchNameStrategy, BranchGenerationOptions } from '../types/branch-naming.js'
import { getLogger } from '../utils/logger-context.js'

// ============================================
// Shared Utilities
// ============================================

/**
* Create a URL-safe slug from a title string
*/
export function slugify(title: string, maxLength = 20): string {
return title
.toLowerCase()
.replace(/[^a-z0-9]+/g, '-')
.replace(/^-|-$/g, '')
.substring(0, maxLength)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Warning] substring(0, maxLength) runs after replace(/^-|-$/g, ''), so a truncation that lands on a hyphen can still leave a trailing -. For example slugify('Add-User', 4) yields add-. Reordering to trim hyphens last fixes it:

return title
  .toLowerCase()
  .replace(/[^a-z0-9]+/g, '-')
  .substring(0, maxLength)
  .replace(/^-|-$/g, '')

Worth adding a test that asserts the output never ends in -.


// ============================================
// Strategy Classes
// ============================================
Expand All @@ -11,13 +26,7 @@ import { getLogger } from '../utils/logger-context.js'
*/
export class SimpleBranchNameStrategy implements BranchNameStrategy {
async generate(issueNumber: string | number, title: string): Promise<string> {
// Create a simple slug from the title
const slug = title
.toLowerCase()
.replace(/[^a-z0-9]+/g, '-')
.replace(/^-|-$/g, '')
.substring(0, 20) // Keep it short for the simple strategy

const slug = slugify(title)
return `feat/issue-${issueNumber}__${slug}`
}
}
Expand All @@ -36,6 +45,32 @@ export class ClaudeBranchNameStrategy implements BranchNameStrategy {
}
}

/**
* Template-based branch naming strategy
* Uses a user-defined template with variable substitution
*
* Supported variables:
* {ticketId} - Full issue identifier (e.g., "PRINT-1234")
* {slug} - Slugified title (lowercase, hyphens, max 40 chars)
*
* Example: "{ticketId}-{slug}" → "PRINT-1234-fix-deps-bug"
*/
export class TemplateBranchNameStrategy implements BranchNameStrategy {
constructor(private template: string) {}

async generate(issueNumber: string | number, title: string): Promise<string> {
const slug = slugify(title, 40)
const ticketId = String(issueNumber)

const branchName = this.template
.replace(/\{ticketId\}/g, ticketId)
.replace(/\{slug\}/g, slug)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Warning] The template is a free-form string from settings.json and nothing here sanitises git-forbidden characters. A user writing "feature WIP/{ticketId}" or "{ticketId}:{slug}" will compile fine but fail later at git checkout -b with a cryptic error. Consider either stripping/rejecting any of :, ~, ^, ?, *, [, \, spaces, and .. here, or adding a Zod .refine() at schema load time (see SettingsManager comment). Loud failure at config load is friendlier than a broken il start.


// Normalize: lowercase, remove trailing hyphens
return branchName.toLowerCase().replace(/-+$/g, '')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Bug] Lowercasing the full template output folds the Jira ticket ID, so PROJ-123 becomes proj-123. That breaks Jira smart-commit integration, Jira/Bitbucket branch-to-issue auto-linking, and any CI parser that keys off the upper-case ticket ID. Since the whole point of opting into a custom template is usually to match external tooling, we're defeating the purpose here.

The {slug} portion is already lowercased inside slugify, so suggest only normalising trailing hyphens and leaving the rest of the template's casing intact:

return branchName.replace(/-+$/g, '')

If we want a safety net for weird casing in user templates, a preserveCase option (default true) is the most flexible path.

}
}

// ============================================
// Service Interface and Implementation
// ============================================
Expand Down Expand Up @@ -73,15 +108,23 @@ export class DefaultBranchNamingService implements BranchNamingService {
}

async generateBranchName(options: BranchGenerationOptions): Promise<string> {
const { issueNumber, title, strategy } = options
const { issueNumber, title, strategy, branchFormat } = options

// Use provided strategy or fall back to default
const nameStrategy = strategy ?? this.defaultStrategy
// Priority: explicit strategy > branchFormat template > default strategy
let nameStrategy: BranchNameStrategy
if (strategy) {
nameStrategy = strategy
} else if (branchFormat) {
nameStrategy = new TemplateBranchNameStrategy(branchFormat)
} else {
nameStrategy = this.defaultStrategy
}

getLogger().debug('Generating branch name', {
issueNumber,
title,
strategy: nameStrategy.constructor.name,
branchFormat,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Small thing: this debug log now includes title, which per CLAUDE.md must never reach telemetry. Local stderr-only logging is fine, but if anything ever wires this logger into the telemetry pipeline the title would leak. Either drop title from the log, or add a comment noting this is deliberately local-only so future us doesn't accidentally promote it.

})

return nameStrategy.generate(issueNumber, title)
Expand Down
3 changes: 3 additions & 0 deletions src/lib/IssueTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,7 @@ export interface IssueTracker {

// Context extraction - formats issue/PR for AI prompts
extractContext(entity: Issue | PullRequest): string

// Branch naming - optional custom format template
branchFormat?: string | undefined
}
2 changes: 2 additions & 0 deletions src/lib/LinearService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class LinearService implements IssueTracker {
// IssueTracker interface implementation
readonly providerName = 'linear'
readonly supportsPullRequests = false // Linear doesn't have pull requests
readonly branchFormat?: string | undefined

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Thanks for wiring branchFormat through on LinearService too! For symmetry, would you mind adding the same .describe(...) to Linear's schema entry that Jira has? Right now Linear's branchFormat field has no hint about {ticketId} / {slug} and users are likely to guess wrong (Handlebars syntax, {{key}}, etc.).


private config: LinearServiceConfig
private prompter: (message: string) => Promise<boolean>
Expand All @@ -44,6 +45,7 @@ export class LinearService implements IssueTracker {
options?: { prompter?: (message: string) => Promise<boolean> },
) {
this.config = config ?? {}
this.branchFormat = this.config.branchFormat
this.prompter = options?.prompter ?? promptConfirmation

// Set API token from config if provided (follows mcp.ts pattern)
Expand Down
4 changes: 3 additions & 1 deletion src/lib/LoomManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,10 +652,12 @@ export class LoomManager {
}

if ((input.type === 'issue' || input.type === 'epic') && issueData) {
// Use BranchNamingService for AI-powered branch name generation
// Use BranchNamingService for branch name generation
// Pass branchFormat from issue tracker if configured (e.g., Jira branchFormat)
const branchName = await this.branchNaming.generateBranchName({
issueNumber: input.identifier as number,
title: issueData.title,
branchFormat: this.issueTracker.branchFormat,
})
return branchName
}
Expand Down
8 changes: 8 additions & 0 deletions src/lib/SettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,10 @@ export const IloomSettingsSchema = z.object({
.optional()
.default(['Done'])
.describe('Status names to exclude from issue lists (e.g., ["Done", "Closed", "Verify"])'),
branchFormat: z
.string()
.optional()
.describe('Branch naming template for Jira issues. Variables: {ticketId} (e.g., "PROJ-123"), {slug} (slugified title). Example: "{ticketId}-{slug}" → "proj-123-fix-bug"'),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Warning] A user can set branchFormat: "my-branch" or "" today and every issue resolves to the same branch (collisions as soon as they run il start twice). Suggest a .refine() that requires the string to be non-empty and contain at least one of {ticketId} or {slug}:

branchFormat: z
  .string()
  .min(1)
  .refine(
    v => v.includes('{ticketId}') || v.includes('{slug}'),
    { message: 'branchFormat must contain at least one of {ticketId} or {slug}' },
  )
  .optional()
  .describe('...')

Same change applies to the IloomSettingsSchemaNoDefaults copy further down.

})
.optional(),
})
Expand Down Expand Up @@ -905,6 +909,10 @@ export const IloomSettingsSchemaNoDefaults = z.object({
.optional()
.default(['Done'])
.describe('Status names to exclude from issue lists (e.g., ["Done", "Closed", "Verify"])'),
branchFormat: z
.string()
.optional()
.describe('Branch naming template for Jira issues. Variables: {ticketId} (e.g., "PROJ-123"), {slug} (slugified title). Example: "{ticketId}-{slug}" → "proj-123-fix-bug"'),
})
.optional(),
})
Expand Down
7 changes: 7 additions & 0 deletions src/lib/providers/jira/JiraIssueTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface JiraTrackerConfig extends JiraConfig {
transitionMappings?: Record<string, string> // Map iloom states to Jira transition names
defaultIssueType?: string // Default issue type for creating issues (e.g., "Task", "Story")
defaultSubtaskType?: string // Default issue type for creating subtasks (e.g., "Subtask", "Sub-task")
branchFormat?: string // Branch naming template (e.g., "{ticketId}-{slug}")
}

/**
Expand All @@ -31,6 +32,7 @@ export interface JiraTrackerConfig extends JiraConfig {
export class JiraIssueTracker implements IssueTracker {
readonly providerName = 'jira'
readonly supportsPullRequests = false
readonly branchFormat?: string | undefined

private readonly client: JiraApiClient
private readonly config: JiraTrackerConfig
Expand Down Expand Up @@ -67,13 +69,18 @@ export class JiraIssueTracker implements IssueTracker {
config.transitionMappings = jiraSettings.transitionMappings
}

if (jiraSettings.branchFormat) {
config.branchFormat = jiraSettings.branchFormat
}

return new JiraIssueTracker(config)
}

constructor(config: JiraTrackerConfig, options?: {
prompter?: (message: string) => Promise<boolean>
}) {
this.config = config
this.branchFormat = config.branchFormat
this.client = new JiraApiClient({
host: config.host,
username: config.username,
Expand Down
1 change: 1 addition & 0 deletions src/types/branch-naming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ export interface BranchGenerationOptions {
issueNumber: string | number
title: string
strategy?: BranchNameStrategy // Optional override
branchFormat?: string | undefined // Template string (e.g., "{ticketId}-{slug}")
}
2 changes: 1 addition & 1 deletion src/utils/claude.ts
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ Generate a git branch name for the following issue:
* Check format: {prefix}/issue-{number}__{description}
* Uses case-insensitive matching for issue number (Linear uses uppercase like MARK-1)
*/
function isValidBranchName(name: string, issueNumber: string | number): boolean {
export function isValidBranchName(name: string, issueNumber: string | number): boolean {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Tiny thing: isValidBranchName flipped from private to export in this PR but I don't see a new caller added. Either include the consumer here or revert to keep the module's public surface tight.

const pattern = new RegExp(`^(feat|fix|docs|refactor|test|chore)/issue-${issueNumber}__[a-z0-9-]+$`, 'i')
return pattern.test(name) && name.length <= 50
}