diff --git a/apps/server/src/routes/app-spec/common.ts b/apps/server/src/routes/app-spec/common.ts index 9c651d29..3ee78009 100644 --- a/apps/server/src/routes/app-spec/common.ts +++ b/apps/server/src/routes/app-spec/common.ts @@ -3,13 +3,22 @@ */ import { createLogger } from "../../lib/logger.js"; -import { getErrorMessage as getErrorMessageShared } from "../common.js"; const logger = createLogger("SpecRegeneration"); -// Shared state for tracking generation status -export let isRunning = false; -export let currentAbortController: AbortController | null = null; +// Shared state for tracking generation status - private +let isRunning = false; +let currentAbortController: AbortController | null = null; + +/** + * Get the current running state + */ +export function getSpecRegenerationStatus(): { + isRunning: boolean; + currentAbortController: AbortController | null; +} { + return { isRunning, currentAbortController }; +} /** * Set the running state and abort controller diff --git a/apps/server/src/routes/app-spec/routes/create.ts b/apps/server/src/routes/app-spec/routes/create.ts index 3fcb48c1..89d6430e 100644 --- a/apps/server/src/routes/app-spec/routes/create.ts +++ b/apps/server/src/routes/app-spec/routes/create.ts @@ -6,7 +6,7 @@ import type { Request, Response } from "express"; import type { EventEmitter } from "../../../lib/events.js"; import { createLogger } from "../../../lib/logger.js"; import { - isRunning, + getSpecRegenerationStatus, setRunningState, logAuthStatus, logError, @@ -48,6 +48,7 @@ export function createCreateHandler(events: EventEmitter) { return; } + const { isRunning } = getSpecRegenerationStatus(); if (isRunning) { logger.warn("Generation already running, rejecting request"); res.json({ success: false, error: "Spec generation already running" }); diff --git a/apps/server/src/routes/app-spec/routes/generate-features.ts b/apps/server/src/routes/app-spec/routes/generate-features.ts index 12a7e2f2..91f3b9b5 100644 --- a/apps/server/src/routes/app-spec/routes/generate-features.ts +++ b/apps/server/src/routes/app-spec/routes/generate-features.ts @@ -6,7 +6,7 @@ import type { Request, Response } from "express"; import type { EventEmitter } from "../../../lib/events.js"; import { createLogger } from "../../../lib/logger.js"; import { - isRunning, + getSpecRegenerationStatus, setRunningState, logAuthStatus, logError, @@ -32,6 +32,7 @@ export function createGenerateFeaturesHandler(events: EventEmitter) { return; } + const { isRunning } = getSpecRegenerationStatus(); if (isRunning) { logger.warn("Generation already running, rejecting request"); res.json({ success: false, error: "Generation already running" }); diff --git a/apps/server/src/routes/app-spec/routes/generate.ts b/apps/server/src/routes/app-spec/routes/generate.ts index 5a73aa93..428de409 100644 --- a/apps/server/src/routes/app-spec/routes/generate.ts +++ b/apps/server/src/routes/app-spec/routes/generate.ts @@ -6,7 +6,7 @@ import type { Request, Response } from "express"; import type { EventEmitter } from "../../../lib/events.js"; import { createLogger } from "../../../lib/logger.js"; import { - isRunning, + getSpecRegenerationStatus, setRunningState, logAuthStatus, logError, @@ -52,6 +52,7 @@ export function createGenerateHandler(events: EventEmitter) { return; } + const { isRunning } = getSpecRegenerationStatus(); if (isRunning) { logger.warn("Generation already running, rejecting request"); res.json({ success: false, error: "Spec generation already running" }); diff --git a/apps/server/src/routes/app-spec/routes/status.ts b/apps/server/src/routes/app-spec/routes/status.ts index a39a3ebe..1ecc6ab2 100644 --- a/apps/server/src/routes/app-spec/routes/status.ts +++ b/apps/server/src/routes/app-spec/routes/status.ts @@ -3,11 +3,15 @@ */ import type { Request, Response } from "express"; -import { isRunning, getErrorMessage } from "../common.js"; +import { + getSpecRegenerationStatus, + getErrorMessage, +} from "../common.js"; export function createStatusHandler() { return async (_req: Request, res: Response): Promise => { try { + const { isRunning } = getSpecRegenerationStatus(); res.json({ success: true, isRunning }); } catch (error) { res.status(500).json({ success: false, error: getErrorMessage(error) }); diff --git a/apps/server/src/routes/app-spec/routes/stop.ts b/apps/server/src/routes/app-spec/routes/stop.ts index e9221b92..7c3bd5ca 100644 --- a/apps/server/src/routes/app-spec/routes/stop.ts +++ b/apps/server/src/routes/app-spec/routes/stop.ts @@ -4,7 +4,7 @@ import type { Request, Response } from "express"; import { - currentAbortController, + getSpecRegenerationStatus, setRunningState, getErrorMessage, } from "../common.js"; @@ -12,6 +12,7 @@ import { export function createStopHandler() { return async (_req: Request, res: Response): Promise => { try { + const { currentAbortController } = getSpecRegenerationStatus(); if (currentAbortController) { currentAbortController.abort(); } diff --git a/apps/server/src/routes/setup/common.ts b/apps/server/src/routes/setup/common.ts index 55918c03..5ea3a584 100644 --- a/apps/server/src/routes/setup/common.ts +++ b/apps/server/src/routes/setup/common.ts @@ -12,8 +12,29 @@ import { const logger = createLogger("Setup"); -// Storage for API keys (in-memory cache) -export const apiKeys: Record = {}; +// Storage for API keys (in-memory cache) - private +const apiKeys: Record = {}; + +/** + * Get an API key for a provider + */ +export function getApiKey(provider: string): string | undefined { + return apiKeys[provider]; +} + +/** + * Set an API key for a provider + */ +export function setApiKey(provider: string, key: string): void { + apiKeys[provider] = key; +} + +/** + * Get all API keys (for read-only access) + */ +export function getAllApiKeys(): Record { + return { ...apiKeys }; +} /** * Helper to persist API keys to .env file diff --git a/apps/server/src/routes/setup/get-claude-status.ts b/apps/server/src/routes/setup/get-claude-status.ts index e9492822..f999625a 100644 --- a/apps/server/src/routes/setup/get-claude-status.ts +++ b/apps/server/src/routes/setup/get-claude-status.ts @@ -7,7 +7,7 @@ import { promisify } from "util"; import os from "os"; import path from "path"; import fs from "fs/promises"; -import { apiKeys } from "./common.js"; +import { getApiKey } from "./common.js"; const execAsync = promisify(exec); @@ -71,8 +71,8 @@ export async function getClaudeStatus() { method: "none" as string, hasCredentialsFile: false, hasToken: false, - hasStoredOAuthToken: !!apiKeys.anthropic_oauth_token, - hasStoredApiKey: !!apiKeys.anthropic, + hasStoredOAuthToken: !!getApiKey("anthropic_oauth_token"), + hasStoredApiKey: !!getApiKey("anthropic"), hasEnvApiKey: !!process.env.ANTHROPIC_API_KEY, hasEnvOAuthToken: !!process.env.CLAUDE_CODE_OAUTH_TOKEN, // Additional fields for detailed status @@ -159,14 +159,14 @@ export async function getClaudeStatus() { } // In-memory stored OAuth token (from setup wizard - subscription auth) - if (!auth.authenticated && apiKeys.anthropic_oauth_token) { + if (!auth.authenticated && getApiKey("anthropic_oauth_token")) { auth.authenticated = true; auth.oauthTokenValid = true; auth.method = "oauth_token"; // Stored OAuth token from setup wizard } // In-memory stored API key (from settings UI - pay-per-use) - if (!auth.authenticated && apiKeys.anthropic) { + if (!auth.authenticated && getApiKey("anthropic")) { auth.authenticated = true; auth.apiKeyValid = true; auth.method = "api_key"; // Manually stored API key diff --git a/apps/server/src/routes/setup/routes/api-keys.ts b/apps/server/src/routes/setup/routes/api-keys.ts index 1fbeb886..b2b09d27 100644 --- a/apps/server/src/routes/setup/routes/api-keys.ts +++ b/apps/server/src/routes/setup/routes/api-keys.ts @@ -3,15 +3,19 @@ */ import type { Request, Response } from "express"; -import { apiKeys, getErrorMessage, logError } from "../common.js"; +import { + getApiKey, + getErrorMessage, + logError, +} from "../common.js"; export function createApiKeysHandler() { return async (_req: Request, res: Response): Promise => { try { res.json({ success: true, - hasAnthropicKey: !!apiKeys.anthropic || !!process.env.ANTHROPIC_API_KEY, - hasGoogleKey: !!apiKeys.google || !!process.env.GOOGLE_API_KEY, + hasAnthropicKey: !!getApiKey("anthropic") || !!process.env.ANTHROPIC_API_KEY, + hasGoogleKey: !!getApiKey("google") || !!process.env.GOOGLE_API_KEY, }); } catch (error) { logError(error, "Get API keys failed"); diff --git a/apps/server/src/routes/setup/routes/store-api-key.ts b/apps/server/src/routes/setup/routes/store-api-key.ts index 5f0e6b44..1f34f22d 100644 --- a/apps/server/src/routes/setup/routes/store-api-key.ts +++ b/apps/server/src/routes/setup/routes/store-api-key.ts @@ -4,7 +4,7 @@ import type { Request, Response } from "express"; import { - apiKeys, + setApiKey, persistApiKeyToEnv, getErrorMessage, logError, @@ -28,7 +28,7 @@ export function createStoreApiKeyHandler() { return; } - apiKeys[provider] = apiKey; + setApiKey(provider, apiKey); // Also set as environment variable and persist to .env // IMPORTANT: OAuth tokens and API keys must be stored separately diff --git a/apps/server/src/routes/suggestions/common.ts b/apps/server/src/routes/suggestions/common.ts index c63d0613..b291c5ae 100644 --- a/apps/server/src/routes/suggestions/common.ts +++ b/apps/server/src/routes/suggestions/common.ts @@ -10,9 +10,19 @@ import { const logger = createLogger("Suggestions"); -// Shared state for tracking generation status -export let isRunning = false; -export let currentAbortController: AbortController | null = null; +// Shared state for tracking generation status - private +let isRunning = false; +let currentAbortController: AbortController | null = null; + +/** + * Get the current running state + */ +export function getSuggestionsStatus(): { + isRunning: boolean; + currentAbortController: AbortController | null; +} { + return { isRunning, currentAbortController }; +} /** * Set the running state and abort controller diff --git a/apps/server/src/routes/suggestions/routes/generate.ts b/apps/server/src/routes/suggestions/routes/generate.ts index 0e7998a9..beafd10f 100644 --- a/apps/server/src/routes/suggestions/routes/generate.ts +++ b/apps/server/src/routes/suggestions/routes/generate.ts @@ -6,7 +6,7 @@ import type { Request, Response } from "express"; import type { EventEmitter } from "../../../lib/events.js"; import { createLogger } from "../../../lib/logger.js"; import { - isRunning, + getSuggestionsStatus, setRunningState, getErrorMessage, logError, @@ -28,6 +28,7 @@ export function createGenerateHandler(events: EventEmitter) { return; } + const { isRunning } = getSuggestionsStatus(); if (isRunning) { res.json({ success: false, diff --git a/apps/server/src/routes/suggestions/routes/status.ts b/apps/server/src/routes/suggestions/routes/status.ts index 72981b6d..b7492d87 100644 --- a/apps/server/src/routes/suggestions/routes/status.ts +++ b/apps/server/src/routes/suggestions/routes/status.ts @@ -3,11 +3,16 @@ */ import type { Request, Response } from "express"; -import { isRunning, getErrorMessage, logError } from "../common.js"; +import { + getSuggestionsStatus, + getErrorMessage, + logError, +} from "../common.js"; export function createStatusHandler() { return async (_req: Request, res: Response): Promise => { try { + const { isRunning } = getSuggestionsStatus(); res.json({ success: true, isRunning }); } catch (error) { logError(error, "Get status failed"); diff --git a/apps/server/src/routes/suggestions/routes/stop.ts b/apps/server/src/routes/suggestions/routes/stop.ts index 6871b3da..3a18a0be 100644 --- a/apps/server/src/routes/suggestions/routes/stop.ts +++ b/apps/server/src/routes/suggestions/routes/stop.ts @@ -4,7 +4,7 @@ import type { Request, Response } from "express"; import { - currentAbortController, + getSuggestionsStatus, setRunningState, getErrorMessage, logError, @@ -13,6 +13,7 @@ import { export function createStopHandler() { return async (_req: Request, res: Response): Promise => { try { + const { currentAbortController } = getSuggestionsStatus(); if (currentAbortController) { currentAbortController.abort(); } diff --git a/apps/server/src/routes/terminal/common.ts b/apps/server/src/routes/terminal/common.ts index 1a2861a7..80b3a496 100644 --- a/apps/server/src/routes/terminal/common.ts +++ b/apps/server/src/routes/terminal/common.ts @@ -17,11 +17,37 @@ function getTerminalEnabledConfig(): boolean { return process.env.TERMINAL_ENABLED !== "false"; // Enabled by default } -// In-memory session tokens (would use Redis in production) -export const validTokens: Map = +// In-memory session tokens (would use Redis in production) - private +const validTokens: Map = new Map(); const TOKEN_EXPIRY_MS = 24 * 60 * 60 * 1000; // 24 hours +/** + * Add a token to the valid tokens map + */ +export function addToken( + token: string, + data: { createdAt: Date; expiresAt: Date } +): void { + validTokens.set(token, data); +} + +/** + * Delete a token from the valid tokens map + */ +export function deleteToken(token: string): void { + validTokens.delete(token); +} + +/** + * Get token data for a given token + */ +export function getTokenData( + token: string +): { createdAt: Date; expiresAt: Date } | undefined { + return validTokens.get(token); +} + /** * Generate a secure random token */ diff --git a/apps/server/src/routes/terminal/routes/auth.ts b/apps/server/src/routes/terminal/routes/auth.ts index d1e18ae5..234d4572 100644 --- a/apps/server/src/routes/terminal/routes/auth.ts +++ b/apps/server/src/routes/terminal/routes/auth.ts @@ -7,7 +7,7 @@ import { getTerminalEnabledConfigValue, getTerminalPasswordConfig, generateToken, - validTokens, + addToken, getTokenExpiryMs, getErrorMessage, } from "../common.js"; @@ -49,7 +49,7 @@ export function createAuthHandler() { // Generate session token const token = generateToken(); const now = new Date(); - validTokens.set(token, { + addToken(token, { createdAt: now, expiresAt: new Date(now.getTime() + getTokenExpiryMs()), }); diff --git a/apps/server/src/routes/terminal/routes/logout.ts b/apps/server/src/routes/terminal/routes/logout.ts index 919fc28e..9e3c8fa3 100644 --- a/apps/server/src/routes/terminal/routes/logout.ts +++ b/apps/server/src/routes/terminal/routes/logout.ts @@ -3,14 +3,14 @@ */ import type { Request, Response } from "express"; -import { validTokens } from "../common.js"; +import { deleteToken } from "../common.js"; export function createLogoutHandler() { return (req: Request, res: Response): void => { const token = (req.headers["x-terminal-token"] as string) || req.body.token; if (token) { - validTokens.delete(token); + deleteToken(token); } res.json({ diff --git a/docs/pr-comment-fix-agent.md b/docs/pr-comment-fix-agent.md new file mode 100644 index 00000000..46099277 --- /dev/null +++ b/docs/pr-comment-fix-agent.md @@ -0,0 +1,152 @@ +# PR Comment Fix Agent Instructions + +## Overview + +This agent automatically reviews a GitHub Pull Request, analyzes all comments, and systematically addresses each comment by making the necessary code changes. + +## Workflow + +### Step 1: Fetch PR Information + +1. Use the GitHub CLI command: `gh pr view --comments --json number,title,body,comments,headRefName,baseRefName` +2. Parse the JSON output to extract: + - PR number and title + - PR description/body + - All comments (including review comments and inline comments) + - Branch information (head and base branches) + +### Step 2: Analyze Comments + +For each comment, identify: + +- **Type**: Review comment, inline comment, or general comment +- **File path**: If it's an inline comment, extract the file path +- **Line number**: If it's an inline comment, extract the line number(s) +- **Intent**: What change is being requested? + - Bug fix + - Code style/formatting + - Performance improvement + - Refactoring + - Missing functionality + - Documentation update + - Test addition/modification +- **Priority**: Determine if it's blocking (must fix) or non-blocking (nice to have) + +### Step 3: Checkout PR Branch + +1. Ensure you're in the correct repository +2. Fetch the latest changes: `git fetch origin` +3. Checkout the PR branch: `git checkout ` +4. Pull latest changes: `git pull origin ` + +### Step 4: Address Each Comment Systematically + +For each comment, follow this process: + +#### 4.1 Read Relevant Files + +- If the comment references a specific file, read that file first +- If the comment is general, read related files based on context +- Understand the current implementation + +#### 4.2 Understand the Request + +- Parse what specific change is needed +- Identify the root cause or issue +- Consider edge cases and implications + +#### 4.3 Make the Fix + +- Implement the requested change +- Ensure the fix addresses the exact concern raised +- Maintain code consistency with the rest of the codebase +- Follow existing code style and patterns + +#### 4.4 Verify the Fix + +- Check that the change resolves the comment +- Ensure no new issues are introduced +- Run relevant tests if available +- Check for linting errors + +### Step 5: Document Changes + +For each comment addressed: + +- Add a comment or commit message referencing the PR comment +- If multiple comments are addressed, group related changes logically + +### Step 6: Commit Changes + +1. Stage all changes: `git add -A` +2. Create a commit with a descriptive message: + + ``` + fix: address PR review comments + + - [Brief description of fix 1] (addresses comment #X) + - [Brief description of fix 2] (addresses comment #Y) + - ... + ``` + +3. Push changes: `git push origin ` + +## Comment Types and Handling + +### Inline Code Comments + +- **Location**: Specific file and line number +- **Action**: Read the file, locate the exact line, understand context, make targeted fix +- **Example**: "This function should handle null values" → Add null check + +### Review Comments + +- **Location**: May reference multiple files or general patterns +- **Action**: Read all referenced files, understand the pattern, apply fix consistently +- **Example**: "We should use async/await instead of promises" → Refactor all instances + +### General Comments + +- **Location**: PR-level, not file-specific +- **Action**: Understand the broader concern, identify affected areas, make comprehensive changes +- **Example**: "Add error handling" → Review entire PR for missing error handling + +## Best Practices + +1. **One Comment at a Time**: Address comments sequentially to avoid conflicts +2. **Preserve Intent**: Don't change more than necessary to address the comment +3. **Test Changes**: Run tests after each significant change +4. **Ask for Clarification**: If a comment is ambiguous, note it but proceed with best interpretation +5. **Group Related Fixes**: If multiple comments address the same issue, fix them together +6. **Maintain Style**: Follow existing code style, formatting, and patterns +7. **Check Dependencies**: Ensure fixes don't break other parts of the codebase + +## Error Handling + +- If a comment references a file that doesn't exist, note it and skip +- If a line number is out of range (file changed), search for similar code nearby +- If a fix introduces breaking changes, revert and try a different approach +- If tests fail after a fix, investigate and adjust the implementation + +## Completion Criteria + +The agent has successfully completed when: + +1. All comments have been analyzed +2. All actionable comments have been addressed with code changes +3. All changes have been committed and pushed +4. A summary of addressed comments is provided + +## Example Output Summary + +``` +PR #123 Review Comments - Addressed + +✅ Comment #1: Fixed null handling in getUserData() (line 45) +✅ Comment #2: Added error handling for API calls +✅ Comment #3: Refactored to use async/await pattern +⚠️ Comment #4: Requires clarification - noted in commit message +✅ Comment #5: Fixed typo in documentation + +Total: 5 comments, 4 addressed, 1 requires clarification +``` diff --git a/docs/pr-comment-fix-prompt.md b/docs/pr-comment-fix-prompt.md new file mode 100644 index 00000000..51bd6b64 --- /dev/null +++ b/docs/pr-comment-fix-prompt.md @@ -0,0 +1,51 @@ +# PR Comment Fix Agent Prompt + +Use this prompt directly with an AI agent (like Claude Code) to automatically fix PR review comments. + +## Direct Prompt + +``` +You are an AI agent tasked with reviewing and fixing GitHub Pull Request review comments. + +Your task: +1. Fetch PR information: Run `gh pr view --comments --json number,title,body,comments,headRefName,baseRefName` +2. Parse all comments from the JSON output +3. Checkout the PR branch: `git checkout ` and `git pull origin ` +4. For each comment: + - Identify the file and line number (if applicable) + - Understand what change is being requested + - Read the relevant file(s) + - Make the necessary code changes to address the comment + - Verify the fix doesn't break existing functionality +5. Commit all changes with a descriptive message referencing the PR comments +6. Push changes back to the PR branch + +Guidelines: +- Address comments one at a time, but group related fixes +- Preserve existing code style and patterns +- Don't change more than necessary to address each comment +- Run tests if available after making changes +- If a comment is unclear, make your best interpretation and note it +- Create a summary of all comments addressed + +Start by asking for the PR number, then proceed with the workflow above. +``` + +## Usage Example + +``` +I need you to fix all review comments on PR #123. + +[Paste the prompt above] + +PR number: 123 +``` + +## Alternative: One-Liner Version + +``` +Review PR # comments using `gh pr view --comments`, checkout the PR branch, +read each comment, understand what needs to be fixed, make the code changes to address each +comment, commit with descriptive messages, and push back to the branch. Provide a summary of +all comments addressed. +```