fix(request-filters): preserve advanced mode when rebinding

[Brisbanehuang]

Summary

Preserve the existing ruleMode and operations when validating provider/group binding-only updates, preventing advanced-mode filters with an empty target from being incorrectly rejected.

Problem

This is a regression from #912 (advanced mode operations). When updating only binding fields (providerIds, groupTags) on an advanced-mode filter, the binding-validation path at src/actions/request-filters.ts:368 constructs a validatePayload call that omits ruleMode and operations. Without ruleMode, the validator defaults to "simple" mode, which enforces a non-empty target — but advanced-mode filters legitimately have empty target fields since they use operations instead. This makes binding-only updates on advanced filters fail with "目标字段不能为空".

Solution

Forward the existing filter's ruleMode and operations into the validatePayload call so the validator correctly recognizes advanced-mode filters and skips the empty-target check.

Follow-up to #912 — the advanced-mode validation path for binding-only updates was missed in the original implementation.

Changes

Core Fix

• src/actions/request-filters.ts — Pass existing.ruleMode and existing.operations to validatePayload in the binding-update validation block

Tests

• tests/unit/actions/request-filters-cache-reload.test.ts — Regression test: updating provider bindings on an advanced final filter with an empty target succeeds and calls the repository correctly

Testing

• bun run test tests/unit/actions/request-filters-cache-reload.test.ts
• bunx biome check src/actions/request-filters.ts tests/unit/actions/request-filters-cache-reload.test.ts

---

Description enhanced by Claude AI

Greptile Summary

This PR fixes a regression introduced in #912 where binding-only updates (providerIds, groupTags) on advanced-mode filters incorrectly failed with "目标字段不能为空". The root cause was the binding-validation call inside updateRequestFilterAction omitting ruleMode and operations, causing validatePayload to default to simple mode and enforce a non-empty target check that advanced filters legitimately bypass.

• Core fix (src/actions/request-filters.ts): effectiveRuleMode and effectiveOperations are now computed and forwarded to validatePayload in the binding-update validation block, so the validator correctly recognises advanced-mode filters and skips the empty-target check.
• Regression tests (tests/unit/actions/request-filters-cache-reload.test.ts): Two new tests cover the primary fix (binding update on advanced filter with empty target succeeds) and the boundary case (transitioning to simple mode without a target still fails correctly).

Confidence Score: 5/5

Safe to merge — the change is narrow, well-targeted, and covered by regression tests; no existing behaviour outside the advanced-mode binding path is affected.

The fix touches only two new lines in a single conditional block, correctly mirrors how every other effective-value computation in that block already works, and is guarded by two new tests covering both the happy path and the failure boundary.

No files require special attention beyond the noted pre-existing gap in the binding validation's target handling.

Important Files Changed

Filename	Overview
src/actions/request-filters.ts	Adds effectiveRuleMode and effectiveOperations to the binding-update validatePayload call, fixing the advanced-mode regression; existing!.target passed to that same call (rather than updates.target ?? existing!.target) is a pre-existing gap for combined target+binding updates.
tests/unit/actions/request-filters-cache-reload.test.ts	Extracts getRequestFilterById mock for reuse and adds two well-scoped regression tests covering both the fixed path and the mode-transition boundary.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[updateRequestFilterAction called] --> B{binding fields in updates?}
    B -- No --> G[other validation / DB write]
    B -- Yes --> C[compute effectiveBindingType\neffectiveProviderIds\neffectiveGroupTags]
    C --> D["compute effectiveRuleMode\n(updates.ruleMode ?? existing.ruleMode)\n[NEW]"]
    D --> E["compute effectiveOperations\n(updates.operations ?? existing.operations)\n[NEW]"]
    E --> F["validatePayload(name, scope, action,\ntarget, bindingType, providerIds, groupTags,\nruleMode, operations)"]
    F -- ruleMode=advanced --> H[validateOperations → skip empty-target check]
    F -- ruleMode=simple --> I{target empty?}
    I -- Yes --> J[return error]
    I -- No --> K[check binding constraints]
    H --> K
    K --> G

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/actions/request-filters.ts:371-381
The binding validation block now correctly computes `effectiveRuleMode` and `effectiveOperations`, but still passes `existing!.target` instead of the effective target. For a combined update such as `{ ruleMode: "simple", target: "new-value", providerIds: [...] }` on an existing advanced-mode filter (which has an empty `target`), the validator receives the old empty target alongside `ruleMode: "simple"` and rejects the request with "目标字段不能为空" even though the final state would be valid. Using the effective target here is consistent with how every other field in this block is computed.

```suggestion
    const effectiveTarget = updates.target ?? existing!.target;
    const validationError = validatePayload({
      name: existing!.name,
      scope: existing!.scope,
      action: existing!.action,
      target: effectiveTarget,
      bindingType: effectiveBindingType,
      providerIds: effectiveProviderIds,
      groupTags: effectiveGroupTags,
      ruleMode: effectiveRuleMode,
      operations: effectiveOperations,
    });
```

_{Reviews (2): Last reviewed commit: "fix(request-filters): preserve advanced ..." | Re-trigger Greptile}

[coderabbitai]

📝 Walkthrough

Walkthrough

补全了 updateRequestFilterAction 在绑定字段校验时传入的规则模式与操作集参数，并新增可控的 getRequestFilterById mock 与两条测试，覆盖允许更新 providerIds 的成功场景与基于更新后规则模式的失败校验场景。

变更

绑定更新校验补全

层 / 文件	总结
绑定校验参数补全 src/actions/request-filters.ts	updateRequestFilterAction 在校验绑定类型、提供者ID和群组标签更新时，补全了 validatePayload 的 ruleMode 和 operations 参数，使校验能基于现有过滤器的规则模式进行。
绑定更新测试与mock配置 tests/unit/actions/request-filters-cache-reload.test.ts	新增 getRequestFilterById 的 mock 定义和配置，并添加测试用例验证 advanced final 类型过滤器在特定绑定条件下允许更新 providerIds，以及在不满足更新后 rule mode 时返回失败且不调用更新与重载。

🎯 2 (Simple) | ⏱️ ~8 分钟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	PR标题清晰准确地概括了主要变更：在重新绑定时保留高级模式，与changeset中的核心修复完全相关。
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.
Description check	✅ Passed	PR description clearly relates to the changeset: fixes advanced-mode filter binding validation by preserving ruleMode/operations, matches file changes in request-filters.ts and test file.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request updates the updateRequestFilterAction to include ruleMode and operations from the existing filter during validation, and adds corresponding unit tests. A review comment points out that if ruleMode or operations are being updated in the same request, using the existing values directly will use stale data and cause validation failures. The reviewer suggests using the updated values from the payload if they are provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

When updating a request filter, if both the binding-related fields (such as providerIds, groupTags, or bindingType) and the rule mode/operations are updated in the same request, using existing!.ruleMode and existing!.operations directly in validatePayload will use the stale (pre-update) values.

This can cause validation to fail or run against the wrong mode (e.g., validating an advanced filter as a simple one, leading to errors like '目标字段不能为空'). We should use the updated values from updates if they are provided.

Suggested change

	ruleMode: existing!.ruleMode,
	operations: existing!.operations,
	ruleMode: updates.ruleMode ?? existing!.ruleMode,
	operations: updates.operations !== undefined ? updates.operations : existing!.operations,

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 279e968e15

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Validate binding updates against the effective rule mode

When a single update both changes bindings and switches an existing advanced filter back to simple, this still validates with existing!.ruleMode/existing!.operations. For an advanced filter whose target is intentionally empty, a PATCH such as { ruleMode: "simple", providerIds: [...] } passes validation because validatePayload skips simple-mode target checks, then persists a simple filter with an empty target. Use the effective updates.ruleMode ?? existing!.ruleMode (and corresponding operations) here so mixed mode/binding updates are validated in the state that will actually be saved.

Useful? React with 👍 / 👎.

[github-actions]

Code Review Summary

No significant issues identified in this PR. The fix correctly preserves ruleMode and operations from the existing record when calling validatePayload during binding-only updates, preventing advanced filters with empty targets from being incorrectly rejected as simple-mode filters. The regression test directly covers the reported bug scenario.

PR Size: XS

• Lines changed: 33 (32 additions, 1 deletion)
• Files changed: 2

Review Coverage

• Logic and correctness - Clean. The existing\! non-null assertion at lines 375-376 is safe because needsExisting (line 296) includes binding-related fields, guaranteeing the null check at line 310 has already passed.
• Security (OWASP Top 10) - Clean. No new attack surface introduced.
• Error handling - Clean. Follows existing patterns with proper error propagation.
• Type safety - Clean. ruleMode and operations types match the validatePayload signature.
• Documentation accuracy - Clean. No comment drift.
• Test coverage - Adequate. Regression test covers the exact bug scenario (advanced filter + empty target + provider binding update).
• Code clarity - Good. Minimal, targeted fix.

---

Automated review by Claude AI