Skip to content

feat(router): move emoji reactions from worker to router for instant feedback#421

Merged
zbigniewsobiecki merged 2 commits intodevfrom
feature/router-immediate-reactions
Feb 18, 2026
Merged

feat(router): move emoji reactions from worker to router for instant feedback#421
zbigniewsobiecki merged 2 commits intodevfrom
feature/router-immediate-reactions

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Feb 18, 2026

Summary

  • New src/router/reactions.ts module — router-side reaction dispatch using raw fetch() following the notifications.ts pattern, supporting Trello (💭), GitHub (👀), and JIRA (💭) reactions
  • Wired into all 3 webhook handlers — fire-and-forget sendAcknowledgeReaction() calls before addJob() in Trello, GitHub, and JIRA POST routes
  • Comprehensive unit tests — 20 tests covering all platforms, payload detection, error handling, credential failures, and JIRA cloudId caching

Test plan

  • Unit tests for sendAcknowledgeReaction() — all platforms, payload type guards, error handling, credential failures
  • npm test passes (46/46 router tests)
  • npm run lint:fix passes (no errors)
  • npm run typecheck passes (zero errors)
  • Manual: Post Trello comment → verify 💭 appears in ~1s (vs ~10-30s before)
  • Manual: Post GitHub PR comment → verify 👀 appears immediately

Key decisions

  • GitHub handler: Passes repoFullName as the identifier to sendAcknowledgeReaction (instead of projectId) since GitHub jobs don't carry a projectId. The reaction module calls findProjectByRepo() internally to resolve credentials.
  • JIRA cloudId: Lightweight in-memory cache (separate from jiraClient's module-level cache) prevents repeated _edge/tenant_info calls on the router hot path. Failure returns null → fallback comment.
  • Worker-side acknowledgeWithReaction() kept: Both router and worker may react. All platforms handle duplicate reactions idempotently. Worker-side can be removed later once router reactions are proven stable.
  • No new npm deps: Router uses only fetch() + existing credential helpers.

Related

Card: https://trello.com/c/6996254bcee54ec08f63ded2

🤖 Generated with Claude Code

@zbigniewsobiecki
Copy link
Copy Markdown
Member

@aaight resolve conflicts

@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Feb 18, 2026

Request Addressed

What was asked

Resolve merge conflicts with the dev branch.

Changes Made

  • Merged origin/dev into feature/router-immediate-reactions
  • Resolved conflict in src/router/index.ts (import section) — kept both the pre-actions.ts import + GitHubJob type from dev and the reactions.ts import from this branch
  • The firePreActions() call for check_suite events (from dev) and sendAcknowledgeReaction() calls (from this branch) both preserved in the GitHub webhook handler

Verification

  • TypeScript typecheck passes (zero errors)
  • Lint passes (zero new errors)
  • All 57 router tests passing (reactions: 20, pre-actions: 11, notifications: 26)

Merge origin/dev into feature/router-immediate-reactions, resolving
the import conflict in src/router/index.ts to include both the new
pre-actions module from dev and the reactions module from this branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Feb 18, 2026

Conflicts resolved ✅ Merged origin/dev into the branch — the only conflict was in src/router/index.ts (import section), where both the new pre-actions.ts import from dev and the reactions.ts import from this branch needed to coexist. All 57 router tests pass, typecheck and lint are clean.

@nhopeatall
Copy link
Copy Markdown
Collaborator

🔍 Reviewing PR...

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

LGTM — clean implementation that follows established router patterns (notifications.ts, pre-actions.ts) for fire-and-forget platform API calls.

Architecture: The new reactions.ts module correctly mirrors the credential resolution, error handling, and dispatch patterns used by notifications.ts. The decision to use raw fetch() (avoiding client library dependencies in the router image) is consistent with the existing approach.

Correctness verified:

  • void sendAcknowledgeReaction(...).catch(...) is idiomatic fire-and-forget — doesn't block the 200 response to the webhook provider
  • GitHub reaction handler correctly early-returns for non-comment events (check_suite, pull_request, etc.), avoiding conflict with the existing addEyesReactionToPR pre-action
  • JIRA fallback path (reactions API → comment) handles all failure modes gracefully
  • JIRA cloudId cache keyed by baseUrl is an improvement over the worker's single-value cache for multi-project setups
  • Different emojis (💭 router vs 👀 worker) for Trello makes the dual-reaction transitional period distinguishable

Tests: 20 tests comprehensively cover all platforms, payload type guards, credential failures, API errors, cache behavior, and fallback paths.

No issues found.

@zbigniewsobiecki zbigniewsobiecki merged commit 86e803c into dev Feb 18, 2026
4 checks passed
@zbigniewsobiecki zbigniewsobiecki deleted the feature/router-immediate-reactions branch March 16, 2026 16:43
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.

3 participants