Skip to content

fix(subscriber): keep issue comment lookups on the owning save's connection - #3536

Open
fallenbagel wants to merge 1 commit into
developfrom
fix/issue-comment-notification-connection
Open

fallenbagel wants to merge 1 commit into
developfrom
fix/issue-comment-notification-connection

Conversation

@fallenbagel

@fallenbagel fallenbagel commented Sep 22, 2026

Copy link
Copy Markdown
Member

Description

IssueCommentSubscriber looked up the comment it had just inserted using the global getRepository(). That can use a different connection from the one running the insert transaction. On Postgres, the second connection cannot see the uncommitted row, so findOneOrFail throws and the ISSUE_COMMENT notification is dropped for every agent. SQLite effectively uses a single shared connection, so the lookup succeeds there, which is why this did not surface in SQLite.

This changes the lookup to use event.manager, which is tied to the connection running the insert transaction. This follows the same pattern already used by MediaSubscriber and MediaRequestSubscriber since #3365.

How Has This Been Tested?

Tested manually by adding a comment on an existing issue, on both
SQLite and Postgres.

Screenshots / Logs (if applicable)

Before the fix on Postgres, every comment threw:

2026-09-22T20:38:22.365Z [error][Notifications]: Something went wrong sending issue comment notification(s) {"errorMessage":"Could not find any entity of type "IssueComment" matching: {\n "where": {\n "id": 3\n },\n "relations": {\n "issue": {\n "createdBy": true\n }\n }\n}","commentId":3}

After the fix on Postgres, the notification sends correctly:

2026-09-22T20:37:33.118Z [info][Notifications]: Sending notification(s) for ISSUE_COMMENT {"subject":"Welcome to Waikiki (2018)"}
2026-09-22T20:37:33.119Z [debug][Notifications]: Sending Discord notification {"type":"ISSUE_COMMENT","subject":"Welcome to Waikiki (2018)"}

Checklist:

  • I have read and followed the contribution guidelines.
  • Disclosed any use of AI (see our policy)
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • Successful build pnpm build
  • Translation keys pnpm i18n:extract
  • Database migration (if required)

Summary by CodeRabbit

  • Bug Fixes
    • Improved issue comment notification processing to ensure newly added comments and related content are loaded consistently.
    • Ensured notification dispatch completes reliably after an issue comment is added.

@fallenbagel
fallenbagel requested a review from a team as a code owner September 22, 2026 20:49
@fallenbagel
fallenbagel enabled auto-merge (squash) September 22, 2026 20:49
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9b44b05c-4951-4e20-8574-ce8e72be04a5

📥 Commits

Reviewing files that changed from the base of the PR and between 794743a and 6fb4c23.

📒 Files selected for processing (1)
  • server/subscriber/IssueCommentSubscriber.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

IssueCommentSubscriber now loads issue comment notification data through the TypeORM event manager. afterInsert passes that manager to the notification helper and awaits its asynchronous processing.

Changes

Issue comment notification flow

Layer / File(s) Summary
Manager-scoped notification flow
server/subscriber/IssueCommentSubscriber.ts
sendIssueCommentNotification uses the event manager to load the comment, creator, and media. afterInsert returns Promise<void>, passes event.manager, and awaits notification processing.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: thecatlady, danshilm

Merge Risk: 🔵 Low · up to 6fb4c

In the uncommon case that saving a comment fails at commit, users may receive a notification for a comment that does not exist. Dispatch notifications from a post-commit outbox to make delivery consistent with saved comments.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: keeping issue comment lookups on the connection that performs the save.
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.

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

I’m a rabbit with comments to send,
Through the right manager, from start to end.
The creator and media hop in line,
Awaited notifications now arrive on time.
Small code changes, neatly done—
Hop, hop, hooray for everyone!

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

This branch has not been deployed

No deployments
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