Skip to content

Commit 123147b

Browse files
committed
refactor(hooks): replace blocking/required with onFailure property
BREAKING CHANGE: ToolConfig interface changed - Remove confusing `blocking` and `required` boolean properties - Add single `onFailure: 'continue' | 'stop'` property - 'stop' halts subsequent tools (for syntax checks) - 'continue' (default) runs all tools and reports at end - Log error details in runStep catch block instead of swallowing - Update tests and documentation
1 parent 868e26d commit 123147b

File tree

5 files changed

+50
-38
lines changed

5 files changed

+50
-38
lines changed

booster/.husky/README.md

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,16 @@ To add a new tool, simply add a `ToolConfig` object to the `TOOLS` array.
2828

2929
### Configuration Object (`ToolConfig`)
3030

31-
| Property | Type | Description |
32-
| ------------------ | ----------------- | ------------------------------------------------------------------------------------------------------------------------------- |
33-
| `name` | `string` | Display name of the tool (used in logs). |
34-
| `command` | `string` | The binary command to run (e.g., `eslint`, `rector`). |
35-
| `type` | `'node' \| 'php'` | Determines where to look for the binary (`node_modules/.bin` or `vendor/bin`). |
36-
| `args` | `string[]` | (Optional) Arguments to pass to the command. |
37-
| `extensions` | `string[]` | (Optional) Only run on files with these extensions. |
38-
| `stagesFilesAfter` | `boolean` | (Optional) If `true`, re-stages files after execution (useful for fixers). |
39-
| `passFiles` | `boolean` | (Optional) If `false`, does not pass the list of staged files to the command. Default is `true`. |
40-
| `required` | `boolean` | (Optional) If `true`, the hook will fail if this tool fails. Default is `false` (but usually the hook fails if any tool fails). |
41-
| `blocking` | `boolean` | (Optional) If `true`, stops running subsequent tools if this tool fails. Useful for syntax checks that must pass first. |
31+
| Property | Type | Description |
32+
| ------------------ | --------------------------- | ------------------------------------------------------------------------------------------------ |
33+
| `name` | `string` | Display name of the tool (used in logs). |
34+
| `command` | `string` | The binary command to run (e.g., `eslint`, `rector`). |
35+
| `type` | `'node' \| 'php'` | Determines where to look for the binary (`node_modules/.bin` or `vendor/bin`). |
36+
| `args` | `string[]` | (Optional) Arguments to pass to the command. |
37+
| `extensions` | `string[]` | (Optional) Only run on files with these extensions. |
38+
| `stagesFilesAfter` | `boolean` | (Optional) If `true`, re-stages files after execution (useful for fixers). |
39+
| `passFiles` | `boolean` | (Optional) If `false`, does not pass the list of staged files to the command. Default is `true`. |
40+
| `onFailure` | `'continue' \| 'stop'` | (Optional) What happens when this tool fails. Default is `'continue'`. Use `'stop'` for syntax checks that must pass before other tools run. |
4241

4342
### Example
4443

