Make outbox-listener-delivery-required path-aware - #1040
Jae-Hyuk-Jang wants to merge 3 commits into
Conversation
The rule decided whether an outbox listener delivers a posted activity by scanning the listener's source text for a ctx.sendActivity()/forwardActivity() call, regardless of whether that code actually runs. A call inside an unused nested helper function, behind a statically-dead branch, or inside a callback passed to an unrelated function (e.g. array.map()) was enough to satisfy the check. Add a scan that follows the listener's own reachable control flow — skipping dead branches and code after an unconditional return/throw, and not descending into a nested function's body unless that function is itself called from reachable code — before falling back to the existing text-based pattern matching for the delivery call itself. fedify-dev#900 Assisted-by: Claude Code:claude-sonnet-5
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
fedify-dev#1040 Assisted-by: Claude Code:claude-sonnet-5
codecov flagged the previous commit's reachable-statement walker as under-tested: the if/else, try/catch/finally, switch, and loop branches it added for control-flow traversal had no test exercising them, and the if (true)/else pairing—the mirror of the existing if (false) case—was untested in either direction. Add tests for a delivery call inside a non-literal if branch, inside try/catch/finally, inside a switch case, and inside a for-of loop, plus a regression test for a delivery call left in the dead else branch of if (true). fedify-dev#1040 Assisted-by: Claude Code:claude-sonnet-5
1 participant
Closes #900
Background
outbox-listener-delivery-required(@fedify/lint) decided whether a listener delivers a posted activity by scanning the whole listener source as text for actx.sendActivity()/forwardActivity()call, without checking whether that code actually runs.Changes
return/throw, and does not descend into a nested function's body unless that function is itself called from reachable code.return, and a positive case for a helper function that is actually called.Testing
mise run check-each lintmise run test-each lint(Deno and Node.js, 515/515 pass)AI disclosure
This was implemented with Claude Code (
claude-sonnet-5): I described the issue and reviewed the design and results at each step, and Claude Code designed the reachability-based rewrite, implemented it, found and fixed a bug during testing (Deno.lint exposes an AST node's children through prototype getters rather than own enumerable properties, soObject.entries()couldn't see them —for...inwas needed instead), and verified the tests pass in both Deno and Node.js.