Skip to content

fix(amber): Push to existing PR instead of creating duplicates #453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeremyeder merged 2 commits into from
Dec 9, 2025
Merged

fix(amber): Push to existing PR instead of creating duplicates #453

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d2694f
fix(amber): Push to existing PR instead of creating duplicates
natifridman Dec 9, 2025
83dda20
fix(amber): Address code review security and reliability issues
natifridman Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
283 changes: 234 additions & 49 deletions .github/workflows/amber-issue-handler.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,22 @@
# Amber Issue-to-PR Handler
#
# This workflow automates issue resolution via the Amber background agent.
#
# TRIGGERS:
# - Issue labeled with: amber:auto-fix, amber:refactor, amber:test-coverage
# - Issue comment containing: /amber execute or @amber
#
# BEHAVIOR:
# - Checks for existing open PR for the issue (prevents duplicate PRs)
# - Creates or updates feature branch: amber/issue-{number}-{sanitized-title}
# - Runs Claude Code to implement changes
# - Creates PR or pushes to existing PR
#
# SECURITY:
# - Validates branch names against injection attacks
# - Uses strict regex matching for PR lookup
# - Handles race conditions when PRs are closed during execution

name: Amber Issue-to-PR Handler

on:
Expand Down Expand Up @@ -180,29 +199,121 @@ jobs:

echo "prompt_file=amber-prompt.md" >> $GITHUB_OUTPUT

- name: Create feature branch
id: create-branch
- name: Check for existing PR
id: check-existing-pr
env:
ISSUE_NUMBER: ${{ steps.issue-details.outputs.issue_number }}
ISSUE_TITLE: ${{ steps.issue-details.outputs.issue_title }}
GH_TOKEN: ${{ github.token }}
run: |
# Improved sanitization (Issue #10) - handles special chars, spaces, consecutive dashes
SANITIZED_TITLE=$(echo "$ISSUE_TITLE" \
| tr '[:upper:]' '[:lower:]' \
| sed 's/[^a-z0-9-]/-/g' \
| sed 's/--*/-/g' \
| sed 's/^-//' \
| sed 's/-$//' \
| cut -c1-50)
# Validate issue number is numeric to prevent injection
if ! [[ "$ISSUE_NUMBER" =~ ^[0-9]+$ ]]; then
echo "Error: Invalid issue number format"
exit 1
fi

BRANCH_NAME="amber/issue-${ISSUE_NUMBER}-${SANITIZED_TITLE}"
# Check if there's already an open PR for this issue using stricter matching
# Search for PRs that reference this issue and filter by body containing exact "Closes #N" pattern
EXISTING_PR=$(gh pr list --state open --json number,headRefName,body --jq \
".[] | select(.body | test(\"Closes #${ISSUE_NUMBER}($|[^0-9])\")) | {number, headRefName}" \
Copy link
Collaborator

@jeremyeder jeremyeder Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be enough to break this bot loop

2>/dev/null | head -1 || echo "")

if [ -n "$EXISTING_PR" ] && [ "$EXISTING_PR" != "null" ] && [ "$EXISTING_PR" != "{}" ]; then
PR_NUMBER=$(echo "$EXISTING_PR" | jq -r '.number')
EXISTING_BRANCH=$(echo "$EXISTING_PR" | jq -r '.headRefName')

# Validate branch name format to prevent command injection
if ! [[ "$EXISTING_BRANCH" =~ ^[a-zA-Z0-9/_.-]+$ ]]; then
echo "Error: Invalid branch name format in existing PR"
echo "existing_pr=false" >> $GITHUB_OUTPUT
exit 0
fi

echo "existing_pr=true" >> $GITHUB_OUTPUT
echo "pr_number=$PR_NUMBER" >> $GITHUB_OUTPUT
echo "existing_branch=$EXISTING_BRANCH" >> $GITHUB_OUTPUT
echo "Found existing PR #$PR_NUMBER on branch $EXISTING_BRANCH"
else
echo "existing_pr=false" >> $GITHUB_OUTPUT
echo "No existing PR found for issue #${ISSUE_NUMBER}"
fi

