codeflash-ai/codeflash-internal
1
0
Fork 0
mirror of https://github.com/codeflash-ai/codeflash-internal.git synced 2026-05-04 18:25:18 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix issue with closed and merged PRs raising suggestion (#2436)

Browse source
This commit is contained in:
Sarthak Agarwal 2026-02-21 01:23:55 +05:30 committed by GitHub
parent eb5f4b460e
commit 2cb3d51ddb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 113 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
js/cf-api/endpoints/suggest-pr-changes.ts
View file

@ -510,6 +510,40 @@ export async function triggerSuggestPrChanges(
repo,
pull_number: pullNumber,
})
// Check if the PR is merged or closed - we can't suggest changes on merged/closed PRs
if (originalPrData.data.merged) {
logger.info(
`PR #${pullNumber} is already merged, cannot suggest changes`,
{
endpoint: "/cfapi/suggest-pr-changes",
operation: "pr_merged_check",
owner,
repo,
userId,
},
)
throw unprocessableEntity(
`Cannot suggest changes on merged PR #${pullNumber}. The PR was already merged.`,
)
}
if (originalPrData.data.state === "closed") {
logger.info(
`PR #${pullNumber} is closed, cannot suggest changes`,
{
endpoint: "/cfapi/suggest-pr-changes",
operation: "pr_closed_check",
owner,
repo,
userId,
},
)
throw unprocessableEntity(
`Cannot suggest changes on closed PR #${pullNumber}. The PR is no longer open.`,
)
}
const baseBranch = originalPrData.data.head.ref
logger.info(`Attempting to access ref for: ${owner}/${repo}, branch: ${baseBranch}`, {
endpoint: "/cfapi/suggest-pr-changes",

41
js/cf-api/endpoints/verify-existing-optimizations.ts
View file

@ -133,6 +133,7 @@ export async function verifyExistingOptimizations(req: Request, res: Response) {
}
// Get PR with specific 404 handling
// Note: GitHub returns 404 for both non-existent PRs and PRs the installation cannot access
let pr
try {
pr = await octokit.rest.pulls.get({
@ -142,7 +143,45 @@ export async function verifyExistingOptimizations(req: Request, res: Response) {
})
} catch (error: any) {
if (error.status === 404) {
throw githubPrNotFound(`#${pr_number} in ${repo_owner}/${repo_name}`)
// Log additional context to help diagnose permission vs not-found issues
logger.warn(
`PR #${pr_number} returned 404 in ${repo_owner}/${repo_name}. This could mean the PR doesn't exist, or the GitHub App installation doesn't have access to it.`,
{
requestId: (req as any).requestId,
userId,
endpoint: "/cfapi/verify-existing-optimizations",
operation: "pr_not_found_or_no_access",
repo_owner,
repo_name,
pr_number,
nickname,
errorMessage: error.message,
errorResponse: error.response?.data,
},
)
throw githubPrNotFound(
`#${pr_number} in ${repo_owner}/${repo_name}. If the PR exists, ensure the GitHub App installation has access to this repository.`,
)
}
// Handle 403 (Forbidden) as a permissions issue
if (error.status === 403) {
logger.warn(
`Access forbidden to PR #${pr_number} in ${repo_owner}/${repo_name}. The GitHub App installation may not have sufficient permissions.`,
{
requestId: (req as any).requestId,
userId,
endpoint: "/cfapi/verify-existing-optimizations",
operation: "pr_access_forbidden",
repo_owner,
repo_name,
pr_number,
nickname,
errorMessage: error.message,
},
)
throw githubInstallationError(
`Access forbidden to PR #${pr_number} in ${repo_owner}/${repo_name}. Please ensure the GitHub App has the necessary permissions.`,
)
}
throw error // Re-throw to be caught by global handler
}

54
js/cf-api/github/optimization_approval.ts
View file

@ -574,28 +574,52 @@ export async function processReaction(event: any): Promise<boolean> {
console.error(
`Error processing approved request for trace ${optimization.trace_id}: ${err}`,
)
await sendSlackMessage(
// Extract helpful error details for Slack notification
const errorMessage = err.message || String(err)
const errorType = err.constructor?.name || "Error"
const isPrMergedOrClosed = errorMessage.includes("merged") || errorMessage.includes("closed")
const errorBlocks: any[] = [
{
blocks: [
type: "section",
text: {
type: "mrkdwn",
text: `:warning: Error processing approved optimization \`${optimization.trace_id}\`:`,
},
},
{
type: "section",
text: {
type: "mrkdwn",
text: `\`\`\`${errorMessage}\`\`\``,
},
},
]
// Add helpful context if PR is merged/closed
if (isPrMergedOrClosed) {
errorBlocks.push({
type: "context",
elements: [
{
type: "section",
text: {
type: "mrkdwn",
text: `:warning: Error processing approved optimization \`${optimization.trace_id}\`:`,
},
},
{
type: "section",
text: {
type: "mrkdwn",
text: `\`\`\`${err.message}\`\`\``,
},
type: "mrkdwn",
text: `ℹ️ The target PR may have been merged or closed since the optimization was queued for approval.`,
},
],
text: `Error processing optimization ${optimization.trace_id}: ${err.message}`,
})
}
await sendSlackMessage(
{
blocks: errorBlocks,
text: `Error processing optimization ${optimization.trace_id}: ${errorMessage}`,
},
channel,
)
// Return false to indicate the reaction processing failed
return false
}
}