diff --git a/CLAUDE.md b/CLAUDE.md index f1bdf840..33bc0913 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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). diff --git a/apps/cli/src/lib/execution/runners/docker-management.ts b/apps/cli/src/lib/execution/runners/docker-management.ts index 5a0c6f65..2e23df68 100644 --- a/apps/cli/src/lib/execution/runners/docker-management.ts +++ b/apps/cli/src/lib/execution/runners/docker-management.ts @@ -386,10 +386,58 @@ export function buildClaudeLifecycleHooks(): Record { ], }, ], + // 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. */ @@ -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 @@ -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) } diff --git a/apps/cli/src/lib/execution/runners/prompt-builder.ts b/apps/cli/src/lib/execution/runners/prompt-builder.ts index 1f0a6fd9..338702c2 100644 --- a/apps/cli/src/lib/execution/runners/prompt-builder.ts +++ b/apps/cli/src/lib/execution/runners/prompt-builder.ts @@ -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) // ============================================================================= @@ -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` } diff --git a/apps/cli/test/unit/agent-lifecycle-hooks.test.ts b/apps/cli/test/unit/agent-lifecycle-hooks.test.ts index 07d080c2..3062c265 100644 --- a/apps/cli/test/unit/agent-lifecycle-hooks.test.ts +++ b/apps/cli/test/unit/agent-lifecycle-hooks.test.ts @@ -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' @@ -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`) @@ -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 }>> } - 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', @@ -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 }>> + } + 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 }>> + } + 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 }>> + } + 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 }>> + } + 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)') + }) + }) +}) diff --git a/package-lock.json b/package-lock.json index 3374228f..1a6909de 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,7 @@ }, "apps/cli": { "name": "@proletariat/cli", - "version": "0.3.102", + "version": "0.3.103", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": {