@@ -49,7 +48,8 @@ To add a new tool, simply add a `ToolConfig` object to the `TOOLS` array.
4948
args: ['--check'],
5049
type: 'node',
5150
extensions: ['.ts', '.js'],
52-
stagesFilesAfter: false
51+
stagesFilesAfter: false,
52+
onFailure: 'continue' // or 'stop' for critical checks
5353
}
5454
```
5555

booster/.husky/shared/tools.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export const TOOLS: ToolConfig[] = [
6161
type: 'php',
6262
runForEachFile: true,
6363
extensions: ['.php'],
64-
blocking: true, // Stop subsequent tools if syntax check fails
64+
onFailure: 'stop', // Stop subsequent tools if syntax check fails
6565
},
6666
{
6767
name: 'Rector',

booster/.husky/shared/types.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@
66
*/
77
export type ToolType = 'node' | 'php' | 'system'
88

9+
/**
10+
* Failure mode for a tool
11+
* - 'continue': Log error, keep running other tools, report failure at end
12+
* - 'stop': Log error, skip remaining tools, report failure immediately
13+
*/
14+
export type FailureMode = 'continue' | 'stop'
15+
916
/**
1017
* Configuration for a quality tool
1118
*/
@@ -28,10 +35,12 @@ export interface ToolConfig {
2835
runForEachFile?: boolean
2936
/** Custom description to show in logs while running */
3037
description?: string
31-
/** If true, the hook will fail if this tool fails. Default is false (but usually the hook fails if any tool fails). */
32-
required?: boolean
33-
/** If true, stops running subsequent tools if this tool fails. Useful for syntax checks that must pass before analysis. */
34-
blocking?: boolean
38+
/**
39+
* What happens when this tool fails. Default is 'continue'.
40+
* - 'continue': Log error, keep running other tools
41+
* - 'stop': Log error, skip remaining tools (use for syntax checks that must pass first)
42+
*/
43+
onFailure?: FailureMode
3544
}
3645

3746
/**

booster/.husky/shared/workflow.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,11 @@ export async function runStep(
3939
log.success(`${toolName} completed successfully (${formattedDuration})`)
4040

4141
return true
42-
} catch {
42+
} catch (error) {
4343
const duration = Date.now() - startTime
4444
const formattedDuration = formatDuration(duration)
45-
log.error(`${toolName} failed after ${formattedDuration}`)
45+
const errorMessage = error instanceof Error ? error.message : String(error)
46+
log.error(`${toolName} failed after ${formattedDuration}: ${errorMessage}`)
4647

4748
return false
4849
}
@@ -190,14 +191,12 @@ export async function runQualityChecks(files: string[], tools: ToolConfig[]): Pr
190191

191192
if (!success) {
192193
allSuccessful = false
193-
if (tool.blocking) {
194+
195+
// Check failure mode - 'stop' halts execution, 'continue' (default) keeps going
196+
if (tool.onFailure === 'stop') {
194197
log.error(`${tool.name} failed. Stopping subsequent checks.`)
195198
return false
196199
}
197-
if (tool.required) {
198-
log.error(`${tool.name} is required but failed`)
199-
return false
200-
}
201200
}
202201
}
203202

booster/.husky/tests/shared/workflow.test.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ describe('workflow.ts', () => {
8181
name: 'test-tool',
8282
command: 'test-cmd',
8383
type: 'system',
84-
extensions: ['.php'],
85-
required: true
84+
extensions: ['.php']
8685
}
8786

8887
it('should skip if tool is skipped via env', async () => {
@@ -171,7 +170,7 @@ describe('workflow.ts', () => {
171170
expect(stageFiles).toHaveBeenCalledWith(['file.php'])
172171
})
173172

174-
it('should fail if required tool fails', async () => {
173+
it('should fail if tool fails', async () => {
175174
vi.mocked(isSkipped).mockReturnValue(false)
176175
vi.mocked(fs.pathExists).mockResolvedValue(true)
177176
vi.mocked(exec).mockRejectedValue(new Error('fail'))
@@ -181,41 +180,46 @@ describe('workflow.ts', () => {
181180
expect(result).toBe(false)
182181
})
183182

184-
it('should not fail if optional tool fails', async () => {
183+
it('should continue to next tool if tool with default onFailure fails', async () => {
185184
vi.mocked(isSkipped).mockReturnValue(false)
186185
vi.mocked(fs.pathExists).mockResolvedValue(true)
187-
vi.mocked(exec).mockRejectedValue(new Error('fail'))
186+
vi.mocked(exec)
187+
.mockRejectedValueOnce(new Error('first tool failed'))
188+
.mockResolvedValueOnce({} as any)
188189

189-
const optionalTool: ToolConfig = { ...mockTool, required: false }
190-
const result = await runQualityChecks(['file.php'], [optionalTool])
190+
const firstTool: ToolConfig = { ...mockTool }
191+
const secondTool: ToolConfig = { ...mockTool, name: 'second-tool' }
192+
193+
const result = await runQualityChecks(['file.php'], [firstTool, secondTool])
191194

192195
expect(result).toBe(false)
196+
expect(exec).toHaveBeenCalledTimes(2) // Both tools ran
193197
})
194198

195-
it('should stop subsequent tools if blocking tool fails', async () => {
199+
it('should stop subsequent tools if onFailure is stop', async () => {
196200
vi.mocked(isSkipped).mockReturnValue(false)
197201
vi.mocked(fs.pathExists).mockResolvedValue(true)
198202
vi.mocked(exec).mockRejectedValue(new Error('syntax error'))
199203

200-
const blockingTool: ToolConfig = { ...mockTool, blocking: true, required: false }
204+
const stoppingTool: ToolConfig = { ...mockTool, onFailure: 'stop' }
201205
const subsequentTool: ToolConfig = { ...mockTool, name: 'subsequent-tool' }
202206

203-
const result = await runQualityChecks(['file.php'], [blockingTool, subsequentTool])
207+
const result = await runQualityChecks(['file.php'], [stoppingTool, subsequentTool])
204208

205209
expect(result).toBe(false)
206-
expect(exec).toHaveBeenCalledTimes(1) // Only blocking tool ran
210+
expect(exec).toHaveBeenCalledTimes(1) // Only stopping tool ran
207211
expect(log.error).toHaveBeenCalledWith(expect.stringContaining('Stopping subsequent checks'))
208212
})
209213

210-
it('should continue to next tool if non-blocking tool fails', async () => {
214+
it('should continue to next tool if onFailure is continue', async () => {
211215
vi.mocked(isSkipped).mockReturnValue(false)
212216
vi.mocked(fs.pathExists).mockResolvedValue(true)
213217
vi.mocked(exec)
214218
.mockRejectedValueOnce(new Error('first tool failed'))
215219
.mockResolvedValueOnce({} as any)
216220

217-
const firstTool: ToolConfig = { ...mockTool, blocking: false, required: false }
218-
const secondTool: ToolConfig = { ...mockTool, name: 'second-tool', required: false }
221+
const firstTool: ToolConfig = { ...mockTool, onFailure: 'continue' }
222+
const secondTool: ToolConfig = { ...mockTool, name: 'second-tool' }
219223

220224
const result = await runQualityChecks(['file.php'], [firstTool, secondTool])
221225

0 commit comments

Comments
 (0)