- name: Create or checkout feature branch
id: create-branch
env:
ISSUE_NUMBER: ${{ steps.issue-details.outputs.issue_number }}
ISSUE_TITLE: ${{ steps.issue-details.outputs.issue_title }}
EXISTING_PR: ${{ steps.check-existing-pr.outputs.existing_pr }}
EXISTING_BRANCH: ${{ steps.check-existing-pr.outputs.existing_branch }}
run: |
git config user.name "Amber Agent"
git config user.email "[email protected]"
git checkout -b "$BRANCH_NAME"

# Validate issue number format
if ! [[ "$ISSUE_NUMBER" =~ ^[0-9]+$ ]]; then
echo "Error: Invalid issue number format"
exit 1
fi

checkout_branch() {
local branch="$1"
local is_existing="$2"

# Validate branch name format (alphanumeric, slashes, dashes, dots, underscores only)
if ! [[ "$branch" =~ ^[a-zA-Z0-9/_.-]+$ ]]; then
echo "Error: Invalid branch name format: $branch"
return 1
fi

echo "Attempting to checkout branch: $branch"
if git fetch origin "$branch" 2>/dev/null; then
git checkout -B "$branch" "origin/$branch"
echo "Checked out existing remote branch: $branch"
elif [ "$is_existing" == "true" ]; then
# Race condition: PR existed but branch was deleted
echo "Warning: Branch $branch no longer exists on remote (PR may have been closed)"
return 1
else
echo "Creating new branch: $branch"
git checkout -b "$branch"
fi
return 0
}

if [ "$EXISTING_PR" == "true" ] && [ -n "$EXISTING_BRANCH" ]; then
# Try to checkout existing PR branch with race condition handling
if ! checkout_branch "$EXISTING_BRANCH" "true"; then
echo "Existing PR branch unavailable, falling back to new branch creation"
# Fall through to create new branch
EXISTING_PR="false"
else
BRANCH_NAME="$EXISTING_BRANCH"
fi
fi

if [ "$EXISTING_PR" != "true" ]; then
# Create new branch with sanitized title
# Sanitization: lowercase, replace non-alphanumeric with dash, collapse dashes, trim
SANITIZED_TITLE=$(echo "$ISSUE_TITLE" \
| tr '[:upper:]' '[:lower:]' \
| sed 's/[^a-z0-9-]/-/g' \
| sed 's/--*/-/g' \
| sed 's/^-//' \
| sed 's/-$//' \
| cut -c1-50)

BRANCH_NAME="amber/issue-${ISSUE_NUMBER}-${SANITIZED_TITLE}"

# Validate the generated branch name
if ! [[ "$BRANCH_NAME" =~ ^[a-zA-Z0-9/_.-]+$ ]]; then
echo "Error: Generated branch name is invalid: $BRANCH_NAME"
exit 1
fi

checkout_branch "$BRANCH_NAME" "false" || exit 1
fi

echo "branch_name=$BRANCH_NAME" >> $GITHUB_OUTPUT
echo "Created branch: $BRANCH_NAME"
echo "Using branch: $BRANCH_NAME"

- name: Read prompt file
id: read-prompt
Expand Down Expand Up @@ -297,6 +408,8 @@ jobs:
RUN_ID: ${{ github.run_id }}
GITHUB_SERVER_URL: ${{ github.server_url }}
GITHUB_REPOSITORY: ${{ github.repository }}
EXISTING_PR: ${{ steps.check-existing-pr.outputs.existing_pr }}
EXISTING_PR_NUMBER: ${{ steps.check-existing-pr.outputs.pr_number }}
uses: actions/github-script@v8
with:
script: |
Expand All @@ -308,18 +421,24 @@ jobs:
const runId = process.env.RUN_ID;
const serverUrl = process.env.GITHUB_SERVER_URL;
const repository = process.env.GITHUB_REPOSITORY;
const existingPr = process.env.EXISTING_PR === 'true';
const existingPrNumber = process.env.EXISTING_PR_NUMBER;

