Skip to content

feat: add @botname sync command for PR comment-triggered merges - #58

Merged
adalton merged 3 commits into
flightctl:mainfrom
adalton:andalton/bot-merge-command
Jul 24, 2026
Merged

adalton merged 3 commits into
flightctl:mainfrom
adalton:andalton/bot-merge-command

Conversation

@adalton

@adalton adalton commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds @botname sync bot command on GitHub PR conversation comments that triggers merging main into the dev branch, with AI-driven conflict resolution if needed
  • Addresses cases where a PR is behind main with no conflicts (so the MergeScanner doesn't trigger) but CI failures fixed on main need to be picked up
  • Uses a single PR reply for feedback (post + edit-in-place), consistent with the existing <!-- addressed: ID --> pattern used by the feedback pipeline

Design

Detection: The MergeScanner (opt-in via WithMergeCommands()) checks conversation comments for @botname sync, filtering via BotRepliedTo to skip already-addressed commands. Conflicts take priority — if a PR is unmergeable, the existing conflict path runs without checking for commands.

Feedback: The executor posts an initial reply ("Merging from main -> branch") with the addressed marker, then edits it with the outcome (success + commit SHA, failure + reason, or already up-to-date). Single comment, no noise.

Multi-repo: commandSourceBaseBranch resolves the correct base branch from the repo the command was posted on, not Repos[0].

Key changes

  • commentfilter: export NormalizeUsername, add BotCommandRe/IsSyncCommand
  • jobmanager: add CommandSource on Event/Job, MergeCommitSHA on JobResult; deep-copy CommandSource in both Submit and snapshot
  • scanner/merge: WithMergeCommands option, findMergeCommand with BotRepliedTo detection
  • executor/merge: reply lifecycle (postMergeCommandReply/updateMergeCommandReply), preflight failure acknowledgment via postPreflightFailureReply
  • services/github: PostIssueComment returns (int64, error) for reply ID

Test plan

  • 15 IsSyncCommand test cases (case sensitivity, word boundaries, [bot] suffix, newlines)
  • 10 sync command scanner tests (detected, addressed, addressed with [bot] suffix, disabled, bot's own comment, skip label, no command, fetch error, review comments ignored, conflicts priority)
  • 4 executor reply lifecycle tests (success with SHA, failure, no CommandSource, preflight failure)
  • 2 coordinator CommandSource propagation tests
  • All existing tests pass (no regressions)
  • make lint passes (no new violations)

Assisted-by: Claude noreply@anthropic.com

Summary

Adds an opt-in @botname sync command that can be discovered in GitHub PR conversation comments to trigger the merge workflow. The merge scanner merges main into the PR’s development branch (using the existing AI-driven conflict resolution path when conflicts occur), but it submits only when it finds an unaddressed sync command (skipping commands already handled via <!-- addressed: <commentID> --> markers). For multi-repository setups it resolves the correct command base/target context, and it propagates the triggering GitHub comment identity through the job pipeline.

The executor now manages a “single reply lifecycle”: it posts one addressed reply when a command-triggered merge starts, then updates that same comment with the outcome—success (including merge commit SHA), failure reason, “already up to date,” or an addressed reply for preflight failures. To enable reliable follow-up edits, GitHub comment creation now returns the created comment ID, and that ID is threaded through executor/merge reply logic.

Affected packages

  • commentfilter/: Exports username normalization (NormalizeUsername), adds regex helpers (BotCommandRe and IsSyncCommand) for detecting @<bot> sync with matching/word-boundary and optional [bot] handling, and updates bot/ignored-bot comparisons to consistently use normalized usernames.
  • scanner/: Extends merge scanning with scanner.WithMergeCommands() (merge-command detection opt-in). When enabled, it scans PR conversation comments for unaddressed @botname sync, selects the triggering CommandSource, and includes it in merge events.
  • jobmanager/: Introduces a CommandSource model and propagates Event.CommandSource into Job.CommandSource (including snapshot cloning). Adds JobResult.MergeCommitSHA for merge jobs that produce commits.
  • executor/: Updates merge execution and feedback/conversation routing to use comment IDs from GitHub (PostIssueComment now returns (int64, error)), posts once then edits once via the triggering CommandSource, reports merge commit SHA on success, and ensures addressed preflight failure acknowledgements.
  • services/: Updates GitHubService.PostIssueComment to return the created GitHub comment ID ((int64, error)), enabling executor reply editing.
  • main.go: Enables the opt-in behavior by constructing the merge scanner with scanner.WithMergeCommands().

Pipeline impact

Extends the PR merge pipeline by adding command-driven triggering via @botname sync conversation comments, requiring unaddressed commands, and ensuring the triggering comment ID is carried end-to-end. Execution pipeline updates include a more robust reply lifecycle (post addressed reply once, then update with the merge outcome) and merge-result reporting that includes the resulting merge commit SHA.

Infrastructure and configuration

No changes to container management, workspace lifecycle, or crash recovery. Configuration/deployment impact is limited to enabling the new scanner behavior in main.go via scanner.WithMergeCommands().

When a user posts @botName merge on a GitHub PR conversation comment,
the bot merges main into the dev branch, resolving conflicts with AI
if needed. This addresses cases where a PR is behind main with no
conflicts (so the MergeScanner doesn't trigger) but CI failures fixed
on main need to be picked up.

The feature uses a single PR reply for feedback, consistent with the
existing <!-- addressed: ID --> pattern:
- Initial: "Merging from <base> -> <branch>"
- Updated on success: "Successfully merged ... ; change <sha>"
- Updated on failure: "Failed to merge ... ; <reason>"

Detection uses BotRepliedTo (same as feedback pipeline) to prevent
re-processing across bot restarts. The scanner filters to conversation
comments only and compiles the command regex once per scan cycle.

Key changes:
- commentfilter: export NormalizeUsername, add BotCommandRe/IsMergeCommand
- jobmanager: add CommandSource on Event/Job, MergeCommitSHA on JobResult
- scanner/merge: WithMergeCommands option, findMergeCommand with
  BotRepliedTo detection, refactored checkAndSubmit + submitMergeEvent
- executor/merge: postMergeCommandReply/updateMergeCommandReply with
  commandSourceBaseBranch for correct multi-repo base branch lookup
- services/github: PostIssueComment returns (int64, error) for reply ID

Assisted-by: Claude Opus 4.6 (1M) <noreply@anthropic.com>
@adalton adalton self-assigned this Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The merge scanner detects unaddressed bot sync commands, propagates their source comments, and enables merge execution to post and update addressed acknowledgments. Issue-comment APIs now return created IDs, and username normalization is exported and reused.

Changes

Merge command workflow

Layer / File(s) Summary
Command detection contracts
commentfilter/*.go
Username normalization is exported and applied consistently; public helpers detect supported bot sync command forms.
Scanner merge-command flow
scanner/merge.go, scanner/merge_test.go, main.go
Optional command detection filters eligible comments, avoids addressed or bot-authored commands, preserves conflict priority, and submits source metadata.
Command-source and comment-ID contracts
jobmanager/*.go, executor/executor.go, services/github.go, executor/executortest/stubs.go
Jobs carry command origins and merge SHAs, while issue-comment posting returns created IDs.
Merge reply lifecycle
executor/merge.go, executor/merge_test.go
Command-triggered merges post addressed replies and update them with merge outcomes.
Issue-comment call-site migration
executor/costcomment.go, executor/feedback.go, executor/*_test.go
Existing posting paths and test doubles consume the new (int64, error) signature.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MergeScanner
  participant Coordinator
  participant Pipeline
  participant GitHubServiceImpl
  MergeScanner->>Coordinator: submit Event with CommandSource
  Coordinator->>Pipeline: dispatch Job with CommandSource
  Pipeline->>GitHubServiceImpl: create acknowledgment comment
  GitHubServiceImpl-->>Pipeline: return comment ID
  Pipeline->>GitHubServiceImpl: update acknowledgment with merge result
Loading

Possibly related PRs

Suggested labels: executor, scanner, github-service

Suggested reviewers: amir-yogev-gh

🚥 Pre-merge checks | ✅ 12 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (12 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed It accurately summarizes the main change: adding an @botname sync command that triggers PR merges from comments.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets ✅ Passed No hardcoded secrets were introduced; added literals are benign command strings/HTML markers, with no API keys, passwords, embedded creds, or long base64 blobs.
No-Weak-Crypto ✅ Passed No weak algorithms, custom crypto, or secret/token comparisons were added; the new comparisons are only username matching.
No-Injection-Vectors ✅ Passed No unsafe sinks found: command parsing is regexp-based, Git invocations use exec.Command arg arrays, and comment data stays in GitHub API calls.
Container-Privileges ✅ Passed PASS: The PR only changes Go code (scanner/merge.go); no container/K8s manifests or flags like privileged, hostPID, or allowPrivilegeEscalation were introduced.
No-Sensitive-Data-In-Logs ✅ Passed No added logs expose secrets/PII; new entries only emit repo, ticket, job ID, PR number, comment IDs, and errors via zap.Error.
Resource-Leaks ✅ Passed No new unmanaged files, response bodies, DB/net connections, or goroutines were introduced; existing scanner/coordinator goroutines already have cancel+WaitGroup shutdown.
Unchecked-Errors ✅ Passed New error-returning calls are logged or returned; I found no PR-introduced blank-identifier error ignores or swallowed errors.
Ai-Attribution ✅ Passed PASS: PR/HEAD include an acceptable Assisted-by trailer for Claude, and no Co-Authored-By trailer was found.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@executor/merge.go`:
- Around line 42-63: Ensure failures from GetWorkItem and ResolveProject are
acknowledged before returning by installing the merge reply/defer lifecycle
before preflight, or by explicitly posting an addressed failure reply on both
early-return paths. Update the surrounding merge flow using
postMergeCommandReply, updateMergeCommandReply, and handleMergeFailure, and add
regression coverage verifying each preflight failure is acknowledged and not
resubmitted.

In `@jobmanager/coordinator.go`:
- Around line 162-170: Clone the caller-owned command metadata before assigning
it to the queued Job in the job construction path. Update the CommandSource
assignment to store an independent copy of event.CommandSource, preserving the
existing metadata values while preventing later caller mutations or concurrent
reads from affecting execution.

In `@scanner/merge.go`:
- Around line 302-380: Update findMergeCommand to pass the already-normalized
normBot value to commentfilter.BotRepliedTo instead of s.cfg.BotUsername,
preserving the existing normalized author checks. Add a regression test covering
a BotUsername with uppercase characters or a “[bot]” suffix and verify addressed
merge commands are not resubmitted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 73a375e8-a83e-4aa5-8a51-d12e08aaf200

📥 Commits

Reviewing files that changed from the base of the PR and between cbf1942 and 8550441.

📒 Files selected for processing (19)
  • commentfilter/ci_fix_attempt.go
  • commentfilter/commentfilter.go
  • commentfilter/commentfilter_test.go
  • executor/costcomment.go
  • executor/costcomment_test.go
  • executor/executor.go
  • executor/executortest/stubs.go
  • executor/feedback.go
  • executor/feedback_test.go
  • executor/merge.go
  • executor/merge_test.go
  • executor/pipeline_test.go
  • jobmanager/coordinator.go
  • jobmanager/coordinator_test.go
  • jobmanager/manager.go
  • main.go
  • scanner/merge.go
  • scanner/merge_test.go
  • services/github.go

Comment thread executor/merge.go Outdated
Comment thread jobmanager/coordinator.go Outdated
Comment thread scanner/merge.go
- Fix BotRepliedTo normalization bug: pass normBot (not raw
  s.cfg.BotUsername) to BotRepliedTo in findMergeCommand. Add
  regression test with [bot]-suffixed username.

- Clone CommandSource at Submit time: defensive copy in coordinator
  prevents caller mutations from affecting queued jobs.

- Acknowledge preflight failures: post addressed failure reply when
  GetWorkItem or ResolveProject fail before the reply lifecycle is
  set up. Prevents the scanner from re-detecting unaddressed commands
  every cycle.

- Rename command from "merge" to "sync": avoids ambiguity about merge
  direction. "sync" clearly means "bring this branch up to date with
  main", not "merge this PR into main".

- Resolve correct base branch for multi-repo: commandSourceBaseBranch
  looks up the base branch from the repo the command was posted on
  instead of defaulting to Repos[0].

Assisted-by: Claude Opus 4.6 (1M) <noreply@anthropic.com>
@adalton adalton changed the title feat: add @botname merge command for PR comment-triggered merges feat: add @botname sync command for PR comment-triggered merges Jul 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
scanner/merge.go (1)

342-347: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor — normalize the filtered comment slice.

var convComments []models.PRComment remains nil when there are no conversation comments. Initialize it as an empty slice to preserve the repository’s empty-result contract.

Proposed fix
-	var convComments []models.PRComment
+	convComments := make([]models.PRComment, 0, len(comments))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scanner/merge.go` around lines 342 - 347, Initialize convComments as a
non-nil empty []models.PRComment before filtering in the comments loop, while
preserving the existing append behavior for conversation comments.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@scanner/merge.go`:
- Around line 342-347: Initialize convComments as a non-nil empty
[]models.PRComment before filtering in the comments loop, while preserving the
existing append behavior for conversation comments.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: f15990ed-b4f1-408e-bb08-602adadc22c1

📥 Commits

Reviewing files that changed from the base of the PR and between 8550441 and cb6010e.

📒 Files selected for processing (7)
  • commentfilter/commentfilter.go
  • commentfilter/commentfilter_test.go
  • executor/merge.go
  • executor/merge_test.go
  • jobmanager/coordinator.go
  • scanner/merge.go
  • scanner/merge_test.go

Assisted-by: Claude Opus 4.6 (1M) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
scanner/merge.go (1)

364-367: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Avoid logging the raw command author username.

The author username is a user identifier and may constitute personal data. Remove it from routine logs or apply the project’s approved redaction/retention policy before emitting it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scanner/merge.go` around lines 364 - 367, Update the “Found sync command”
logger call in the merge flow to stop emitting c.Author.Username, or apply the
project’s approved redaction policy before logging it; preserve the repository
and pull-request fields.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@scanner/merge.go`:
- Around line 364-367: Update the “Found sync command” logger call in the merge
flow to stop emitting c.Author.Username, or apply the project’s approved
redaction policy before logging it; preserve the repository and pull-request
fields.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 1af4596c-f812-489d-9041-98bb0c07f111

📥 Commits

Reviewing files that changed from the base of the PR and between cb6010e and f24a3b1.

📒 Files selected for processing (1)
  • scanner/merge.go

@adalton

adalton commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai GitHub usernames are public identifiers — they are visible in every PR, commit, issue, and comment on the platform. Logging the author of a sync command is consistent with how this codebase already logs usernames throughout the feedback scanner, idle detection, and comment filtering paths (e.g., lastHumanCommentTime, Filter, isIdlePR). Removing it here would make debugging harder (no way to trace who issued the command) for no real security benefit.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@adalton Agreed. Severity: low; impact: no actionable security or correctness issue. Logging the author of an explicit @botname sync command is consistent with the existing observability model and materially helps trace operational actions during debugging. I’m withdrawing the concern; no change is needed.


✏️ Learnings added
Learnt from: adalton
URL: https://github.com/flightctl/jira-ai-issue-solver/pull/58

Timestamp: 2026-07-24T17:05:27.658Z
Learning: In this Go codebase, GitHub usernames are intentionally logged throughout feedback scanning, idle detection, and comment filtering (including `lastHumanCommentTime`, `Filter`, and `isIdlePR`). For explicit bot commands such as `botname sync`, logging the command author is an accepted observability practice for debugging and auditability.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@adalton
adalton merged commit f080377 into flightctl:main Jul 24, 2026
3 checks passed
@adalton
adalton deleted the andalton/bot-merge-command branch July 24, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant