Skip to content
Merged
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
6 changes: 6 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ cd apps/cli && pnpm build

**Every bug fix PR MUST include a regression test** that fails if the fix is reverted.

**Every PR MUST include tests for changed code. No exceptions.**
- Write unit tests for new functions and modified logic
- Write integration tests for new flows and command changes
- Test files live in `apps/cli/test/unit/` and `apps/cli/test/e2e/`
- If you modify source code without adding or updating tests, your PR will be blocked

## Command Code Rules

- **Never call `process.exit()` in command code** — let oclif handle lifecycle (use `return` instead).
Expand Down
57 changes: 57 additions & 0 deletions apps/cli/src/lib/execution/runners/docker-management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,58 @@ export function buildClaudeLifecycleHooks(): Record<string, unknown> {
],
},
],
// PRLT-1225: Enforce test coverage before PR creation or push
// Checks git diff for test file changes — injects warning if none found
PreToolUse: [
{
matcher: 'Bash',
hooks: [
{
type: 'command' as const,
command: '/home/node/.claude/hooks/enforce-tests.sh',
},
],
},
],
},
}
}

/**
* PRLT-1225: Build the enforce-tests hook script content.
* This script fires before Bash tool use, checks if the command is
* 'gh pr create' or 'git push', and verifies test files were changed.
* If no tests found, injects a system message telling the agent to write tests.
*/
export function buildEnforceTestsHookScript(): string {
return `#!/bin/bash
# PRLT-1225: Enforce test coverage in agent PRs
# Fires before Bash tool use — checks if the command is a push/PR command
# and verifies that test files were added or modified in the branch.

INPUT=$(cat)

# Quick check: only care about git push and gh pr create
case "$INPUT" in
*"gh pr create"*|*"git push"*|*"prlt work propose"*|*"prlt pr create"*)
;;
*)
exit 0
;;
esac

# Check for test files in the branch diff (all commits on this branch)
TEST_FILES=$(git diff --name-only origin/main...HEAD 2>/dev/null | grep -iE '\\.(test|spec)\\.(ts|js|tsx|jsx)$')

if [ -z "$TEST_FILES" ]; then
# No tests found — inject warning message into agent context
printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow","additionalContext":"WARNING: No test files (.test.ts, .spec.ts) have been added or modified in this branch. Every PR MUST include tests for changed code — unit tests for new functions, integration tests for new flows. Write tests before creating the PR."}}\\n'
fi

exit 0
`
}

/**
* @deprecated Use buildClaudeLifecycleHooks() instead. Kept for backward compatibility.
*/
Expand Down Expand Up @@ -464,6 +512,7 @@ export function runContainerSetup(containerId: string, permissionMode: Permissio
// Start hook: move ticket to in-progress when agent session begins
// Stop hook: run session report (cleanup + safety net + ticket transition)
// SubagentStop hook: same session report for sub-agent sessions
// PreToolUse hook: enforce test coverage before push/PR (PRLT-1225)
// Uses $PRLT_AGENT_NAME and $PRLT_TICKET_ID shell variables set as container env vars
const lifecycleHooks = buildClaudeLifecycleHooks()
// Merge lifecycle hooks into existing settings.json
Expand All @@ -473,6 +522,14 @@ export function runContainerSetup(containerId: string, permissionMode: Permissio
{ input: JSON.stringify(mergedSettings), stdio: ['pipe', 'pipe', 'pipe'] }
)
console.debug(`[runners:docker] Configured Claude Code lifecycle hooks for agent containers`)

// PRLT-1225: Write the enforce-tests hook script into the container
const enforceTestsScript = buildEnforceTestsHookScript()
execSync(
`docker exec -i ${containerId} bash -c 'mkdir -p /home/node/.claude/hooks && cat > /home/node/.claude/hooks/enforce-tests.sh && chmod +x /home/node/.claude/hooks/enforce-tests.sh'`,
{ input: enforceTestsScript, stdio: ['pipe', 'pipe', 'pipe'] }
)
console.debug(`[runners:docker] Wrote enforce-tests hook script to container`)
} catch (error) {
console.debug('[runners:docker] Failed to copy Claude settings to container:', error)
}
Expand Down
26 changes: 26 additions & 0 deletions apps/cli/src/lib/execution/runners/prompt-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,27 @@ export function buildTicketOperationsGuidance(): string {
return section
}

// =============================================================================
// Test Enforcement Guidance (PRLT-1225)
// =============================================================================