// Safely get git diff (no shell injection risk with execFile)
const { stdout: diff } = await execFileAsync('git', ['diff', 'HEAD~1', '--stat']);

const nextSteps = existingPr
? `- Review that changes match the issue description\n- Verify no scope creep or unintended modifications\n- Changes pushed to existing PR #${existingPrNumber}`
: `- Review that changes match the issue description\n- Verify no scope creep or unintended modifications\n- A PR will be created shortly for formal review`;

await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
body: `## Amber Change Summary\n\nThe following files were modified:\n\n\`\`\`\n${diff}\n\`\`\`\n\n**Next Steps:**\n- Review that changes match the issue description\n- Verify no scope creep or unintended modifications\n- A PR will be created shortly for formal review\n\n---\n🔍 [View AI decision process](${serverUrl}/${repository}/actions/runs/${runId}) (logs available for 90 days)`
body: `## Amber Change Summary\n\nThe following files were modified:\n\n\`\`\`\n${diff}\n\`\`\`\n\n**Next Steps:**\n${nextSteps}\n\n---\n🔍 [View AI decision process](${serverUrl}/${repository}/actions/runs/${runId}) (logs available for 90 days)`
});

- name: Create Pull Request
- name: Create or Update Pull Request
if: steps.check-changes.outputs.has_changes == 'true'
env:
BRANCH_NAME: ${{ steps.check-changes.outputs.branch_name }}
Expand All @@ -330,6 +449,8 @@ jobs:
GITHUB_REPOSITORY: ${{ github.repository }}
RUN_ID: ${{ github.run_id }}
GITHUB_SERVER_URL: ${{ github.server_url }}
EXISTING_PR: ${{ steps.check-existing-pr.outputs.existing_pr }}
EXISTING_PR_NUMBER: ${{ steps.check-existing-pr.outputs.pr_number }}
uses: actions/github-script@v8
with:
script: |
Expand All @@ -341,6 +462,8 @@ jobs:
const repository = process.env.GITHUB_REPOSITORY;
const runId = process.env.RUN_ID;
const serverUrl = process.env.GITHUB_SERVER_URL;
const existingPr = process.env.EXISTING_PR === 'true';
const existingPrNumber = process.env.EXISTING_PR_NUMBER ? parseInt(process.env.EXISTING_PR_NUMBER) : null;

// Helper function for retrying API calls with exponential backoff
// Retries on: 5xx errors, network errors (no status), JSON parse errors
Expand Down Expand Up @@ -368,8 +491,57 @@ jobs:
throw new Error('retryWithBackoff: max retries exceeded');
}

// Create PR with error handling (Issue #3)
// Helper function to safely add a comment with fallback logging
async function safeComment(issueNum, body, description) {
try {
await retryWithBackoff(async () => {
return await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNum,
body: body
});
});
console.log(`Successfully added comment: ${description}`);
} catch (commentError) {
// Log but don't fail the workflow for comment failures
console.log(`Warning: Failed to add comment (${description}): ${commentError.message}`);
console.log(`Comment body was: ${body.substring(0, 200)}...`);
}
}

try {
// If PR already exists, just add a comment about the new push
if (existingPr && existingPrNumber) {
console.log(`PR #${existingPrNumber} already exists, adding update comment`);

// Add comment to PR about the new commit (with fallback)
await safeComment(
existingPrNumber,
`🤖 **Amber pushed additional changes**

- **Commit:** ${commitSha.substring(0, 7)}
- **Action Type:** ${actionType}

New changes have been pushed to this PR. Please review the updated code.

---
🔍 [View AI decision process](${serverUrl}/${repository}/actions/runs/${runId}) (logs available for 90 days)`,
`PR #${existingPrNumber} update notification`
);

// Also notify on the issue (with fallback)
await safeComment(
issueNumber,
`🤖 Amber pushed additional changes to the existing PR #${existingPrNumber}.\n\n---\n🔍 [View AI decision process](${serverUrl}/${repository}/actions/runs/${runId}) (logs available for 90 days)`,
`Issue #${issueNumber} update notification`
);

console.log(`Updated existing PR #${existingPrNumber}`);
return;
}

// Create new PR
const pr = await github.rest.pulls.create({
owner: context.repo.owner,
repo: context.repo.repo,
Expand Down Expand Up @@ -403,36 +575,38 @@ jobs:
Closes #${issueNumber}`
});

// Add labels with retry logic for transient API failures
await retryWithBackoff(async () => {
return await github.rest.issues.addLabels({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pr.data.number,
labels: ['amber-generated', 'auto-fix', actionType]
// Add labels with retry logic for transient API failures (non-critical)
try {
await retryWithBackoff(async () => {
return await github.rest.issues.addLabels({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pr.data.number,
labels: ['amber-generated', 'auto-fix', actionType]
});
});
});
} catch (labelError) {
console.log(`Warning: Failed to add labels to PR #${pr.data.number}: ${labelError.message}`);
}

// Link PR back to issue
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
body: `🤖 Amber has created a pull request to address this issue: #${pr.data.number}\n\nThe changes are ready for review. All automated checks will run on the PR.\n\n---\n🔍 [View AI decision process](${serverUrl}/${repository}/actions/runs/${runId}) (logs available for 90 days)`
});
// Link PR back to issue (with fallback)
await safeComment(
issueNumber,
`🤖 Amber has created a pull request to address this issue: #${pr.data.number}\n\nThe changes are ready for review. All automated checks will run on the PR.\n\n---\n🔍 [View AI decision process](${serverUrl}/${repository}/actions/runs/${runId}) (logs available for 90 days)`,
`Issue #${issueNumber} PR link notification`
);

console.log('Created PR:', pr.data.html_url);
} catch (error) {
console.error('Failed to create PR:', error);
core.setFailed(`PR creation failed: ${error.message}`);

// Notify on issue about failure
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
body: `⚠️ Amber completed changes but failed to create a pull request.\n\n**Error:** ${error.message}\n\nChanges committed to \`${branchName}\`. A maintainer can manually create the PR.`
});
console.error('Failed to create/update PR:', error);
core.setFailed(`PR creation/update failed: ${error.message}`);

// Notify on issue about failure (with fallback - best effort)
await safeComment(
issueNumber,
`⚠️ Amber completed changes but failed to create a pull request.\n\n**Error:** ${error.message}\n\nChanges committed to \`${branchName}\`. A maintainer can manually create the PR.`,
`Issue #${issueNumber} PR failure notification`
);
}

- name: Report failure
Expand All @@ -447,16 +621,23 @@ jobs:
with:
script: |
const issueNumber = parseInt(process.env.ISSUE_NUMBER);
const actionType = process.env.ACTION_TYPE;
const actionType = process.env.ACTION_TYPE || 'unknown';
const runId = process.env.RUN_ID;
const serverUrl = process.env.GITHUB_SERVER_URL;
const repository = process.env.GITHUB_REPOSITORY;

await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
body: `⚠️ Amber encountered an error while processing this issue.
// Validate issue number before attempting comment
if (!issueNumber || isNaN(issueNumber)) {
console.log('Error: Invalid issue number, cannot post failure comment');
return;
}

try {
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issueNumber,
body: `⚠️ Amber encountered an error while processing this issue.

**Action Type:** ${actionType}
**Workflow Run:** ${serverUrl}/${repository}/actions/runs/${runId}
Expand All @@ -467,4 +648,8 @@ jobs:
3. Ensure the changes are feasible for automation

Manual intervention may be required for complex changes.`
});
});
console.log(`Posted failure comment to issue #${issueNumber}`);
} catch (commentError) {
console.log(`Warning: Failed to post failure comment to issue #${issueNumber}: ${commentError.message}`);
}
Loading