Skip to content

Commit 56151b7

Browse files
acreegeracreeger
committed
Add skipVerify configuration to skip pre-commit hooks in finish workflow - Add noVerify boolean setting to workflow configurations - Implement --no-verify git flag when skipVerify is enabled - Update CommitManager to conditionally include --no-verify in commit commands - Add comprehensive tests for skipVerify functionality - Update types and settings schema to support noVerify option - Fix ClaudeService test to use correct default permission mode Fixes #103 
1 parent 055c05b commit 56151b7

File tree

10 files changed

+271
-7
lines changed

10 files changed

+271
-7
lines changed

‎.hatchbox/settings.example.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
"mainBranch": "main",
33
"workflows": {
44
"issue": {
5-
"permissionMode": "default"
5+
"permissionMode": "default",
6+
"noVerify": false
67
}
78
},
89
"capabilities": {

‎.hatchbox/settings.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
"mainBranch": "main",
33
"workflows": {
44
"issue": {
5-
"permissionMode": "bypassPermissions"
5+
"permissionMode": "bypassPermissions",
6+
"noVerify": true
67
}
78
}
89
}

‎src/commands/finish.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,13 @@ export class FinishCommand {
477477
} else {
478478
logger.info('Validation passed, auto-committing uncommitted changes...')
479479

480+
// Load settings to get skipVerify configuration
481+
const settings = await this.settingsManager.loadSettings(worktree.path)
482+
const skipVerify = settings.workflows?.issue?.noVerify ?? false
483+
480484
const commitOptions: CommitOptions = {
481485
dryRun: options.dryRun ?? false,
486+
skipVerify,
482487
}
483488

484489
// Only add issueNumber if it's an issue
@@ -574,8 +579,14 @@ export class FinishCommand {
574579
logger.info('[DRY RUN] Would commit uncommitted changes')
575580
} else {
576581
logger.info('Committing uncommitted changes...')
582+
583+
// Load settings to get skipVerify configuration
584+
const settings = await this.settingsManager.loadSettings(worktree.path)
585+
const skipVerify = settings.workflows?.pr?.noVerify ?? false
586+
577587
await this.commitManager.commitChanges(worktree.path, {
578588
dryRun: false,
589+
skipVerify,
579590
// Do NOT pass issueNumber for PRs - no "Fixes #" trailer needed
580591
})
581592
logger.success('Changes committed')

‎src/lib/ClaudeService.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ describe('ClaudeService', () => {
8484

8585
expect(claudeUtils.launchClaudeInNewTerminalWindow).toHaveBeenCalledWith(prompt, {
8686
model: 'claude-sonnet-4-20250514',
87-
permissionMode: 'acceptEdits', // Default for issue workflow
87+
permissionMode: 'acceptEdits', // Default fallback when no settings configured
8888
workspacePath: '/workspace/issue-123',
8989
addDir: '/workspace/issue-123',
9090
headless: false,

‎src/lib/CommitManager.test.ts‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,4 +853,137 @@ describe('CommitManager', () => {
853853
).resolves.not.toThrow()
854854
})
855855
})
856+
857+
describe('commitChanges with skipVerify option', () => {
858+
beforeEach(() => {
859+
vi.mocked(claude.detectClaudeCli).mockResolvedValue(false)
860+
})
861+
862+
it('should include --no-verify flag when skipVerify is true (no editor review)', async () => {
863+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
864+
865+
await manager.commitChanges(mockWorktreePath, {
866+
skipVerify: true,
867+
noReview: true,
868+
dryRun: false,
869+
})
870+
871+
expect(git.executeGitCommand).toHaveBeenCalledWith(
872+
['commit', '-m', 'WIP: Auto-commit uncommitted changes', '--no-verify'],
873+
{ cwd: mockWorktreePath }
874+
)
875+
})
876+
877+
it('should include --no-verify flag when skipVerify is true (with editor review)', async () => {
878+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
879+
880+
await manager.commitChanges(mockWorktreePath, {
881+
skipVerify: true,
882+
dryRun: false,
883+
})
884+
885+
expect(git.executeGitCommand).toHaveBeenCalledWith(
886+
['commit', '-e', '-m', 'WIP: Auto-commit uncommitted changes', '--no-verify'],
887+
{ cwd: mockWorktreePath, stdio: 'inherit', timeout: 300000 }
888+
)
889+
})
890+
891+
it('should NOT include --no-verify flag when skipVerify is false', async () => {
892+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
893+
894+
await manager.commitChanges(mockWorktreePath, {
895+
skipVerify: false,
896+
dryRun: false,
897+
})
898+
899+
expect(git.executeGitCommand).toHaveBeenCalledWith(
900+
['commit', '-e', '-m', 'WIP: Auto-commit uncommitted changes'],
901+
{ cwd: mockWorktreePath, stdio: 'inherit', timeout: 300000 }
902+
)
903+
})
904+
905+
it('should NOT include --no-verify flag when skipVerify is undefined', async () => {
906+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
907+
908+
await manager.commitChanges(mockWorktreePath, { dryRun: false })
909+
910+
expect(git.executeGitCommand).toHaveBeenCalledWith(
911+
['commit', '-e', '-m', 'WIP: Auto-commit uncommitted changes'],
912+
{ cwd: mockWorktreePath, stdio: 'inherit', timeout: 300000 }
913+
)
914+
})
915+
916+
it('should log warning when --no-verify flag is used', async () => {
917+
const { logger } = await import('../utils/logger.js')
918+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
919+
920+
await manager.commitChanges(mockWorktreePath, {
921+
skipVerify: true,
922+
dryRun: false,
923+
})
924+
925+
expect(logger.warn).toHaveBeenCalledWith(
926+
expect.stringContaining('Skipping pre-commit hooks')
927+
)
928+
})
929+
930+
it('should log correct dry-run message when skipVerify is true', async () => {
931+
const { logger } = await import('../utils/logger.js')
932+
933+
await manager.commitChanges(mockWorktreePath, {
934+
skipVerify: true,
935+
dryRun: true,
936+
})
937+
938+
expect(logger.info).toHaveBeenCalledWith(
939+
expect.stringContaining('[DRY RUN] Would commit with message --no-verify:')
940+
)
941+
})
942+
943+
it('should include --no-verify flag with custom message', async () => {
944+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
945+
946+
await manager.commitChanges(mockWorktreePath, {
947+
skipVerify: true,
948+
message: 'Custom message',
949+
dryRun: false,
950+
})
951+
952+
expect(git.executeGitCommand).toHaveBeenCalledWith(
953+
['commit', '-m', 'Custom message', '--no-verify'],
954+
{ cwd: mockWorktreePath }
955+
)
956+
})
957+
958+
it('should include --no-verify flag with issue number', async () => {
959+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
960+
961+
await manager.commitChanges(mockWorktreePath, {
962+
skipVerify: true,
963+
issueNumber: 123,
964+
dryRun: false,
965+
})
966+
967+
expect(git.executeGitCommand).toHaveBeenCalledWith(
968+
['commit', '-e', '-m', 'WIP: Auto-commit for issue #123\n\nFixes #123', '--no-verify'],
969+
{ cwd: mockWorktreePath, stdio: 'inherit', timeout: 300000 }
970+
)
971+
})
972+
973+
it('should include --no-verify flag with Claude-generated message', async () => {
974+
vi.mocked(claude.detectClaudeCli).mockResolvedValue(true)
975+
vi.mocked(claude.launchClaude).mockResolvedValue('Add authentication feature')
976+
vi.mocked(git.executeGitCommand).mockResolvedValue('')
977+
978+
await manager.commitChanges(mockWorktreePath, {
979+
skipVerify: true,
980+
dryRun: false,
981+
})
982+
983+
expect(git.executeGitCommand).toHaveBeenCalledWith(
984+
['commit', '-e', '-m', 'Add authentication feature', '--no-verify'],
985+
{ cwd: mockWorktreePath, stdio: 'inherit', timeout: 300000 }
986+
)
987+
})
988+
})
856989
})

‎src/lib/CommitManager.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ export class CommitManager {
4848
logger.info('[DRY RUN] Would run: git add -A')
4949
logger.info('[DRY RUN] Would generate commit message with Claude (if available)')
5050
const fallbackMessage = this.generateFallbackMessage(options)
51-
logger.info(`[DRY RUN] Would commit with message: ${fallbackMessage}`)
51+
const verifyFlag = options.skipVerify ? ' --no-verify' : ''
52+
logger.info(`[DRY RUN] Would commit with message${verifyFlag}: ${fallbackMessage}`)
5253
return
5354
}
5455

@@ -70,15 +71,28 @@ export class CommitManager {
7071
// Fallback to simple message if Claude failed or unavailable
7172
message ??= this.generateFallbackMessage(options)
7273

73-
// Step 4: Commit with user review via git editor (unless noReview specified)
74+
// Step 4: Log warning if --no-verify is configured
75+
if (options.skipVerify) {
76+
logger.warn('⚠️ Skipping pre-commit hooks (--no-verify configured in settings)')
77+
}
78+
79+
// Step 5: Commit with user review via git editor (unless noReview specified)
7480
try {
7581
if (options.noReview || options.message) {
7682
// Direct commit without editor review
77-
await executeGitCommand(['commit', '-m', message], { cwd: worktreePath })
83+
const commitArgs = ['commit', '-m', message]
84+
if (options.skipVerify) {
85+
commitArgs.push('--no-verify')
86+
}
87+
await executeGitCommand(commitArgs, { cwd: worktreePath })
7888
} else {
7989
// Use git editor for user review - pre-populate message and open editor
8090
logger.info('Opening git editor for commit message review...')
81-
await executeGitCommand(['commit', '-e', '-m', message], {
91+
const commitArgs = ['commit', '-e', '-m', message]
92+
if (options.skipVerify) {
93+
commitArgs.push('--no-verify')
94+
}
95+
await executeGitCommand(commitArgs, {
8296
cwd: worktreePath,
8397
stdio: 'inherit',
8498
timeout: 300000 // 5 minutes for interactive editing

‎src/lib/SettingsManager.test.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,103 @@ describe('SettingsManager', () => {
430430
})
431431
})
432432

433+
describe('workflows.{type}.noVerify configuration', () => {
434+
it('should accept valid noVerify boolean in workflows.issue', async () => {
435+
const projectRoot = '/test/project'
436+
const settings = {
437+
workflows: {
438+
issue: {
439+
permissionMode: 'plan',
440+
noVerify: true,
441+
},
442+
},
443+
}
444+
445+
vi.mocked(readFile).mockResolvedValueOnce(JSON.stringify(settings))
446+
447+
const result = await settingsManager.loadSettings(projectRoot)
448+
expect(result.workflows?.issue?.noVerify).toBe(true)
449+
})
450+
451+
it('should accept valid noVerify boolean in workflows.pr', async () => {
452+
const projectRoot = '/test/project'
453+
const settings = {
454+
workflows: {
455+
pr: {
456+
permissionMode: 'acceptEdits',
457+
noVerify: false,
458+
},
459+
},
460+
}
461+
462+
vi.mocked(readFile).mockResolvedValueOnce(JSON.stringify(settings))
463+
464+
const result = await settingsManager.loadSettings(projectRoot)
465+
expect(result.workflows?.pr?.noVerify).toBe(false)
466+
})
467+
468+
it('should accept missing noVerify field (defaults to undefined)', async () => {
469+
const projectRoot = '/test/project'
470+
const settings = {
471+
workflows: {
472+
issue: {
473+
permissionMode: 'plan',
474+
},
475+
},
476+
}
477+
478+
vi.mocked(readFile).mockResolvedValueOnce(JSON.stringify(settings))
479+
480+
const result = await settingsManager.loadSettings(projectRoot)
481+
expect(result.workflows?.issue?.noVerify).toBeUndefined()
482+
})
483+
484+
it('should reject invalid noVerify types (non-boolean)', async () => {
485+
const projectRoot = '/test/project'
486+
const settings = {
487+
workflows: {
488+
issue: {
489+
permissionMode: 'plan',
490+
noVerify: 'true', // String instead of boolean
491+
},
492+
},
493+
}
494+
495+
vi.mocked(readFile).mockResolvedValueOnce(JSON.stringify(settings))
496+
497+
await expect(settingsManager.loadSettings(projectRoot)).rejects.toThrow(
498+
/Settings validation failed[\s\S]*workflows\.issue\.noVerify[\s\S]*Expected boolean, received string/,
499+
)
500+
})
501+
502+
it('should handle multiple workflow types with different noVerify settings', async () => {
503+
const projectRoot = '/test/project'
504+
const settings = {
505+
workflows: {
506+
issue: {
507+
permissionMode: 'plan',
508+
noVerify: true,
509+
},
510+
pr: {
511+
permissionMode: 'acceptEdits',
512+
noVerify: false,
513+
},
514+
regular: {
515+
permissionMode: 'bypassPermissions',
516+
noVerify: true,
517+
},
518+
},
519+
}
520+
521+
vi.mocked(readFile).mockResolvedValueOnce(JSON.stringify(settings))
522+
523+
const result = await settingsManager.loadSettings(projectRoot)
524+
expect(result.workflows?.issue?.noVerify).toBe(true)
525+
expect(result.workflows?.pr?.noVerify).toBe(false)
526+
expect(result.workflows?.regular?.noVerify).toBe(true)
527+
})
528+
})
529+
433530
describe('loadSettings with workflows', () => {
434531
it('should load settings with workflows configuration correctly', async () => {
435532
const projectRoot = '/test/project'

‎src/lib/SettingsManager.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ export const WorkflowPermissionSchema = z.object({
2222
.enum(['plan', 'acceptEdits', 'bypassPermissions', 'default'])
2323
.optional()
2424
.describe('Permission mode for Claude CLI in this workflow type'),
25+
noVerify: z
26+
.boolean()
27+
.optional()
28+
.describe('Skip pre-commit hooks (--no-verify) when committing during finish workflow'),
2529
})
2630

2731
/**

‎src/types/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ export interface CommitOptions {
223223
issueNumber?: number // For "Fixes #N" trailer
224224
message?: string // Custom message override
225225
noReview?: boolean // Skip user review of commit message
226+
skipVerify?: boolean // Skip pre-commit hooks (--no-verify flag)
226227
}
227228

228229
// Merge management types

‎src/utils/git.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import path from 'path'
22
import { execa, type ExecaError } from 'execa'
33
import { type GitWorktree } from '../types/worktree.js'
44
import type { SettingsManager } from '../lib/SettingsManager.js'
5+
import { logger } from './logger.js'
56

67
/**
78
* Execute a Git command and return the stdout result
@@ -17,6 +18,7 @@ export async function executeGitCommand(
1718
timeout: options?.timeout ?? 30000,
1819
encoding: 'utf8',
1920
stdio: options?.stdio ?? 'pipe',
21+
verbose: logger.isDebugEnabled(),
2022
})
2123

2224
return result.stdout

0 commit comments

Comments
 (0)