Skip to content

Fix bugs, clean up code, and restore fire-and-forget reactions#837

Merged
skjnldsv merged 3 commits intomasterfrom
copilot/cleanup-code-and-find-issues
Mar 18, 2026
Merged

Fix bugs, clean up code, and restore fire-and-forget reactions#837
skjnldsv merged 3 commits intomasterfrom
copilot/cleanup-code-and-find-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 18, 2026

Several real bugs found and fixed across the codebase, plus a design clarification: reactions should be non-blocking background operations, not part of the critical path.

Bugs fixed

  • pushBranch race conditiongit.remote(['set-url', ...]) was not awaited; the authenticated URL could be set after the push already ran
  • Commit message amendment lost subject line (cherryPickCommits) — only body was used when reconstructing the amended message, dropping the subject line entirely; commits with no body silently skipped adding [skip ci]
  • Wrong todo step for diff-only warning (getBackportBody) — pushed STEP_REVIEW_CONFLICTS even when there were no conflicts, only a diff; added STEP_REVIEW_CHANGES constant for this case
  • error(task, e) type mismatch in queue.ts — passed an Error object to a string-typed parameter
  • Stray console.trace() in production failure handler
  • Log message typo${commentAuthor}} had an extra }
  • Extra leading space in warning body text ("\n\n Warning,""\n\nWarning,")
  • Variable shadowing — local hasEmptyCommits variable inside the hasEmptyCommits function

Reactions: fire-and-forget via .catch()

Reactions were originally unawaited (intentionally non-blocking), but that made surrounding try/catch blocks dead code. Adding await fixed the error handling but made reactions block the backport process — wrong trade-off.

The correct pattern: fire-and-forget with .catch() for error logging.

// Before (blocking):
try {
    await addReaction(octokit, task, Reaction.THUMBS_UP)
} catch (e) {
    error(task, `Failed to add reaction: ${e.message}`)
}

// After (non-blocking, errors still logged):
addReaction(octokit, task, Reaction.THUMBS_UP)
    .catch(err => error(task, `Failed to add reaction: ${err.message}`))

All addReaction call sites in index.ts and backport.ts updated. Standalone try/catch blocks that existed solely for a single reaction, and a now-dead try/catch around the synchronous isFriendly() call, are removed. In the failure handler, the THUMBS_DOWN reaction is fire-and-forget while commentOnPR remains awaited.


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Co-authored-by: skjnldsv <14975046+skjnldsv@users.noreply.github.com>
Copilot AI changed the title [WIP] Clean up code and find potential bugs Fix bugs and clean up code Mar 18, 2026
Copilot AI requested a review from skjnldsv March 18, 2026 09:09
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2026

Codecov Report

❌ Patch coverage is 61.53846% with 25 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/gitUtils.ts 0.00% 10 Missing ⚠️
src/index.ts 0.00% 8 Missing ⚠️
src/backport.ts 0.00% 4 Missing ⚠️
src/nextcloudUtils.spec.ts 94.87% 0 Missing and 2 partials ⚠️
src/queue.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@skjnldsv skjnldsv marked this pull request as ready for review March 18, 2026 09:15
…catch

Co-authored-by: skjnldsv <14975046+skjnldsv@users.noreply.github.com>
Copilot AI changed the title Fix bugs and clean up code Fix bugs, clean up code, and restore fire-and-forget reactions Mar 18, 2026
@skjnldsv skjnldsv enabled auto-merge March 18, 2026 09:27
@skjnldsv skjnldsv disabled auto-merge March 18, 2026 09:27
@skjnldsv skjnldsv merged commit 23d8fdb into master Mar 18, 2026
8 checks passed
@skjnldsv skjnldsv deleted the copilot/cleanup-code-and-find-issues branch March 18, 2026 09:27
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.

2 participants