fix: issue with multiple on query plugins with args override

[ymc9]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/on-query-args-override

---

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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: Fix for Multiple OnQuery Plugins with Args Override

Review Checklist:

• Read and analyze changed files
• Assess code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Analyze test coverage
• Provide comprehensive feedback

Summary

This PR fixes a critical issue with the plugin system where arguments overridden by one onQuery plugin were not properly propagated to earlier plugins in the chain. The fix ensures proper argument flow through the plugin middleware chain.

Code Quality & Best Practices ✅

Strengths:

• Clean, focused change addressing a specific issue
• Proper TypeScript typing with explicit parameter types
• Good separation of concerns in the plugin middleware pattern
• Clear variable naming (_args, nextArgs) indicating argument flow

Minor Suggestions:

• The comment "reflect the latest override if provided" could be more specific about the override propagation mechanism

Bug Fixes & Correctness ✅

Problem Solved:
The original code had a critical flaw in packages/runtime/src/client/client-impl.ts:382:

// Before (buggy)
proceed = () => onQuery({ client, model, operation, args, proceed: _proceed })

// After (fixed) 
proceed = (_args: unknown) => onQuery({
    client, model, operation,
    args: _args,  // Now properly reflects overrides
    proceed: (nextArgs: unknown) => _proceed(nextArgs)
})

Key Improvements:

1. Parameter Threading: proceed now accepts _args parameter and passes it to onQuery
2. Override Propagation: Arguments modified by later plugins are now visible to earlier plugins
3. Type Safety: Removed optional parameter (_args?: unknown → _args: unknown) for better type safety
4. Middleware Chain: Inner plugin overrides properly propagate through the chain via proceed: (nextArgs: unknown) => _proceed(nextArgs)

Performance Considerations ✅

• Minimal Overhead: Changes don't introduce new allocations or computational complexity
• Function Closure: Proper closure handling maintains performance characteristics
• No Breaking Changes: Existing plugin behavior remains unchanged for plugins not using arg overrides

Security Concerns ✅

• Input Validation: Arguments still go through existing validation pipelines
• No Injection Risks: Type-safe parameter passing prevents injection issues
• Plugin Isolation: Plugins can still only modify their own scope

Test Coverage ✅

Excellent Test Addition:
The test in packages/runtime/test/plugin/on-query-hooks.test.ts:215-251 specifically validates the fix:

it('propagates overridden args across multiple onQuery plugins', async () => {
    // Plugin A should see overrides from Plugin B
    const clientA = _client.$use({
        id: 'plugin-a',
        onQuery: (ctx) => {
            // Validates it sees the overridden args
            earlierSawOverridden = (ctx.args as any)?.where?.id === 'non-exist';
            return ctx.proceed(ctx.args);
        },
    });

    // Plugin B overrides args
    const client = clientA.$use({
        id: 'plugin-b', 
        onQuery: (ctx) => {
            return ctx.proceed({ where: { id: 'non-exist' } });
        },
    });
    // ... validation logic
});

This test perfectly captures the bug scenario and validates the fix.

Overall Assessment ⭐⭐⭐⭐⭐

Excellent PR:

• ✅ Fixes a real architectural issue in the plugin system
• ✅ Minimal, surgical change with maximum impact
• ✅ Comprehensive test coverage for the specific issue
• ✅ Maintains backward compatibility
• ✅ Type-safe implementation
• ✅ No performance regressions

Recommendation: APPROVE - This is a well-crafted fix that resolves a critical plugin system bug while maintaining code quality standards.

---

[Copilot]

Pull Request Overview

This PR fixes an issue where multiple onQuery plugins with argument overrides weren't properly propagating changes between plugins in the correct order.

• Modified the plugin chain execution to ensure argument overrides from later plugins are visible to earlier plugins
• Added comprehensive test coverage to verify the fix works correctly for multiple chained onQuery plugins

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
packages/runtime/src/client/client-impl.ts	Updates plugin execution logic to properly propagate argument overrides between chained onQuery plugins
packages/runtime/test/plugin/on-query-hooks.test.ts	Adds test case to verify multiple onQuery plugins correctly share overridden arguments

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}