fix: 规则保存后即时生效（同步内存缓存 + 刷新列表），无需手动点刷新#1244
Conversation
Previously, after creating, editing, or deleting an error rule or request filter, the in-memory cache was not updated and the UI list remained stale until the user clicked the refresh cache button. This meant proxy requests continued to use old rule versions until a manual refresh. Now server actions directly trigger engine and detector reloads (errorRuleDetector.reload() and requestFilterEngine.reload()) and UI components call router.refresh() to reflect changes immediately. Additionally, request-filter-engine was strengthened to handle concurrent reloads: if a reload is in-flight and another is requested, a second DB read is queued and executed automatically, preventing dropped updates. The UI for error rules (add, edit, toggle, delete, refresh button) now refreshes the list via router.refresh after successful mutations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough在 Actions 的错误规则与请求过滤器 mutation 成功后新增本进程缓存重载调用；多个 error-rules UI 组件在成功后调用 router.refresh() 刷新页面；RequestFilterEngine 将 reload() 改为并发合并 + 补跑队列模型；新增对应单元与回归测试覆盖。 变更缓存同步与 UI 刷新实现
总体概览该 PR 在 mutation 成功后实现本进程缓存的 best-effort 重载与页面级刷新，并为请求过滤器引擎引入并发复用与排队补跑语义，配套增加单元与回归测试以验证新行为。 代码审查工作量🎯 4 (Complex) | ⏱️ ~45 分钟 可能相关的 PR
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces immediate memory cache synchronization and UI refreshing upon creating, updating, or deleting error rules and request filters, along with a queuing mechanism in RequestFilterEngine to handle concurrent reloads. The reviewer provided valuable feedback to optimize performance and availability: first, by checking initialization status before waiting for active reloads in ensureInitialized to avoid blocking proxy requests with database queries; and second, by introducing a queue parameter to bypass redundant, back-to-back reload operations triggered concurrently by event emissions and direct action calls.
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.
| private async ensureInitialized(): Promise<void> { | ||
| // 有在途 reload（含排队补跑）时等待它完成，避免读到补跑前的旧快照。 | ||
| if (this.activeReloadPromise) { | ||
| await this.activeReloadPromise; | ||
| return; | ||
| } | ||
| if (this.isInitialized) return; |
There was a problem hiding this comment.
在代理请求的路径中，延迟和可用性是至关重要的。目前的 ensureInitialized 实现将 activeReloadPromise 的等待放在了 this.isInitialized 判断之前。这意味着，一旦管理员在后台修改了规则触发了 reload，所有并发的代理请求都将被阻塞，直到数据库查询和缓存重构完成。这不仅会增加代理请求的延迟，还让代理请求的可用性直接强绑定到了数据库的可用性上（如果数据库慢或宕机，代理请求也会挂起或失败）。
在缓存设计中，对于代理服务，最终一致性（在后台加载完成前继续使用旧的内存快照）远比强一致性（阻塞请求等待最新快照）更合适。
建议将 this.isInitialized 的判断移到最前面，只有在缓存完全未初始化（如服务刚启动）时才阻塞等待在途的 reload。
| private async ensureInitialized(): Promise<void> { | |
| // 有在途 reload（含排队补跑）时等待它完成，避免读到补跑前的旧快照。 | |
| if (this.activeReloadPromise) { | |
| await this.activeReloadPromise; | |
| return; | |
| } | |
| if (this.isInitialized) return; | |
| private async ensureInitialized(): Promise<void> { | |
| if (this.isInitialized) return; | |
| // 仅在未初始化且有在途 reload 时等待其完成，避免在已初始化的情况下阻塞代理请求。 | |
| if (this.activeReloadPromise) { | |
| await this.activeReloadPromise; | |
| return; | |
| } |
| async reload(): Promise<void> { | ||
| if (this.isLoading) return; | ||
| this.isLoading = true; | ||
|
|
||
| try { | ||
| const { getActiveRequestFilters } = await import("@/repository/request-filters"); | ||
| const filters = await getActiveRequestFilters(); | ||
| this.loadFilters(filters); | ||
| } catch (error) { | ||
| logger.error("[RequestFilterEngine] Failed to reload filters", { error }); | ||
| } finally { | ||
| this.isLoading = false; | ||
| if (this.activeReloadPromise) { | ||
| // 已有 reload 在途时排队补跑一轮：保存规则后的显式 reload 不能被丢弃， | ||
| // 否则代理仍会命中旧的内存快照，必须等用户手动点"刷新缓存"才生效。 | ||
| this.reloadRequestedWhileLoading = true; | ||
| return this.activeReloadPromise; | ||
| } |
There was a problem hiding this comment.
目前的 reload() 实现中，只要有在途的 reload，就会无条件设置 this.reloadRequestedWhileLoading = true 并排队补跑一轮。
然而，在新增/编辑/删除规则的 Action 中，代码会先调用 emitRequestFiltersUpdated()（这会同步触发本地事件监听器并启动一个 reload），紧接着又调用 await requestFilterEngine.reload()。这会导致 reloadRequestedWhileLoading 总是被设为 true，从而强制执行第二次完全冗余的 reload 循环（包括一次额外的数据库查询）。
这不仅使数据库查询开销翻倍，还导致管理员在 UI 上保存规则时的等待时间翻倍（因为 Action 会等待两个完整的 reload 周期完成）。
建议为 reload() 增加一个可选的 queue 参数（默认为 true），在 Action 中显式传入 false，这样 Action 就可以直接等待已经由事件触发的、包含最新数据的在途 reload，而无需排队补跑第二轮。
async reload(queue = true): Promise<void> {
if (this.activeReloadPromise) {
// 已有 reload 在途时排队补跑一轮：保存规则后的显式 reload 不能被丢弃，
// 否则代理仍会命中旧的内存快照，必须等用户手动点"刷新缓存"才生效。
if (queue) {
this.reloadRequestedWhileLoading = true;
}
return this.activeReloadPromise;
}| // 立即同步内存缓存，确保新规则对代理请求即时生效，无需用户手动点"刷新缓存"。 | ||
| await requestFilterEngine.reload(); |
There was a problem hiding this comment.
配合 requestFilterEngine.reload(queue) 的优化，这里应该传入 false。因为 createRequestFilter 内部已经调用了 emitRequestFiltersUpdated() 触发了本地事件驱动的 reload，此处我们只需要 await 该在途的 reload 即可，无需排队补跑第二轮，从而避免冗余的数据库查询并缩短响应时间。
| // 立即同步内存缓存，确保新规则对代理请求即时生效，无需用户手动点"刷新缓存"。 | |
| await requestFilterEngine.reload(); | |
| // 立即同步内存缓存，确保新规则对代理请求即时生效，无需用户手动点"刷新缓存"。 | |
| await requestFilterEngine.reload(false); |
| // 立即同步内存缓存，确保规则改动对代理请求即时生效，无需用户手动点"刷新缓存"。 | ||
| await requestFilterEngine.reload(); |
There was a problem hiding this comment.
配合 requestFilterEngine.reload(queue) 的优化，这里应该传入 false。因为 updateRequestFilter 内部已经调用了 emitRequestFiltersUpdated() 触发了本地事件驱动的 reload，此处我们只需要 await 该在途的 reload 即可，无需排队补跑第二轮，从而避免冗余的数据库查询并缩短响应时间。
| // 立即同步内存缓存，确保规则改动对代理请求即时生效，无需用户手动点"刷新缓存"。 | |
| await requestFilterEngine.reload(); | |
| // 立即同步内存缓存，确保规则改动对代理请求即时生效，无需用户手动点"刷新缓存"。 | |
| await requestFilterEngine.reload(false); |
| // 立即同步内存缓存，确保删除对代理请求即时生效，无需用户手动点"刷新缓存"。 | ||
| await requestFilterEngine.reload(); |
There was a problem hiding this comment.
配合 requestFilterEngine.reload(queue) 的优化，这里应该传入 false。因为 deleteRequestFilter 内部已经调用了 emitRequestFiltersUpdated() 触发了本地事件驱动的 reload，此处我们只需要 await 该在途的 reload 即可，无需排队补跑第二轮，从而避免冗余的数据库查询并缩短响应时间。
| // 立即同步内存缓存，确保删除对代理请求即时生效，无需用户手动点"刷新缓存"。 | |
| await requestFilterEngine.reload(); | |
| // 立即同步内存缓存，确保删除对代理请求即时生效，无需用户手动点"刷新缓存"。 | |
| await requestFilterEngine.reload(false); |
| // 本进程立即同步内存缓存，确保规则改动对代理请求即时生效，无需手动点"刷新缓存"。 | ||
| await errorRuleDetector.reload(); |
There was a problem hiding this comment.
这里存在与 request-filters 类似的双重 reload 性能问题。emitErrorRulesUpdated() 会触发本地事件监听器启动一个 reload，随后紧接着调用 await errorRuleDetector.reload() 会导致 reloadRequestedWhileLoading 被设为 true，从而强制排队补跑第二轮。
这导致了冗余的数据库查询，并使管理员保存规则时的响应时间翻倍。建议后续对 ErrorRuleDetector 也进行类似的 reload(queue = false) 优化，并在此处传入 false。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/actions/error-rules-cache-reload.test.ts (1)
68-118: ⚡ Quick win建议补充
reload()抛错时的行为覆盖。当前用例验证了成功路径与“删除目标不存在”路径，但未覆盖
reload()抛错的场景。结合src/actions/error-rules.ts中 reload 失败会被误报为整体失败的问题，建议新增一条断言：当reloadMock.mockRejectedValueOnce(...)时，写入已成功的动作仍返回ok: true，以锁定修复后的语义。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/actions/error-rules-cache-reload.test.ts` around lines 68 - 118, Add a test ensuring a failing reload does not flip a successful action to failed: in the existing suite add a case that mocks reloadMock.mockRejectedValueOnce(new Error("boom")), calls each action under test (e.g., createErrorRuleAction({...}), updateErrorRuleAction(1, {...}), or deleteErrorRuleAction(1) as appropriate), and asserts the action result still returns ok: true while reloadMock was called (to confirm reload was attempted but its rejection does not change the action's success semantics).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/actions/error-rules.ts`:
- Around line 179-184: The current flow lets errorRuleDetector.reload() failures
turn a successful repo.createErrorRule into an overall failure; change each
action (createErrorRuleAction, updateErrorRuleAction, deleteErrorRuleAction) so
that the DB operation (repo.createErrorRule / repo.updateErrorRule /
repo.deleteErrorRule) and emitErrorRulesUpdated() determine success, while
errorRuleDetector.reload() and revalidatePath("/settings/error-rules") are
executed in their own try/catch that logs the error (or processLogger.error) but
does not re-throw or flip the action result; in short, wrap calls to
errorRuleDetector.reload() and revalidatePath(...) in a safe catch block and
ensure the action returns { ok: true, ... } whenever the repository operation
succeeded.
---
Nitpick comments:
In `@tests/unit/actions/error-rules-cache-reload.test.ts`:
- Around line 68-118: Add a test ensuring a failing reload does not flip a
successful action to failed: in the existing suite add a case that mocks
reloadMock.mockRejectedValueOnce(new Error("boom")), calls each action under
test (e.g., createErrorRuleAction({...}), updateErrorRuleAction(1, {...}), or
deleteErrorRuleAction(1) as appropriate), and asserts the action result still
returns ok: true while reloadMock was called (to confirm reload was attempted
but its rejection does not change the action's success semantics).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58a005c2-d4dd-4668-a31e-0b75954c6cab
📒 Files selected for processing (11)
src/actions/error-rules.tssrc/actions/request-filters.tssrc/app/[locale]/settings/error-rules/_components/add-rule-dialog.tsxsrc/app/[locale]/settings/error-rules/_components/edit-rule-dialog.tsxsrc/app/[locale]/settings/error-rules/_components/refresh-cache-button.tsxsrc/app/[locale]/settings/error-rules/_components/rule-list-table.tsxsrc/lib/request-filter-engine.tstests/unit/actions/error-rules-cache-reload.test.tstests/unit/actions/request-filters-cache-reload.test.tstests/unit/error-rules-list-refresh-ui.test.tsxtests/unit/lib/request-filter-engine-reload-queue.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR fixes a real user-facing bug where request filters and error rules required a manual "Refresh Cache" click after mutations. The fix is cohesive: server-side actions now await reload() after DB writes to synchronize the in-memory cache, and client-side error-rule components now call router.refresh() to match the request-filters behavior that already existed. The RequestFilterEngine.reload() method is correctly ported from the proven ErrorRuleDetector queueing pattern (activeReloadPromise + reloadRequestedWhileLoading do/while loop with post-settlement safety net). Test coverage is solid with 4 new test files covering reload queueing, cache-refresh-on-mutation, and UI refresh behavior.
PR Size: L
- Lines changed: 718
- Files changed: 11
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
No significant issues identified. All changes are in new/modified lines, correctly scoped to the fix, and covered by tests.
Split Suggestion (L PR)
The PR is already cohesive around a single bug fix. If a split were desired, the natural seam would be: (1) server-side reload + engine queueing (core fix) and (2) client-side router.refresh() additions (UX polish) — but both halves are tightly coupled to the same user story, so splitting adds process overhead without real benefit.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (all new
await reload()calls are inside existing try/catch blocks; engine reload has internal error logging) - Type safety - Clean
- Documentation accuracy - Clean (comments accurately describe behavior)
- Test coverage - Adequate (4 new test files with 10 test cases covering reload queue, mutation-triggered reload, and UI refresh)
- Code clarity - Good (reload queueing pattern mirrors proven ErrorRuleDetector implementation)
Automated review by Claude AI
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // 刷新缓存（事件广播，支持多 worker 同步） | ||
| await emitErrorRulesUpdated(); | ||
| // 本进程立即同步内存缓存，确保规则改动对代理请求即时生效，无需手动点"刷新缓存"。 | ||
| await errorRuleDetector.reload(); |
There was a problem hiding this comment.
emitErrorRulesUpdated() synchronously fires the errorRulesUpdated event, whose local listener immediately calls void this.reload(), starting the first reload and setting activeReloadPromise. The very next line then calls await errorRuleDetector.reload(), which — because activeReloadPromise is already set — sets reloadRequestedWhileLoading = true and piggybacks on the first reload, causing a mandatory second DB read after the first completes.
The result is 2 DB round-trips per create/update/delete, whereas request-filters (which does not call an equivalent emitRequestFiltersUpdated() from the action layer) only does 1. If the intent is to keep the event for multi-worker Redis pub/sub sync and also guarantee the local process is current, consider whether the local EventEmitter listener should be skipped when the action already calls reload() explicitly, or simply remove the local EventEmitter subscription from ErrorRuleDetector and rely solely on the action's direct reload() for local sync and Redis pub/sub for remote workers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/error-rules.ts
Line: 179-182
Comment:
**Double DB read per error-rules mutation**
`emitErrorRulesUpdated()` synchronously fires the `errorRulesUpdated` event, whose local listener immediately calls `void this.reload()`, starting the first reload and setting `activeReloadPromise`. The very next line then calls `await errorRuleDetector.reload()`, which — because `activeReloadPromise` is already set — sets `reloadRequestedWhileLoading = true` and piggybacks on the first reload, causing a mandatory second DB read after the first completes.
The result is 2 DB round-trips per create/update/delete, whereas `request-filters` (which does not call an equivalent `emitRequestFiltersUpdated()` from the action layer) only does 1. If the intent is to keep the event for multi-worker Redis pub/sub sync and also guarantee the local process is current, consider whether the local EventEmitter listener should be skipped when the action already calls `reload()` explicitly, or simply remove the local EventEmitter subscription from `ErrorRuleDetector` and rely solely on the action's direct `reload()` for local sync and Redis pub/sub for remote workers.
How can I resolve this? If you propose a fix, please make it concise.| if (this.activeReloadPromise) { | ||
| await this.activeReloadPromise; | ||
| return; | ||
| } |
There was a problem hiding this comment.
ensureInitialized() skips re-initialization after a silently-failed reload
If activeReloadPromise is set and the in-flight reload fails internally (exception caught and logged, isInitialized never set to true), this method awaits the promise, then returns unconditionally without falling through to the initializationPromise path. The caller proceeds with an uninitialized engine (empty filter arrays). The next proxy request will trigger a fresh initializationPromise, so the gap is transient — but for that one request, filters may be silently skipped. Adding if (this.isInitialized) return; after the await would close this gap.
| if (this.activeReloadPromise) { | |
| await this.activeReloadPromise; | |
| return; | |
| } | |
| if (this.activeReloadPromise) { | |
| await this.activeReloadPromise; | |
| if (this.isInitialized) return; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/request-filter-engine.ts
Line: 453-456
Comment:
**`ensureInitialized()` skips re-initialization after a silently-failed reload**
If `activeReloadPromise` is set and the in-flight reload fails internally (exception caught and logged, `isInitialized` never set to `true`), this method awaits the promise, then returns unconditionally without falling through to the `initializationPromise` path. The caller proceeds with an uninitialized engine (empty filter arrays). The next proxy request will trigger a fresh `initializationPromise`, so the gap is transient — but for that one request, filters may be silently skipped. Adding `if (this.isInitialized) return;` after the await would close this gap.
```suggestion
if (this.activeReloadPromise) {
await this.activeReloadPromise;
if (this.isInitialized) return;
}
```
How can I resolve this? If you propose a fix, please make it concise.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Previous implementation awaited the active reload promise on every ensureInitialized call, potentially blocking the proxy hot path while a reload was in progress. Since loadFilters performs atomic bucket swaps, the proxy can read a consistent snapshot without waiting. This change removes the blocking await, relying on the action-side awaited reload to guarantee immediate effect after saves. Add regression coverage for AddRuleDialog and RefreshCacheButton to verify that router.refresh is called after creating a rule and refreshing the cache. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🧪 测试结果
总体结果: ✅ 所有测试通过 |
After mutation, the cache reload was called synchronously and its failure would cause the action to return an error, potentially leading to double-create on retry. Now the reload is best-effort, logging a warning on failure while the successful write is preserved. Additionally, the reload methods now accept a parameter to control queueing when a reload is already in progress. Mutation actions pass false to reuse the emit-triggered reload, avoiding a redundant second DB read and speeding up the save response. The ensureInitialized method also reuses an in-flight reload during cold start to prevent duplicate reads. Added regression tests for reload-failure tolerance and reload(false) reuse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
已处理 Review 意见（commit
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/request-filter-engine.ts (1)
341-383: 💤 Low valuereload() 的 finally 递归返回值未被正确传播。
在 Line 378，
finally块中的return this.reload()返回值不会被.finally()传播给调用方。.finally()的返回值仅在抛出异常或返回一个 rejected Promise 时才会影响结果链。这意味着当shouldRestart为 true 时，调用方拿到的是第一轮reloadLoop的 resolved promise，而非补跑那一轮的 promise。虽然在当前业务场景下（best-effort reload）这不会导致数据不一致，但与
ErrorRuleDetector中使用async finally+return this.reload()的写法存在细微差异，可能导致调用方误以为 await 返回时缓存已是最新。建议：对齐 ErrorRuleDetector 的写法，使用 async finally
- this.activeReloadPromise = reloadLoop.finally(() => { + this.activeReloadPromise = reloadLoop.finally(async () => { // 极窄窗口：do/while 已判定无需继续，但 finally 微任务执行前又来了新请求， // 此处再检查一次，避免晚到的补跑被静默吞掉。 const shouldRestart = this.reloadRequestedWhileLoading; this.activeReloadPromise = null; if (shouldRestart) { - return this.reload(); + await this.reload(); } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/request-filter-engine.ts` around lines 341 - 383, The finally callback on reloadLoop doesn't propagate the recursive reload promise to callers; change how this.activeReloadPromise is set so the restart is awaited and returned: instead of using reloadLoop.finally(...), wrap awaiting reloadLoop in an async IIFE (or an async wrapper) that awaits reloadLoop, then clears this.activeReloadPromise, checks this.reloadRequestedWhileLoading and if true returns await this.reload() so the returned promise reflects the restarted run. Update references around reload(), reloadLoop and this.activeReloadPromise to use this async wrapper approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/request-filter-engine.ts`:
- Around line 341-383: The finally callback on reloadLoop doesn't propagate the
recursive reload promise to callers; change how this.activeReloadPromise is set
so the restart is awaited and returned: instead of using
reloadLoop.finally(...), wrap awaiting reloadLoop in an async IIFE (or an async
wrapper) that awaits reloadLoop, then clears this.activeReloadPromise, checks
this.reloadRequestedWhileLoading and if true returns await this.reload() so the
returned promise reflects the restarted run. Update references around reload(),
reloadLoop and this.activeReloadPromise to use this async wrapper approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5914e83-402a-49eb-b3d8-682cffb501dd
📒 Files selected for processing (7)
src/actions/error-rules.tssrc/actions/request-filters.tssrc/lib/error-rule-detector.tssrc/lib/request-filter-engine.tstests/unit/actions/error-rules-cache-reload.test.tstests/unit/actions/request-filters-cache-reload.test.tstests/unit/lib/request-filter-engine-reload-queue.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/actions/request-filters.ts
- tests/unit/actions/request-filters-cache-reload.test.ts
- tests/unit/actions/error-rules-cache-reload.test.ts
- tests/unit/lib/request-filter-engine-reload-queue.test.ts
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
问题
请求过滤器（Request Filters）和错误规则（Error Rules）在新增/编辑/删除/启停后，需要手动点击"刷新缓存"按钮才会真正生效。用户在请求过滤器侧可稳定复现。
复现与根因分析
定位到代理运行时有一层进程内内存缓存（
RequestFilterEngine/ErrorRuleDetector），代理请求实际命中的是这层缓存，而非数据库。回答用户的两个问题：
filter-dialog/filter-table已调用router.refresh()），但代理内存缓存只靠仓储层 fire-and-forget 的事件触发void reload()，动作层从不await。add/edit/toggle/delete均未调用router.refresh()，仅靠revalidatePath——经 REST 客户端调用时不会触发客户端重渲染），代理内存缓存同样只靠 fire-and-forget 事件。await reload()同步刷新内存缓存的路径。此外
RequestFilterEngine.reload()有一个隐患：if (this.isLoading) return;会在已有 reload 进行中时静默丢弃并发的 reload（没有补跑队列，不同于已经健壮的ErrorRuleDetector）。改动
服务端（让规则对代理即时生效）
RequestFilterEngine.reload()：移植ErrorRuleDetector的activeReloadPromise+reloadRequestedWhileLoading补跑队列，并发 reload 不再被丢弃；ensureInitialized()会等待在途 reload 完成，避免读到旧快照。request-filters/error-rules的 create/update/delete 动作：写库成功后直接await reload()，确保返回前本进程缓存已是最新。仍保留事件广播（Redis pub/sub）用于多 worker 同步。前端（让列表即时更新）
add-rule-dialog、edit-rule-dialog、rule-list-table（启停 + 删除）、refresh-cache-button全部在成功后调用router.refresh()，与请求过滤器已有行为对齐。测试（TDD，先红后绿）
新增 4 个测试文件，均先确认 RED 再实现转 GREEN：
request-filter-engine-reload-queue.test.ts：并发/事件触发的 reload 不再被丢弃，await的 reload 能观察到最新快照。request-filters-cache-reload.test.ts/error-rules-cache-reload.test.ts：create/update/delete 成功后会await引擎/检测器的reload()，失败时不会。error-rules-list-refresh-ui.test.tsx：错误规则启停/删除/编辑保存后会调用router.refresh()。验证
bun run build✅bun run typecheck✅bun run lint✅bun run test✅ 6262 passed / 13 skipped（696 文件）Related PRs
activeReloadPromise+reloadRequestedWhileLoadingqueued reload pattern fromErrorRuleDetectortoRequestFilterEngine, closing the concurrent-reload race window.🤖 Generated with Claude Code
Greptile Summary
This PR fixes stale in-memory caches in
RequestFilterEngineandErrorRuleDetectorby: (1) porting theactiveReloadPromise+reloadRequestedWhileLoadingdo/while queued-reload pattern fromErrorRuleDetectortoRequestFilterEngine, and (2) adding explicitawait reload()calls in the create/update/delete server actions so the cache is synced before the response is returned, eliminating the "must click Refresh Cache" workaround. UI components for error-rules also receiverouter.refresh()calls on mutation success to align them with the existing behavior in request-filter components.request-filter-engine.ts): replaces the silent-discardif (isLoading) returnwith a do/while queued-reload loop identical toErrorRuleDetector's;ensureInitialized()gains anactiveReloadPromisefast-path for cold-start deduplication.request-filters.ts,error-rules.ts): all create/update/delete actions nowawait reload()after the DB write;ErrorRuleDetector.reload()gains aqueueIfRunningoption so therefreshCacheActioncan force a full re-read while normal action calls reuse any in-flight promise without queuing a redundant second pass.add-rule-dialog,edit-rule-dialog,rule-list-table,refresh-cache-button): addsrouter.refresh()on success, mirroring the existing behavior in the request-filter equivalents.Confidence Score: 5/5
The change is safe to merge. Core reload logic is correct in all single-mutation and concurrent-mutation scenarios; cache integrity is maintained through the do/while queued-reload pattern, and action failures are handled gracefully without flipping mutation results.
The queued do/while pattern correctly handles both the common case (no pre-existing in-flight reload) and the concurrent case (pre-existing reload from a prior mutation) — the event handler's queue=true ensures a follow-up iteration that the awaited promise covers. The only notable gap is a test that uses the wrong calling convention for the action path, but this does not affect production behaviour. All mutation paths are guarded, the best-effort catch blocks prevent reload errors from poisoning successful writes, and the full test suite passes.
tests/unit/lib/request-filter-engine-reload-queue.test.ts — test #3 should model the action's reload(false) convention alongside the event handler's reload() call to accurately represent the concurrent-mutation scenario.
Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as UI Component participant Action as Server Action participant Repo as Repository Layer participant EE as EventEmitter participant Engine as RequestFilterEngine / ErrorRuleDetector participant DB as Database UI->>Action: createRequestFilterAction(data) Action->>Repo: createRequestFilter(data) Repo->>DB: INSERT DB-->>Repo: created record Repo->>EE: emit("requestFiltersUpdated") [sync] EE->>Engine: "void reload() [queue=true, fire-and-forget]" Note over Engine: activeReloadPromise = new do/while loop Repo-->>Action: created record Action->>Engine: await reload(false) Note over Engine: activeReloadPromise already set →<br/>return existing promise (no second round queued) Engine->>DB: SELECT active filters DB-->>Engine: [fresh rows including new record] Engine-->>Engine: "loadFilters() → isInitialized = true" Engine-->>Action: promise resolves (cache is current) Action->>UI: "{ ok: true }" UI->>UI: router.refresh() Note over Engine,DB: Concurrent mutation scenario (pre-existing in-flight reload) Engine->>DB: "SELECT (reload #prev, started before write)" Action->>Engine: "[event fires] void reload() [queue=true]" Note over Engine: reloadRequestedWhileLoading = true Action->>Engine: await reload(false) → returns promisePrev DB-->>Engine: [stale snapshot] Engine->>DB: SELECT again (second do/while iteration) DB-->>Engine: [fresh rows] Engine-->>Action: promisePrev resolves (both iterations done)Comments Outside Diff (1)
src/lib/request-filter-engine.ts, line 470-492 (link)applyGlobal/applyForProvidercan bypass the new reload-waitBoth
applyGlobalandapplyForProviderhave an early returnif (this.isInitialized && this.<filters>.length === 0) return;that fires beforeensureInitialized()is called. When a user adds the very first global guard filter, the engine is initialized with an empty list. A concurrent proxy request entering this fast path will return immediately — even ifactiveReloadPromiseis set for a reload that would load the new filter. The newensureInitialized()guard added in this PR only helps when there are already filters in the bucket. This is a pre-existing design trade-off, but it limits the correctness guarantee the PR aims to provide for the "add first rule" case.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "fix(cache): tolerate reload failures and..." | Re-trigger Greptile