From dd5e6e61463e91b3aba1d2fef7daecb9ca437106 Mon Sep 17 00:00:00 2001 From: Cheffromspace Date: Tue, 3 Jun 2025 14:11:02 -0500 Subject: [PATCH] feat\!: Remove deprecated /api/claude endpoint in favor of webhook-based sessions (#172) BREAKING CHANGE: The /api/claude endpoint has been removed. All Claude API functionality is now available through the more robust /api/webhooks/claude endpoint. Migration guide: - For creating sessions: POST /api/webhooks/claude with type: 'session.create' - For checking status: POST /api/webhooks/claude with type: 'session.get' - Sessions now run asynchronously and return immediately with a session ID Changes: - Removed src/routes/claude.ts entirely - Removed related test files (claude.test.ts, claude-simple.test.ts) - Updated all documentation to use webhook endpoint - Updated test utilities to use async session API - Fixed formatting in modified files The webhook-based approach provides: - Async session management with immediate response - Better error handling and recovery - Session status tracking - Parallel session execution - Consistent API with other webhook operations --- CLAUDE.md | 4 +- README.md | 27 ++- src/index.ts | 2 - src/routes/claude.ts | 124 ----------- test/README.md | 24 ++- test/test-claude-api.js | 83 +++++--- test/unit/routes/claude-simple.test.ts | 119 ----------- test/unit/routes/claude.test.ts | 279 ------------------------- 8 files changed, 97 insertions(+), 565 deletions(-) delete mode 100644 src/routes/claude.ts delete mode 100644 test/unit/routes/claude-simple.test.ts delete mode 100644 test/unit/routes/claude.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 71869d1..9e4a0bd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,8 +56,8 @@ This repository contains a webhook service that integrates Claude with GitHub, a - Setup Claude authentication: `./scripts/setup/setup-claude-auth.sh` ### Testing Utilities -- Test Claude API directly: `node test/test-claude-api.js owner/repo` -- Test with container execution: `node test/test-claude-api.js owner/repo container "Your command here"` +- Test Claude webhook API (async): `node test/test-claude-api.js owner/repo async "Your command here"` +- Check session status: `node test/test-claude-api.js status ` - Test outgoing webhook: `node test/test-outgoing-webhook.js` - Test pre-commit hooks: `pre-commit run --all-files` - Test AWS credential provider: `node test/test-aws-credential-provider.js` diff --git a/README.md b/README.md index 30b38a5..fba99f2 100644 --- a/README.md +++ b/README.md @@ -206,16 +206,31 @@ AWS_SECRET_ACCESS_KEY=xxx ### Direct API Access -Integrate Claude without GitHub webhooks: +Create async Claude sessions via the webhook API: ```bash -curl -X POST http://localhost:3002/api/claude \ +# Create a new session +curl -X POST http://localhost:3002/api/webhooks/claude \ -H "Content-Type: application/json" \ + -H "Authorization: Bearer your-webhook-secret" \ -d '{ - "repoFullName": "owner/repo", - "command": "Analyze security vulnerabilities", - "authToken": "your-token", - "useContainer": true + "type": "session.create", + "session": { + "type": "implementation", + "project": { + "repository": "owner/repo", + "requirements": "Analyze security vulnerabilities" + } + } + }' + +# Check session status +curl -X POST http://localhost:3002/api/webhooks/claude \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer your-webhook-secret" \ + -d '{ + "type": "session.get", + "sessionId": "session-id-from-create" }' ``` diff --git a/src/index.ts b/src/index.ts index d6b83b9..3e5a0c4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,7 +5,6 @@ import rateLimit from 'express-rate-limit'; import { createLogger } from './utils/logger'; import { StartupMetrics } from './utils/startup-metrics'; import githubRoutes from './routes/github'; -import claudeRoutes from './routes/claude'; import webhookRoutes from './routes/webhooks'; import type { WebhookRequest, HealthCheckResponse, ErrorResponse } from './types/express'; import { execSync } from 'child_process'; @@ -102,7 +101,6 @@ startupMetrics.recordMilestone('middleware_configured', 'Express middleware conf // Routes app.use('/api/webhooks/github', githubRoutes); // Legacy endpoint app.use('/api/webhooks', webhookRoutes); // New modular webhook endpoint -app.use('/api/claude', claudeRoutes); startupMetrics.recordMilestone('routes_configured', 'API routes configured'); diff --git a/src/routes/claude.ts b/src/routes/claude.ts deleted file mode 100644 index 554c091..0000000 --- a/src/routes/claude.ts +++ /dev/null @@ -1,124 +0,0 @@ -import express from 'express'; -import { processCommand } from '../services/claudeService'; -import { createLogger } from '../utils/logger'; -import type { ClaudeAPIHandler } from '../types/express'; - -const router = express.Router(); -const logger = createLogger('claudeRoutes'); - -/** - * Direct endpoint for Claude processing - * Allows calling Claude without GitHub webhook integration - */ -const handleClaudeRequest: ClaudeAPIHandler = async (req, res) => { - logger.info({ request: req.body }, 'Received direct Claude request'); - try { - const { - repoFullName, - repository, - command, - authToken, - useContainer = false, - issueNumber, - isPullRequest = false, - branchName - } = req.body; - - // Handle both repoFullName and repository parameters - const repoName = repoFullName ?? repository; - - // Validate required parameters - if (!repoName) { - logger.warn('Missing repository name in request'); - return res.status(400).json({ error: 'Repository name is required' }); - } - - if (!command) { - logger.warn('Missing command in request'); - return res.status(400).json({ error: 'Command is required' }); - } - - // Validate authentication if enabled - if (process.env['CLAUDE_API_AUTH_REQUIRED'] === '1') { - if (!authToken || authToken !== process.env['CLAUDE_API_AUTH_TOKEN']) { - logger.warn('Invalid authentication token'); - return res.status(401).json({ error: 'Invalid authentication token' }); - } - } - - logger.info( - { - repo: repoName, - commandLength: command.length, - useContainer, - issueNumber, - isPullRequest - }, - 'Processing direct Claude command' - ); - - // Process the command with Claude - let claudeResponse: string; - try { - claudeResponse = await processCommand({ - repoFullName: repoName, - issueNumber: issueNumber ?? null, - command, - isPullRequest, - branchName: branchName ?? null - }); - - logger.debug( - { - responseType: typeof claudeResponse, - responseLength: claudeResponse ? claudeResponse.length : 0 - }, - 'Raw Claude response received' - ); - - // Force a default response if empty - if (!claudeResponse || claudeResponse.trim() === '') { - claudeResponse = - 'No output received from Claude container. This is a placeholder response.'; - } - } catch (processingError) { - const err = processingError as Error; - logger.error({ error: err }, 'Error during Claude processing'); - // When Claude processing fails, we still return 200 but with the error message - // This allows the webhook to complete successfully even if Claude had issues - claudeResponse = `Error: ${err.message}`; - } - - logger.info( - { - responseLength: claudeResponse ? claudeResponse.length : 0 - }, - 'Successfully processed Claude command' - ); - - return res.status(200).json({ - message: 'Command processed successfully', - response: claudeResponse - }); - } catch (error) { - const err = error as Error; - logger.error( - { - err: { - message: err.message, - stack: err.stack - } - }, - 'Error processing direct Claude command' - ); - - return res.status(500).json({ - error: 'Failed to process command', - message: err.message - }); - } -}; - -router.post('/', handleClaudeRequest as express.RequestHandler); - -export default router; diff --git a/test/README.md b/test/README.md index 6b56031..2fb77c0 100644 --- a/test/README.md +++ b/test/README.md @@ -107,13 +107,27 @@ Example: ```javascript // Test for Claude container execution describe('Container Execution E2E Tests', () => { - test('Should process a simple Claude request', async () => { - const response = await axios.post('/api/claude', { - command: 'Hello Claude', - repoFullName: 'test-org/test-repo' - }); + test('Should create a Claude session', async () => { + const response = await axios.post( + '/api/webhooks/claude', + { + type: 'session.create', + session: { + type: 'implementation', + project: { + repository: 'test-org/test-repo', + requirements: 'Hello Claude' + } + } + }, + { + headers: { Authorization: 'Bearer test-secret' } + } + ); expect(response.status).toBe(200); + expect(response.data.success).toBe(true); + expect(response.data.session.id).toBeDefined(); }); }); ``` diff --git a/test/test-claude-api.js b/test/test-claude-api.js index 72dfb39..218be9e 100644 --- a/test/test-claude-api.js +++ b/test/test-claude-api.js @@ -2,50 +2,77 @@ const axios = require('axios'); require('dotenv').config(); // Configuration -const apiUrl = process.env.API_URL || 'http://localhost:3003/api/claude'; -const authToken = process.env.CLAUDE_API_AUTH_TOKEN; +const apiUrl = process.env.API_URL || 'http://localhost:3003/api/webhooks/claude'; +const authToken = process.env.CLAUDE_WEBHOOK_SECRET || process.env.CLAUDE_API_AUTH_TOKEN; const repoFullName = process.argv[2] || 'test-org/test-repo'; -const useContainer = process.argv[3] === 'container'; +const asyncMode = process.argv[3] === 'async'; // The command to send to Claude const command = process.argv[4] || 'Explain what this repository does and list its main components'; console.log(` -Claude API Test Utility -======================= +Claude Webhook API Test Utility +============================== API URL: ${apiUrl} Repository: ${repoFullName} -Container: ${useContainer ? 'Yes' : 'No'} +Mode: ${asyncMode ? 'Async (session)' : 'Sync'} Auth Token: ${authToken ? '[REDACTED]' : 'Not provided'} Command: "${command}" `); -// Send the request to the Claude API -async function testClaudeApi() { +// Send the request to the Claude webhook API +async function testClaudeWebhook() { try { - console.log('Sending request to Claude API...'); + if (asyncMode) { + // Create a session + console.log('Creating Claude session...'); - const payload = { - repoFullName, - command, - useContainer - }; + const createPayload = { + type: 'session.create', + session: { + type: 'implementation', + project: { + repository: repoFullName, + requirements: command + } + } + }; - if (authToken) { - payload.authToken = authToken; + const headers = authToken ? { Authorization: `Bearer ${authToken}` } : {}; + + console.time('Session creation time'); + const createResponse = await axios.post(apiUrl, createPayload, { headers }); + console.timeEnd('Session creation time'); + + console.log('\nSession Created:', JSON.stringify(createResponse.data, null, 2)); + + if (createResponse.data.success && createResponse.data.session) { + const sessionId = createResponse.data.session.id; + console.log(`\nSession ID: ${sessionId}`); + console.log('Use the following command to check status:'); + console.log(`node test/test-claude-api.js status ${sessionId}`); + } + } else if (process.argv[2] === 'status' && process.argv[3]) { + // Check session status + const sessionId = process.argv[3]; + console.log(`Checking status for session: ${sessionId}`); + + const statusPayload = { + type: 'session.get', + sessionId + }; + + const headers = authToken ? { Authorization: `Bearer ${authToken}` } : {}; + + const statusResponse = await axios.post(apiUrl, statusPayload, { headers }); + console.log('\nSession Status:', JSON.stringify(statusResponse.data, null, 2)); + } else { + console.error('Synchronous mode is no longer supported.'); + console.error('Please use async mode: node test/test-claude-api.js async ""'); + console.error('Or check session status: node test/test-claude-api.js status '); } - - console.time('Claude processing time'); - const response = await axios.post(apiUrl, payload); - console.timeEnd('Claude processing time'); - - console.log('\nResponse Status:', response.status); - console.log('Full Response Data:', JSON.stringify(response.data, null, 2)); - console.log('\n--- Claude Response ---\n'); - console.log(response.data.response || 'No response received'); - console.log('\n--- End Response ---\n'); } catch (error) { - console.error('Error calling Claude API:', error.message); + console.error('Error calling Claude webhook API:', error.message); if (error.response) { console.error('Status:', error.response.status); @@ -55,4 +82,4 @@ async function testClaudeApi() { } // Run the test -testClaudeApi(); +testClaudeWebhook(); diff --git a/test/unit/routes/claude-simple.test.ts b/test/unit/routes/claude-simple.test.ts deleted file mode 100644 index 2ebadd0..0000000 --- a/test/unit/routes/claude-simple.test.ts +++ /dev/null @@ -1,119 +0,0 @@ -import express from 'express'; -import request from 'supertest'; - -// Mock dependencies first -jest.mock('../../../src/services/claudeService', () => ({ - processCommand: jest.fn().mockResolvedValue('Mock response') -})); - -jest.mock('../../../src/utils/logger', () => ({ - createLogger: jest.fn(() => ({ - info: jest.fn(), - warn: jest.fn(), - error: jest.fn(), - debug: jest.fn() - })) -})); - -describe('Claude Routes - Simple Coverage', () => { - let app: express.Application; - const mockProcessCommand = require('../../../src/services/claudeService').processCommand; - const originalEnv = process.env; - - beforeEach(() => { - jest.clearAllMocks(); - process.env = { ...originalEnv }; - app = express(); - app.use(express.json()); - - // Import the router fresh - jest.isolateModules(() => { - const claudeRouter = require('../../../src/routes/claude').default; - app.use('/api/claude', claudeRouter); - }); - }); - - afterEach(() => { - process.env = originalEnv; - }); - - it('should handle a basic request', async () => { - const response = await request(app).post('/api/claude').send({ - repository: 'test/repo', - command: 'test command' - }); - - expect(response.status).toBe(200); - expect(response.body.message).toBe('Command processed successfully'); - }); - - it('should handle missing repository', async () => { - const response = await request(app).post('/api/claude').send({ - command: 'test command' - }); - - expect(response.status).toBe(400); - expect(response.body.error).toBe('Repository name is required'); - }); - - it('should handle missing command', async () => { - const response = await request(app).post('/api/claude').send({ - repository: 'test/repo' - }); - - expect(response.status).toBe(400); - expect(response.body.error).toBe('Command is required'); - }); - - it('should validate authentication when required', async () => { - process.env.CLAUDE_API_AUTH_REQUIRED = '1'; - process.env.CLAUDE_API_AUTH_TOKEN = 'secret-token'; - - const response = await request(app).post('/api/claude').send({ - repository: 'test/repo', - command: 'test command' - }); - - expect(response.status).toBe(401); - expect(response.body.error).toBe('Invalid authentication token'); - }); - - it('should accept valid authentication', async () => { - process.env.CLAUDE_API_AUTH_REQUIRED = '1'; - process.env.CLAUDE_API_AUTH_TOKEN = 'secret-token'; - - const response = await request(app).post('/api/claude').send({ - repository: 'test/repo', - command: 'test command', - authToken: 'secret-token' - }); - - expect(response.status).toBe(200); - }); - - it('should handle empty response from Claude', async () => { - mockProcessCommand.mockResolvedValueOnce(''); - - const response = await request(app).post('/api/claude').send({ - repository: 'test/repo', - command: 'test command' - }); - - expect(response.status).toBe(200); - expect(response.body.response).toBe( - 'No output received from Claude container. This is a placeholder response.' - ); - }); - - it('should handle Claude processing error', async () => { - mockProcessCommand.mockRejectedValueOnce(new Error('Processing failed')); - - const response = await request(app).post('/api/claude').send({ - repository: 'test/repo', - command: 'test command' - }); - - expect(response.status).toBe(200); - expect(response.body.response).toBe('Error: Processing failed'); - }); -}); diff --git a/test/unit/routes/claude.test.ts b/test/unit/routes/claude.test.ts deleted file mode 100644 index 6a5971f..0000000 --- a/test/unit/routes/claude.test.ts +++ /dev/null @@ -1,279 +0,0 @@ -import request from 'supertest'; -import express from 'express'; - -// Mock dependencies before imports -jest.mock('../../../src/services/claudeService'); -jest.mock('../../../src/utils/logger'); - -const mockProcessCommand = jest.fn<() => Promise>(); -jest.mocked(require('../../../src/services/claudeService')).processCommand = mockProcessCommand; - -interface MockLogger { - info: jest.Mock; - warn: jest.Mock; - error: jest.Mock; - debug: jest.Mock; -} - -const mockLogger: MockLogger = { - info: jest.fn(), - warn: jest.fn(), - error: jest.fn(), - debug: jest.fn() -}; -jest.mocked(require('../../../src/utils/logger')).createLogger = jest.fn(() => mockLogger); - -// Import router after mocks are set up -import claudeRouter from '../../../src/routes/claude'; - -describe('Claude Routes', () => { - let app: express.Application; - const originalEnv = process.env; - - beforeEach(() => { - jest.clearAllMocks(); - process.env = { ...originalEnv }; - - app = express(); - app.use(express.json()); - app.use('/api/claude', claudeRouter); - }); - - afterEach(() => { - process.env = originalEnv; - }); - - describe('POST /api/claude', () => { - it('should process valid Claude request with repository and command', async () => { - mockProcessCommand.mockResolvedValue('Claude response'); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - expect(response.body).toEqual({ - message: 'Command processed successfully', - response: 'Claude response' - }); - - expect(mockProcessCommand).toHaveBeenCalledWith({ - repoFullName: 'owner/repo', - issueNumber: null, - command: 'Test command', - isPullRequest: false, - branchName: null - }); - - expect(mockLogger.info).toHaveBeenCalledWith( - expect.objectContaining({ request: expect.any(Object) }), - 'Received direct Claude request' - ); - }); - - it('should handle repoFullName parameter as alternative to repository', async () => { - mockProcessCommand.mockResolvedValue('Claude response'); - - const response = await request(app).post('/api/claude').send({ - repoFullName: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - expect(mockProcessCommand).toHaveBeenCalledWith( - expect.objectContaining({ - repoFullName: 'owner/repo' - }) - ); - }); - - it('should process request with all optional parameters', async () => { - mockProcessCommand.mockResolvedValue('Claude response'); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command', - useContainer: true, - issueNumber: 42, - isPullRequest: true, - branchName: 'feature-branch' - }); - - expect(response.status).toBe(200); - expect(mockProcessCommand).toHaveBeenCalledWith({ - repoFullName: 'owner/repo', - issueNumber: 42, - command: 'Test command', - isPullRequest: true, - branchName: 'feature-branch' - }); - - expect(mockLogger.info).toHaveBeenCalledWith( - expect.objectContaining({ - repo: 'owner/repo', - commandLength: 12, - useContainer: true, - issueNumber: 42, - isPullRequest: true - }), - 'Processing direct Claude command' - ); - }); - - it('should return 400 when repository is missing', async () => { - const response = await request(app).post('/api/claude').send({ - command: 'Test command' - }); - - expect(response.status).toBe(400); - expect(response.body).toEqual({ - error: 'Repository name is required' - }); - - expect(mockLogger.warn).toHaveBeenCalledWith('Missing repository name in request'); - expect(mockProcessCommand).not.toHaveBeenCalled(); - }); - - it('should return 400 when command is missing', async () => { - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo' - }); - - expect(response.status).toBe(400); - expect(response.body).toEqual({ - error: 'Command is required' - }); - - expect(mockLogger.warn).toHaveBeenCalledWith('Missing command in request'); - expect(mockProcessCommand).not.toHaveBeenCalled(); - }); - - it('should validate authentication when required', async () => { - process.env.CLAUDE_API_AUTH_REQUIRED = '1'; - process.env.CLAUDE_API_AUTH_TOKEN = 'secret-token'; - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command', - authToken: 'wrong-token' - }); - - expect(response.status).toBe(401); - expect(response.body).toEqual({ - error: 'Invalid authentication token' - }); - - expect(mockLogger.warn).toHaveBeenCalledWith('Invalid authentication token'); - expect(mockProcessCommand).not.toHaveBeenCalled(); - }); - - it('should accept valid authentication token', async () => { - process.env.CLAUDE_API_AUTH_REQUIRED = '1'; - process.env.CLAUDE_API_AUTH_TOKEN = 'secret-token'; - mockProcessCommand.mockResolvedValue('Authenticated response'); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command', - authToken: 'secret-token' - }); - - expect(response.status).toBe(200); - expect(response.body.response).toBe('Authenticated response'); - }); - - it('should skip authentication when not required', async () => { - process.env.CLAUDE_API_AUTH_REQUIRED = '0'; - mockProcessCommand.mockResolvedValue('Response'); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - }); - - it('should handle empty Claude response with default message', async () => { - mockProcessCommand.mockResolvedValue(''); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - expect(response.body.response).toBe( - 'No output received from Claude container. This is a placeholder response.' - ); - }); - - it('should handle whitespace-only Claude response', async () => { - mockProcessCommand.mockResolvedValue(' \n\t '); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - expect(response.body.response).toBe( - 'No output received from Claude container. This is a placeholder response.' - ); - }); - - it('should handle Claude processing errors gracefully', async () => { - const error = new Error('Claude processing failed'); - mockProcessCommand.mockRejectedValue(error); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - expect(response.body).toEqual({ - message: 'Command processed successfully', - response: 'Error: Claude processing failed' - }); - - expect(mockLogger.error).toHaveBeenCalledWith({ error }, 'Error during Claude processing'); - }); - - it('should log debug information about Claude response', async () => { - mockProcessCommand.mockResolvedValue('Test response content'); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - expect(mockLogger.debug).toHaveBeenCalledWith( - { - responseType: 'string', - responseLength: 21 - }, - 'Raw Claude response received' - ); - }); - - it('should log successful completion', async () => { - mockProcessCommand.mockResolvedValue('Response'); - - const response = await request(app).post('/api/claude').send({ - repository: 'owner/repo', - command: 'Test command' - }); - - expect(response.status).toBe(200); - expect(mockLogger.info).toHaveBeenCalledWith( - { - responseLength: 8 - }, - 'Successfully processed Claude command' - ); - }); - }); -});