diff --git a/frontend/src/pages/xray/routing/RoutingTab.tsx b/frontend/src/pages/xray/routing/RoutingTab.tsx index 1b54a6213..95cc1d165 100644 --- a/frontend/src/pages/xray/routing/RoutingTab.tsx +++ b/frontend/src/pages/xray/routing/RoutingTab.tsx @@ -21,7 +21,7 @@ import RuleFormModal from './RuleFormModal'; import type { RoutingRule } from './RuleFormModal'; import RuleCardList from './RuleCardList'; import { useRoutingColumns } from './useRoutingColumns'; -import { arrJoin, originalRuleIndex } from './helpers'; +import { arrJoin, originalRuleIndex, isApiRule } from './helpers'; import type { RuleRow } from './types'; import type { XraySettingsValue, SetTemplate } from '@/hooks/useXraySetting'; import type { RuleObject } from '@/schemas/routing'; @@ -209,8 +209,20 @@ export default function RoutingTab({ if (!tt.routing) tt.routing = { rules: [] }; if (!Array.isArray(tt.routing.rules)) tt.routing.rules = []; const typed = rule as unknown as RuleObject; - if (editingIndex == null) tt.routing.rules.push(typed); - else tt.routing.rules[editingIndex] = typed; + if (editingIndex == null) { + // Rules match top-to-bottom, first hit wins, so a brand-new rule + // appended at the end is silently shadowed by any earlier + // broader/catch-all rule that also happens to match its traffic — + // a real trap: the rule looks saved and enabled, but never actually + // fires. Insert it as early as possible instead (right after the + // pinned api rule, if present) so a newly created rule takes effect + // by default; the admin can still drag it lower with moveDown if + // that's genuinely what they want. + const insertAt = isApiRule(tt.routing.rules[0] as RuleObject) ? 1 : 0; + tt.routing.rules.splice(insertAt, 0, typed); + } else { + tt.routing.rules[editingIndex] = typed; + } }); setRuleModalOpen(false); } diff --git a/frontend/src/test/routing-new-rule-insert-position.test.tsx b/frontend/src/test/routing-new-rule-insert-position.test.tsx new file mode 100644 index 000000000..609dc5813 --- /dev/null +++ b/frontend/src/test/routing-new-rule-insert-position.test.tsx @@ -0,0 +1,62 @@ +import { describe, it, expect, vi } from 'vitest'; +import { fireEvent, screen } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +import RoutingTab from '@/pages/xray/routing/RoutingTab'; +import type { XraySettingsValue } from '@/hooks/useXraySetting'; + +import { renderWithProviders } from './test-utils'; + +function settingsWithApiAndBlockRules(): XraySettingsValue { + return { + routing: { + rules: [ + { type: 'field', inboundTag: ['api'], outboundTag: 'api', enabled: true }, + { type: 'field', ip: ['ext:geoip_RU.dat:ru'], outboundTag: 'blocked', enabled: true }, + { type: 'field', protocol: ['bittorrent'], outboundTag: 'blocked', enabled: true }, + ], + }, + } as unknown as XraySettingsValue; +} + +// Rules match top-to-bottom, first hit wins. A brand-new rule used to be +// appended at the end, where a pre-existing broader/catch-all rule (e.g. a +// block rule with no inboundTag restriction, like the ones here) silently +// shadows it forever -- the rule looks saved and enabled but never actually +// fires. See RoutingTab.tsx onRuleConfirm. +describe('RoutingTab new-rule insert position', () => { + it('inserts a newly created rule right after the pinned api rule, not at the end', () => { + const setTemplateSettings = vi.fn(); + const initial = settingsWithApiAndBlockRules(); + + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + renderWithProviders( + + + , + ); + + fireEvent.click(screen.getByRole('tab', { name: /Routing Rules/ })); + fireEvent.click(screen.getByRole('button', { name: /Routing Rules/ })); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + + expect(setTemplateSettings).toHaveBeenCalledTimes(1); + const updater = setTemplateSettings.mock.calls[0][0] as (prev: XraySettingsValue) => XraySettingsValue; + const next = updater(initial); + const rules = (next.routing as { rules: Array<{ inboundTag?: string[]; outboundTag?: string }> }).rules; + + expect(rules.length).toBe(4); + expect(rules[0].outboundTag).toBe('api'); + // The two pre-existing block rules must have been pushed down, not the + // new rule appended after them. + expect(rules[1].outboundTag).not.toBe('blocked'); + expect(rules[2].outboundTag).toBe('blocked'); + expect(rules[3].outboundTag).toBe('blocked'); + }); +});