/**
* Build test enforcement guidance telling agents they MUST write tests.
* This is "soft enforcement" (LLM tier) — the Claude Code PreToolUse hook
* provides "hard enforcement" as a safety net.
*/
export function buildTestEnforcementGuidance(): string {
let section = `\n## Test Requirements (MANDATORY)\n\n`
section += `**Every PR MUST include tests for changed code. No exceptions.**\n\n`
section += `- Write **unit tests** for new functions, helpers, and modified logic\n`
section += `- Write **integration/e2e tests** for new flows, commands, and API changes\n`
section += `- Test files go in \`test/unit/\` and \`test/e2e/\` within the package directory\n`
section += `- If you modify source code, you MUST add or update corresponding tests\n`
section += `- Run \`pnpm test:unit\` to verify tests pass before committing\n\n`
section += `A pre-commit hook will block your commit if no test files were added or modified. Write tests as you go, not at the end.\n\n`
return section
}

// =============================================================================
// CI Verification Instructions (PRLT-1126)
// =============================================================================
Expand Down Expand Up @@ -438,6 +459,11 @@ export function buildPrompt(context: ExecutionContext): string {
// PRLT-1224: Soft guidance (LLM tier) — tell agents to use prlt for ticket operations
prompt += buildTicketOperationsGuidance()

// PRLT-1225: Soft guidance (LLM tier) — tell agents they must write tests
if (context.modifiesCode) {
prompt += buildTestEnforcementGuidance()
}

if (context.customMessage) {
prompt += `\n## Additional Instructions\n\n${context.customMessage}\n`
}
Expand Down
187 changes: 183 additions & 4 deletions apps/cli/test/unit/agent-lifecycle-hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { expect } from 'chai'
import {
buildClaudeLifecycleHooks,
buildClaudeStopHookConfig,
buildEnforceTestsHookScript,
} from '../../src/lib/execution/runners/docker-management.js'
import { buildPrompt, buildTicketOperationsGuidance } from '../../src/lib/execution/runners/prompt-builder.js'
import { buildPrompt, buildTicketOperationsGuidance, buildTestEnforcementGuidance } from '../../src/lib/execution/runners/prompt-builder.js'
import { PRESETS, getPreset } from '../../src/lib/orchestrate/presets.js'
import type { ExecutionContext } from '../../src/lib/execution/types.js'

Expand Down Expand Up @@ -78,7 +79,7 @@ describe('PRLT-1224: Layer 1 — Claude Code lifecycle hooks', () => {
}
for (const [hookName, entries] of Object.entries(config.hooks)) {
for (const entry of entries) {
expect(entry).to.have.property('matcher', '', `${hookName} must have matcher field`)
expect(entry).to.have.property('matcher').that.is.a('string', `${hookName} must have matcher field`)
expect(entry).to.have.property('hooks').that.is.an('array', `${hookName} must use hooks array`)
// Regression: old format had type/command at same level as array entries
expect(entry).to.not.have.property('type', `${hookName} must not have flat type`)
Expand All @@ -87,11 +88,15 @@ describe('PRLT-1224: Layer 1 — Claude Code lifecycle hooks', () => {
}
})

it('should have all hooks use error-suppressed commands', () => {
it('should have lifecycle hooks use error-suppressed commands', () => {
const config = buildClaudeLifecycleHooks() as {
hooks: Record<string, Array<{ hooks: Array<{ command: string }> }>>
}
for (const [hookName, entries] of Object.entries(config.hooks)) {
// Only lifecycle hooks (Start/Stop/SubagentStop) need error suppression.
// PreToolUse hooks are scripts that handle their own exit codes.
const lifecycleHooks = ['Start', 'Stop', 'SubagentStop']
for (const hookName of lifecycleHooks) {
const entries = config.hooks[hookName]
for (const entry of entries) {
for (const hook of entry.hooks) {
expect(hook.command).to.include('|| true',
Expand Down Expand Up @@ -238,3 +243,177 @@ describe('PRLT-1224: Layer 3 — Daemon preset lifecycle hooks', () => {
})
})
})

// =============================================================================
// PRLT-1225: Test Enforcement — PreToolUse Hook + Prompt Guidance
// =============================================================================

describe('PRLT-1225: Layer 1 — PreToolUse hook for test enforcement', () => {
describe('buildClaudeLifecycleHooks includes PreToolUse', () => {
it('should include a PreToolUse hook entry', () => {
const config = buildClaudeLifecycleHooks() as {
hooks: Record<string, Array<{ matcher: string; hooks: Array<{ type: string; command: string }> }>>
}
expect(config.hooks).to.have.property('PreToolUse')
expect(config.hooks.PreToolUse).to.be.an('array').with.lengthOf(1)
})

it('should match only Bash tool', () => {
const config = buildClaudeLifecycleHooks() as {
hooks: Record<string, Array<{ matcher: string; hooks: Array<{ type: string; command: string }> }>>
}
const preToolUse = config.hooks.PreToolUse[0]
expect(preToolUse.matcher).to.equal('Bash')
})

it('should reference the enforce-tests hook script', () => {
const config = buildClaudeLifecycleHooks() as {
hooks: Record<string, Array<{ matcher: string; hooks: Array<{ type: string; command: string }> }>>
}
const hook = config.hooks.PreToolUse[0].hooks[0]
expect(hook.type).to.equal('command')
expect(hook.command).to.include('enforce-tests.sh')
})

it('should point to the container hooks directory', () => {
const config = buildClaudeLifecycleHooks() as {
hooks: Record<string, Array<{ matcher: string; hooks: Array<{ type: string; command: string }> }>>
}
const hook = config.hooks.PreToolUse[0].hooks[0]
expect(hook.command).to.equal('/home/node/.claude/hooks/enforce-tests.sh')
})
})

describe('buildEnforceTestsHookScript', () => {
let script: string

before(() => {
script = buildEnforceTestsHookScript()
})

it('should start with a shebang', () => {
expect(script).to.match(/^#!/)
expect(script).to.include('#!/bin/bash')
})

it('should read stdin for tool input', () => {
expect(script).to.include('INPUT=$(cat)')
})

it('should check for gh pr create commands', () => {
expect(script).to.include('gh pr create')
})

it('should check for git push commands', () => {
expect(script).to.include('git push')
})

it('should check for prlt work propose commands', () => {
expect(script).to.include('prlt work propose')
})

it('should check for prlt pr create commands', () => {
expect(script).to.include('prlt pr create')
})

it('should exit early for non-matching commands', () => {
// The case statement should exit 0 for non-matching commands
expect(script).to.include('exit 0')
})

it('should use git diff to check for test files', () => {
expect(script).to.include('git diff --name-only')
expect(script).to.include('origin/main...HEAD')
})

it('should match test file patterns (.test.ts, .spec.ts, etc)', () => {
// Script uses grep -iE with extended regex: \.(test|spec)\.(ts|js|tsx|jsx)$
expect(script).to.include('.test')
expect(script).to.include('.spec')
expect(script).to.include('ts|js|tsx|jsx')
})

it('should output hookSpecificOutput JSON when no tests found', () => {
expect(script).to.include('hookSpecificOutput')
expect(script).to.include('PreToolUse')
expect(script).to.include('permissionDecision')
expect(script).to.include('allow')
expect(script).to.include('additionalContext')
})

it('should warn about missing tests in the additionalContext', () => {
expect(script).to.include('No test files')
expect(script).to.include('MUST include tests')
})

it('should allow (not block) the command even when no tests found', () => {
// The hook warns but does not block — permissionDecision is "allow"
expect(script).to.include('"permissionDecision":"allow"')
})

it('should always exit 0 (never block the tool)', () => {
// Script must not exit with code 2 (which would block)
expect(script).to.not.include('exit 2')
expect(script).to.not.include('exit 1')
})
})
})

describe('PRLT-1225: Layer 2 — Role prompt test enforcement guidance', () => {
describe('buildTestEnforcementGuidance', () => {
let guidance: string

before(() => {
guidance = buildTestEnforcementGuidance()
})

it('should include a mandatory heading', () => {
expect(guidance).to.include('Test Requirements')
expect(guidance).to.include('MANDATORY')
})

it('should require unit tests for new functions', () => {
expect(guidance).to.include('unit tests')
})

it('should require integration/e2e tests for new flows', () => {
expect(guidance).to.include('integration')
})

it('should mention test file locations', () => {
expect(guidance).to.include('test/unit/')
expect(guidance).to.include('test/e2e/')
})

it('should mention the pre-commit hook as enforcement', () => {
expect(guidance).to.include('pre-commit hook')
})

it('should tell agents to run tests before committing', () => {
expect(guidance).to.include('pnpm test:unit')
})
})

describe('buildPrompt injects test guidance', () => {
it('should include test guidance when modifiesCode is true', () => {
const prompt = buildPrompt(makeContext({ modifiesCode: true }))
expect(prompt).to.include('Test Requirements')
expect(prompt).to.include('MUST include tests')
})

it('should NOT include test guidance when modifiesCode is false', () => {
const prompt = buildPrompt(makeContext({ modifiesCode: false }))
expect(prompt).to.not.include('Test Requirements (MANDATORY)')
})

it('should NOT include test guidance when modifiesCode is undefined', () => {
const prompt = buildPrompt(makeContext())
expect(prompt).to.not.include('Test Requirements (MANDATORY)')
})

it('should NOT include test guidance in orchestrator prompts', () => {
const prompt = buildPrompt(makeContext({ isOrchestrator: true, modifiesCode: true }))
expect(prompt).to.not.include('Test Requirements (MANDATORY)')
})
})
})
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading