Skip to content

Start the fanout worker when only the fanout queue is configured - #1392

Open
dahlia wants to merge 1 commit into
fedify-dev:2.0-maintenancefrom
dahlia:bugfix/fanout-only-queue-never-starts-worker
Open

dahlia wants to merge 1 commit into
fedify-dev:2.0-maintenancefrom
dahlia:bugfix/fanout-only-queue-never-starts-worker

Conversation

@dahlia

@dahlia dahlia commented Nov 5, 2026

Copy link
Copy Markdown
Member

FederationImpl._startQueueInternal() in packages/fedify/src/federation/middleware.ts returned early when the inbox and outbox queues were unset. The fanout queue was added later without updating that guard, so queue: { fanout: ... } enqueued FanoutMessage objects that nothing consumed. Adding an inbox or outbox queue started the fanout worker as a side effect, which is why the bug looked intermittent.

The guard now includes fanoutQueue. I left the per-role identity checks unchanged: the shared-instance selector defect is pre-existing, already fixed on 2.4, and unrelated to the fanout-only configuration, so it is out of scope here.

The regression tests cover explicit startQueue(..., { queue: "fanout" }), no-selector startQueue(), and implicit startup after sendActivity(..., { fanout: "force" }). All three regression tests fail with the old guard. A manual-start case asserts that sending enqueues without starting a worker. The changelog fragment is materialized into CHANGES.md so sacho check passes.

Fixes #1389.

FederationImpl._startQueueInternal() began with a guard that only
checked the inbox and outbox queues.  The fanout queue was added in
1.5.0, but the guard was not updated, so a federation configured with
only queue: { fanout: ... } returned before reaching the fanout
branch.  ContextImpl.sendActivityInternal() still enqueued a
FanoutMessage, and because nothing called fanoutQueue.listen(), the
messages stayed in the backend unprocessed.

The failure looked intermittent because adding an inbox or outbox
queue started the fanout worker as a side effect, and non-forced sends
with fewer than five inboxes bypass the fanout queue entirely.
Nothing was logged either, because the early return preceded the
logger.

Include fanoutQueue in the guard, and add regression tests for an
explicit startQueue(..., { queue: "fanout" }), a no-selector
startQueue(), and the implicit startup after a forced fanout enqueue.
A manual-start test asserts that sending still enqueues without
starting a worker.

fedify-dev#1389

Assisted-by: OpenCode:deepseek-flash
Assisted-by: Codex:gpt-6-astra
Assisted-by: Claude Code:claude-fable-5-1
Assisted-by: OpenCode:mimo-v2.6-flash-free
@dahlia dahlia self-assigned this Nov 5, 2026
@dahlia dahlia added component/federation Federation object related component/mq Message queue related labels Nov 5, 2026
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The queue startup guard now checks for a configured fanout queue before returning. Tests cover explicit startup, default queue selection, automatic startup after fanout enqueue, and the manuallyStartQueue option.

Changes: Fanout queue startup

Layer / File(s) Summary
Fanout worker startup and regression coverage
packages/fedify/src/federation/middleware.ts, packages/fedify/src/federation/middleware.test.ts, CHANGES.md, changes.d/fedify/fanout-only-queue-worker.md
The startup guard now proceeds when a fanout queue is configured without inbox or outbox queues. Tests cover explicit and default startup, repeated explicit startup, automatic startup after enqueue, and suppression of automatic startup when manuallyStartQueue is enabled. The changelog entries describe the fix and link issues #1389 and #1392.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

Possibly related PRs

  • fedify-dev/fedify#806: Both changes modify _startQueueInternal to control which configured queue workers start.

Suggested labels: component/testing

Suggested reviewers: 2chanhaeng

Merge Risk: Minimal · up to 061f0

Fanout-only configurations can start their worker, and no material merge risk remains identified.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: starting the fanout worker when only the fanout queue is configured.
Description check Passed The description directly explains the fanout-only queue bug, the guard change, regression tests, scope, and changelog update.
Linked Issues check Passed [#1389] Requirement: the _startQueueInternal() guard must not return early when only the fanout queue is configured. Evidence: the diff at head changes the guard to return only when inboxQueue, `o…
Out of Scope Changes check Passed The changes at head are: the _startQueueInternal() guard in middleware.ts (the fix itself), regression and manual-start tests in middleware.test.ts (supporting tests for #1389), a changelog fragment…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check

Explanation

[#1389] Requirement: the _startQueueInternal() guard must not return early when only the fanout queue is configured. Evidence: the diff at head changes the guard to return only when inboxQueue, outboxQueue, and fanoutQueue are all null (packages/fedify/src/federation/middleware.ts, lines 509-518). This matches the fix proposed in #1389. Requirement: startQueue() with no selector or with { queue: "fanout" }, and automatic startup after a fanout enqueue, must call fanoutQueue.listen(). The guard fix allows these paths to reach the existing fanout branch. The supplied summary states that middleware.test.ts adds counting-queue tests for explicit fanout startup (called twice, started once), no-selector startup, and automatic startup after fanout enqueue, plus a manual-start case with manuallyStartQueue enabled. Requirement: a changelog fragment for the next 2.0.x release. Evidence: changes.d/fedify/fanout-only-queue-worker.md is added and CHANGES.md is updated under 2.0.36. Limits: the test file blob was unavailable, so the test assertions rely on the supplied summary and were not read directly. The body of the fanout branch in _startQueueInternal() was truncated in the read and was not inspected; the issue states the remainder of the function is null-safe. The claim that the new tests fail on the old guard is not verified here. Backports to 2.1, 2.2, and 2.3 maintenance branches named in #1389 are branch operations outside this PR's diff; the target branch of this PR cannot be confirmed from the head and merge-base evidence.

Full details: Out of Scope Changes check

Explanation

The changes at head are: the _startQueueInternal() guard in middleware.ts (the fix itself), regression and manual-start tests in middleware.test.ts (supporting tests for #1389), a changelog fragment in changes.d/fedify/ (required by #1389), and the matching CHANGES.md entry under 2.0.36 (the materialized changelog). The PR description states that the per-role identity checks are left unchanged. The supplied summary lists no other source changes. No change was found that is unconnected to #1389. The middleware.test.ts diff was not directly readable, so test-file content is judged from the supplied summary only.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (2 skipped: 2 unsupported.)

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 49e92b66-17a6-49e3-8c89-b38b4d01680f
Commits

Reviewing files that changed from the base of the PR and between 90504ae and 061f0ad.

Files selected for processing: 4

  • CHANGES.md
  • changes.d/fedify/fanout-only-queue-worker.md
  • packages/fedify/src/federation/middleware.test.ts
  • packages/fedify/src/federation/middleware.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T15:52:34.492612Z 061f0ad PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@dahlia dahlia linked an issue Nov 5, 2026 that may be closed by this pull request
@codecov

codecov Bot commented Nov 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines Coverage Δ
packages/fedify/src/federation/middleware.ts 92.24% <100.00%> (+0.14%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

component/federation Federation object related component/mq Message queue related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Start the fanout worker when only the fanout queue is configured

1 participant