Skip to content

fix: use catch-all handler for merge_test_to_main action#280

Merged
sweetmantech merged 3 commits intotestfrom
fix/merge-test-to-main-action-handler
Mar 10, 2026
Merged

fix: use catch-all handler for merge_test_to_main action#280
sweetmantech merged 3 commits intotestfrom
fix/merge-test-to-main-action-handler

Conversation

@sweetmantech
Copy link
Contributor

@sweetmantech sweetmantech commented Mar 10, 2026

Summary

  • Chat SDK's onAction with a string argument uses exact match, not prefix match
  • merge_test_to_main:recoupable/api was not matching merge_test_to_main:
  • Changed to catch-all handler pattern (same as onMergeAction) that uses startsWith() internally

Test plan

  • Unit tests pass
  • Update Slack interactivity URL to preview deployment
  • Click "Merge test → main" button in Slack and verify it works

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Improved internal code structure for the merge test to main action handler to enhance maintainability.

Chat SDK's onAction with a string argument uses exact match, not prefix
match. Changed to a catch-all handler that filters with
startsWith("merge_test_to_main:") so dynamic action IDs like
"merge_test_to_main:recoupable/api" are matched correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Contributor

vercel bot commented Mar 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
recoup-api Ready Ready Preview Mar 10, 2026 4:01pm

Request Review

@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 96f4390a-3b15-41db-b936-77a59a52b96e

📥 Commits

Reviewing files that changed from the base of the PR and between ff15919 and 6cba9b2.

⛔ Files ignored due to path filters (1)
  • lib/coding-agent/__tests__/onMergeTestToMainAction.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (1)
  • lib/coding-agent/handlers/onMergeTestToMainAction.ts

📝 Walkthrough

Walkthrough

The PR refactors the action handler registration in the merge-test-to-main handler from an explicit action name filter to a generic action handler with an inline runtime guard that filters based on the action ID prefix. The core merge logic remains unchanged.

Changes

Cohort / File(s) Summary
Handler Registration Pattern
lib/coding-agent/handlers/onMergeTestToMainAction.ts
Changed from explicit action name filtering to a generic onAction handler with a runtime guard checking event.actionId prefix. Processing logic for parsing, repository extraction, token validation, and merge operations remains unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

A handler finds new clothes to wear,
From named to generic, stripped to bare,
The guard stands watch with keen intent,
Same logic flows, but elegantly bent. 🎭✨

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning Handlers onMergeAction and onMergeTestToMainAction are nearly identical (23 lines each), violating DRY principle. Common logic should be abstracted into a reusable factory function parameterized by action prefix. Extract common merge action handler logic into a factory function createMergeActionHandler(prefix: string) and move hardcoded action prefixes into named constants.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/merge-test-to-main-action-handler

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

sweetmantech and others added 2 commits March 10, 2026 10:58
Slack requires interaction responses within 3 seconds. The
mergeGithubBranch call was blocking the response. Changed to
fire-and-forget pattern so the handler returns immediately.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sweetmantech sweetmantech merged commit 2b44147 into test Mar 10, 2026
3 checks passed
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