mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-01-30 06:12:03 +00:00
refactor: encapsulate state management for spec and suggestions generation
- Made the generation status variables private and introduced getter functions for both spec and suggestions generation states. - Updated relevant route handlers to utilize the new getter functions, improving encapsulation and reducing direct access to shared state. - Enhanced code maintainability by centralizing state management logic.
This commit is contained in:
@@ -3,13 +3,22 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import { createLogger } from "../../lib/logger.js";
|
import { createLogger } from "../../lib/logger.js";
|
||||||
import { getErrorMessage as getErrorMessageShared } from "../common.js";
|
|
||||||
|
|
||||||
const logger = createLogger("SpecRegeneration");
|
const logger = createLogger("SpecRegeneration");
|
||||||
|
|
||||||
// Shared state for tracking generation status
|
// Shared state for tracking generation status - private
|
||||||
export let isRunning = false;
|
let isRunning = false;
|
||||||
export let currentAbortController: AbortController | null = null;
|
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
|
* Set the running state and abort controller
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import type { Request, Response } from "express";
|
|||||||
import type { EventEmitter } from "../../../lib/events.js";
|
import type { EventEmitter } from "../../../lib/events.js";
|
||||||
import { createLogger } from "../../../lib/logger.js";
|
import { createLogger } from "../../../lib/logger.js";
|
||||||
import {
|
import {
|
||||||
isRunning,
|
getSpecRegenerationStatus,
|
||||||
setRunningState,
|
setRunningState,
|
||||||
logAuthStatus,
|
logAuthStatus,
|
||||||
logError,
|
logError,
|
||||||
@@ -48,6 +48,7 @@ export function createCreateHandler(events: EventEmitter) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const { isRunning } = getSpecRegenerationStatus();
|
||||||
if (isRunning) {
|
if (isRunning) {
|
||||||
logger.warn("Generation already running, rejecting request");
|
logger.warn("Generation already running, rejecting request");
|
||||||
res.json({ success: false, error: "Spec generation already running" });
|
res.json({ success: false, error: "Spec generation already running" });
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import type { Request, Response } from "express";
|
|||||||
import type { EventEmitter } from "../../../lib/events.js";
|
import type { EventEmitter } from "../../../lib/events.js";
|
||||||
import { createLogger } from "../../../lib/logger.js";
|
import { createLogger } from "../../../lib/logger.js";
|
||||||
import {
|
import {
|
||||||
isRunning,
|
getSpecRegenerationStatus,
|
||||||
setRunningState,
|
setRunningState,
|
||||||
logAuthStatus,
|
logAuthStatus,
|
||||||
logError,
|
logError,
|
||||||
@@ -32,6 +32,7 @@ export function createGenerateFeaturesHandler(events: EventEmitter) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const { isRunning } = getSpecRegenerationStatus();
|
||||||
if (isRunning) {
|
if (isRunning) {
|
||||||
logger.warn("Generation already running, rejecting request");
|
logger.warn("Generation already running, rejecting request");
|
||||||
res.json({ success: false, error: "Generation already running" });
|
res.json({ success: false, error: "Generation already running" });
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import type { Request, Response } from "express";
|
|||||||
import type { EventEmitter } from "../../../lib/events.js";
|
import type { EventEmitter } from "../../../lib/events.js";
|
||||||
import { createLogger } from "../../../lib/logger.js";
|
import { createLogger } from "../../../lib/logger.js";
|
||||||
import {
|
import {
|
||||||
isRunning,
|
getSpecRegenerationStatus,
|
||||||
setRunningState,
|
setRunningState,
|
||||||
logAuthStatus,
|
logAuthStatus,
|
||||||
logError,
|
logError,
|
||||||
@@ -52,6 +52,7 @@ export function createGenerateHandler(events: EventEmitter) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const { isRunning } = getSpecRegenerationStatus();
|
||||||
if (isRunning) {
|
if (isRunning) {
|
||||||
logger.warn("Generation already running, rejecting request");
|
logger.warn("Generation already running, rejecting request");
|
||||||
res.json({ success: false, error: "Spec generation already running" });
|
res.json({ success: false, error: "Spec generation already running" });
|
||||||
|
|||||||
@@ -3,11 +3,15 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import type { Request, Response } from "express";
|
import type { Request, Response } from "express";
|
||||||
import { isRunning, getErrorMessage } from "../common.js";
|
import {
|
||||||
|
getSpecRegenerationStatus,
|
||||||
|
getErrorMessage,
|
||||||
|
} from "../common.js";
|
||||||
|
|
||||||
export function createStatusHandler() {
|
export function createStatusHandler() {
|
||||||
return async (_req: Request, res: Response): Promise<void> => {
|
return async (_req: Request, res: Response): Promise<void> => {
|
||||||
try {
|
try {
|
||||||
|
const { isRunning } = getSpecRegenerationStatus();
|
||||||
res.json({ success: true, isRunning });
|
res.json({ success: true, isRunning });
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
res.status(500).json({ success: false, error: getErrorMessage(error) });
|
res.status(500).json({ success: false, error: getErrorMessage(error) });
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
|
|
||||||
import type { Request, Response } from "express";
|
import type { Request, Response } from "express";
|
||||||
import {
|
import {
|
||||||
currentAbortController,
|
getSpecRegenerationStatus,
|
||||||
setRunningState,
|
setRunningState,
|
||||||
getErrorMessage,
|
getErrorMessage,
|
||||||
} from "../common.js";
|
} from "../common.js";
|
||||||
@@ -12,6 +12,7 @@ import {
|
|||||||
export function createStopHandler() {
|
export function createStopHandler() {
|
||||||
return async (_req: Request, res: Response): Promise<void> => {
|
return async (_req: Request, res: Response): Promise<void> => {
|
||||||
try {
|
try {
|
||||||
|
const { currentAbortController } = getSpecRegenerationStatus();
|
||||||
if (currentAbortController) {
|
if (currentAbortController) {
|
||||||
currentAbortController.abort();
|
currentAbortController.abort();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -12,8 +12,29 @@ import {
|
|||||||
|
|
||||||
const logger = createLogger("Setup");
|
const logger = createLogger("Setup");
|
||||||
|
|
||||||
// Storage for API keys (in-memory cache)
|
// Storage for API keys (in-memory cache) - private
|
||||||
export const apiKeys: Record<string, string> = {};
|
const apiKeys: Record<string, string> = {};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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<string, string> {
|
||||||
|
return { ...apiKeys };
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Helper to persist API keys to .env file
|
* Helper to persist API keys to .env file
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ import { promisify } from "util";
|
|||||||
import os from "os";
|
import os from "os";
|
||||||
import path from "path";
|
import path from "path";
|
||||||
import fs from "fs/promises";
|
import fs from "fs/promises";
|
||||||
import { apiKeys } from "./common.js";
|
import { getApiKey } from "./common.js";
|
||||||
|
|
||||||
const execAsync = promisify(exec);
|
const execAsync = promisify(exec);
|
||||||
|
|
||||||
@@ -71,8 +71,8 @@ export async function getClaudeStatus() {
|
|||||||
method: "none" as string,
|
method: "none" as string,
|
||||||
hasCredentialsFile: false,
|
hasCredentialsFile: false,
|
||||||
hasToken: false,
|
hasToken: false,
|
||||||
hasStoredOAuthToken: !!apiKeys.anthropic_oauth_token,
|
hasStoredOAuthToken: !!getApiKey("anthropic_oauth_token"),
|
||||||
hasStoredApiKey: !!apiKeys.anthropic,
|
hasStoredApiKey: !!getApiKey("anthropic"),
|
||||||
hasEnvApiKey: !!process.env.ANTHROPIC_API_KEY,
|
hasEnvApiKey: !!process.env.ANTHROPIC_API_KEY,
|
||||||
hasEnvOAuthToken: !!process.env.CLAUDE_CODE_OAUTH_TOKEN,
|
hasEnvOAuthToken: !!process.env.CLAUDE_CODE_OAUTH_TOKEN,
|
||||||
// Additional fields for detailed status
|
// Additional fields for detailed status
|
||||||
@@ -159,14 +159,14 @@ export async function getClaudeStatus() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// In-memory stored OAuth token (from setup wizard - subscription auth)
|
// 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.authenticated = true;
|
||||||
auth.oauthTokenValid = true;
|
auth.oauthTokenValid = true;
|
||||||
auth.method = "oauth_token"; // Stored OAuth token from setup wizard
|
auth.method = "oauth_token"; // Stored OAuth token from setup wizard
|
||||||
}
|
}
|
||||||
|
|
||||||
// In-memory stored API key (from settings UI - pay-per-use)
|
// 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.authenticated = true;
|
||||||
auth.apiKeyValid = true;
|
auth.apiKeyValid = true;
|
||||||
auth.method = "api_key"; // Manually stored API key
|
auth.method = "api_key"; // Manually stored API key
|
||||||
|
|||||||
@@ -3,15 +3,19 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import type { Request, Response } from "express";
|
import type { Request, Response } from "express";
|
||||||
import { apiKeys, getErrorMessage, logError } from "../common.js";
|
import {
|
||||||
|
getApiKey,
|
||||||
|
getErrorMessage,
|
||||||
|
logError,
|
||||||
|
} from "../common.js";
|
||||||
|
|
||||||
export function createApiKeysHandler() {
|
export function createApiKeysHandler() {
|
||||||
return async (_req: Request, res: Response): Promise<void> => {
|
return async (_req: Request, res: Response): Promise<void> => {
|
||||||
try {
|
try {
|
||||||
res.json({
|
res.json({
|
||||||
success: true,
|
success: true,
|
||||||
hasAnthropicKey: !!apiKeys.anthropic || !!process.env.ANTHROPIC_API_KEY,
|
hasAnthropicKey: !!getApiKey("anthropic") || !!process.env.ANTHROPIC_API_KEY,
|
||||||
hasGoogleKey: !!apiKeys.google || !!process.env.GOOGLE_API_KEY,
|
hasGoogleKey: !!getApiKey("google") || !!process.env.GOOGLE_API_KEY,
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
logError(error, "Get API keys failed");
|
logError(error, "Get API keys failed");
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
|
|
||||||
import type { Request, Response } from "express";
|
import type { Request, Response } from "express";
|
||||||
import {
|
import {
|
||||||
apiKeys,
|
setApiKey,
|
||||||
persistApiKeyToEnv,
|
persistApiKeyToEnv,
|
||||||
getErrorMessage,
|
getErrorMessage,
|
||||||
logError,
|
logError,
|
||||||
@@ -28,7 +28,7 @@ export function createStoreApiKeyHandler() {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
apiKeys[provider] = apiKey;
|
setApiKey(provider, apiKey);
|
||||||
|
|
||||||
// Also set as environment variable and persist to .env
|
// Also set as environment variable and persist to .env
|
||||||
// IMPORTANT: OAuth tokens and API keys must be stored separately
|
// IMPORTANT: OAuth tokens and API keys must be stored separately
|
||||||
|
|||||||
@@ -10,9 +10,19 @@ import {
|
|||||||
|
|
||||||
const logger = createLogger("Suggestions");
|
const logger = createLogger("Suggestions");
|
||||||
|
|
||||||
// Shared state for tracking generation status
|
// Shared state for tracking generation status - private
|
||||||
export let isRunning = false;
|
let isRunning = false;
|
||||||
export let currentAbortController: AbortController | null = null;
|
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
|
* Set the running state and abort controller
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import type { Request, Response } from "express";
|
|||||||
import type { EventEmitter } from "../../../lib/events.js";
|
import type { EventEmitter } from "../../../lib/events.js";
|
||||||
import { createLogger } from "../../../lib/logger.js";
|
import { createLogger } from "../../../lib/logger.js";
|
||||||
import {
|
import {
|
||||||
isRunning,
|
getSuggestionsStatus,
|
||||||
setRunningState,
|
setRunningState,
|
||||||
getErrorMessage,
|
getErrorMessage,
|
||||||
logError,
|
logError,
|
||||||
@@ -28,6 +28,7 @@ export function createGenerateHandler(events: EventEmitter) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const { isRunning } = getSuggestionsStatus();
|
||||||
if (isRunning) {
|
if (isRunning) {
|
||||||
res.json({
|
res.json({
|
||||||
success: false,
|
success: false,
|
||||||
|
|||||||
@@ -3,11 +3,16 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import type { Request, Response } from "express";
|
import type { Request, Response } from "express";
|
||||||
import { isRunning, getErrorMessage, logError } from "../common.js";
|
import {
|
||||||
|
getSuggestionsStatus,
|
||||||
|
getErrorMessage,
|
||||||
|
logError,
|
||||||
|
} from "../common.js";
|
||||||
|
|
||||||
export function createStatusHandler() {
|
export function createStatusHandler() {
|
||||||
return async (_req: Request, res: Response): Promise<void> => {
|
return async (_req: Request, res: Response): Promise<void> => {
|
||||||
try {
|
try {
|
||||||
|
const { isRunning } = getSuggestionsStatus();
|
||||||
res.json({ success: true, isRunning });
|
res.json({ success: true, isRunning });
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
logError(error, "Get status failed");
|
logError(error, "Get status failed");
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
|
|
||||||
import type { Request, Response } from "express";
|
import type { Request, Response } from "express";
|
||||||
import {
|
import {
|
||||||
currentAbortController,
|
getSuggestionsStatus,
|
||||||
setRunningState,
|
setRunningState,
|
||||||
getErrorMessage,
|
getErrorMessage,
|
||||||
logError,
|
logError,
|
||||||
@@ -13,6 +13,7 @@ import {
|
|||||||
export function createStopHandler() {
|
export function createStopHandler() {
|
||||||
return async (_req: Request, res: Response): Promise<void> => {
|
return async (_req: Request, res: Response): Promise<void> => {
|
||||||
try {
|
try {
|
||||||
|
const { currentAbortController } = getSuggestionsStatus();
|
||||||
if (currentAbortController) {
|
if (currentAbortController) {
|
||||||
currentAbortController.abort();
|
currentAbortController.abort();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -17,11 +17,37 @@ function getTerminalEnabledConfig(): boolean {
|
|||||||
return process.env.TERMINAL_ENABLED !== "false"; // Enabled by default
|
return process.env.TERMINAL_ENABLED !== "false"; // Enabled by default
|
||||||
}
|
}
|
||||||
|
|
||||||
// In-memory session tokens (would use Redis in production)
|
// In-memory session tokens (would use Redis in production) - private
|
||||||
export const validTokens: Map<string, { createdAt: Date; expiresAt: Date }> =
|
const validTokens: Map<string, { createdAt: Date; expiresAt: Date }> =
|
||||||
new Map();
|
new Map();
|
||||||
const TOKEN_EXPIRY_MS = 24 * 60 * 60 * 1000; // 24 hours
|
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
|
* Generate a secure random token
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ import {
|
|||||||
getTerminalEnabledConfigValue,
|
getTerminalEnabledConfigValue,
|
||||||
getTerminalPasswordConfig,
|
getTerminalPasswordConfig,
|
||||||
generateToken,
|
generateToken,
|
||||||
validTokens,
|
addToken,
|
||||||
getTokenExpiryMs,
|
getTokenExpiryMs,
|
||||||
getErrorMessage,
|
getErrorMessage,
|
||||||
} from "../common.js";
|
} from "../common.js";
|
||||||
@@ -49,7 +49,7 @@ export function createAuthHandler() {
|
|||||||
// Generate session token
|
// Generate session token
|
||||||
const token = generateToken();
|
const token = generateToken();
|
||||||
const now = new Date();
|
const now = new Date();
|
||||||
validTokens.set(token, {
|
addToken(token, {
|
||||||
createdAt: now,
|
createdAt: now,
|
||||||
expiresAt: new Date(now.getTime() + getTokenExpiryMs()),
|
expiresAt: new Date(now.getTime() + getTokenExpiryMs()),
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -3,14 +3,14 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import type { Request, Response } from "express";
|
import type { Request, Response } from "express";
|
||||||
import { validTokens } from "../common.js";
|
import { deleteToken } from "../common.js";
|
||||||
|
|
||||||
export function createLogoutHandler() {
|
export function createLogoutHandler() {
|
||||||
return (req: Request, res: Response): void => {
|
return (req: Request, res: Response): void => {
|
||||||
const token = (req.headers["x-terminal-token"] as string) || req.body.token;
|
const token = (req.headers["x-terminal-token"] as string) || req.body.token;
|
||||||
|
|
||||||
if (token) {
|
if (token) {
|
||||||
validTokens.delete(token);
|
deleteToken(token);
|
||||||
}
|
}
|
||||||
|
|
||||||
res.json({
|
res.json({
|
||||||
|
|||||||
152
docs/pr-comment-fix-agent.md
Normal file
152
docs/pr-comment-fix-agent.md
Normal file
@@ -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 <pr-number> --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 <headRefName>`
|
||||||
|
4. Pull latest changes: `git pull origin <headRefName>`
|
||||||
|
|
||||||
|
### 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 <headRefName>`
|
||||||
|
|
||||||
|
## 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
|
||||||
|
```
|
||||||
51
docs/pr-comment-fix-prompt.md
Normal file
51
docs/pr-comment-fix-prompt.md
Normal file
@@ -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 <pr-number> --comments --json number,title,body,comments,headRefName,baseRefName`
|
||||||
|
2. Parse all comments from the JSON output
|
||||||
|
3. Checkout the PR branch: `git checkout <headRefName>` and `git pull origin <headRefName>`
|
||||||
|
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 #<number> comments using `gh pr view <number> --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.
|
||||||
|
```
|
||||||
Reference in New Issue
Block a user