Skip to content

fix(rules): make list order the single source of priority#12

Merged
jackerjay merged 3 commits into
mainfrom
fix/list-order-priority
May 19, 2026
Merged

fix(rules): make list order the single source of priority#12
jackerjay merged 3 commits into
mainfrom
fix/list-order-priority

Conversation

@jackerjay
Copy link
Copy Markdown
Owner

Why

LinkPilot was carrying two competing notions of "priority":

  1. A numeric Rule.priority: i32 field.
  2. The position of each rule inside config.rules.

They drifted apart in practice — the GUI editor stamped 100 on every new rule, the CLI defaulted to 10, the demo config used 10, and drag-to-reorder restamped (N - idx) * 10. So a user with a source-app = Lark → Ask rule and a url-host = github.com → Open Chrome rule could easily end up with both at the same numeric priority. The stable sort then silently favored whichever rule was added first; the picker for Ask did pop but the user, expecting github.com → Chrome to dispatch directly, read it as "nothing happened — priority isn't working."

User report (translated): "当我的 rules 存在冲突的时候,例如我配置了 lark 打开使用 ask,同时配置了 github.com 默认走 Chrome,此时我在 lark 中点击 github 相关的域名不会有任何的浏览器跳转反应... 而且不能出现重复的优先级,应该按照列表排序的方式来规定优先级".

What

Collapse the two sources into one: list order in config.rules IS priority. Top of list wins. Duplicates are now structurally impossible.

Core (crates/core/)

  • Drop Rule.priority from the schema.
  • Router::evaluate_explained walks config.rules top-to-bottom and returns the first matching enabled rule — no sort.
  • One-shot migration in ConfigStore::load_or_init: if a pre-v0.2 config still has priority fields on rules, stable-sort by priority descending (preserving user intent), strip the field, and persist atomically. The fsnotify external-edit path runs the same migration so hand-edited legacy configs still work.

CLI (crates/cli/)

  • rules add drops --priority; new rules append at the bottom (safe default — won't silently override existing ones).
  • set-priority removed; replaced by rules move <id> <top|up|down|bottom>.
  • rules list renders 1-based slot numbers instead of a priority column.

Frontend (apps/desktop/src/)

  • RuleEditor removes the Priority input and explains that priority is now set by list order on the Rules page.
  • pages/rules.tsx drops the sort + restamp; drag-reorder is a pure list rewrite. The leading #N badge is now the rule's 1-based global slot.
  • pages/inspector.tsx, pages/test-url.tsx, pages/workspace.tsx: same — show the slot, not a numeric priority.
  • OnboardingFlow prepends toggled templates in their declared order so the first template lands at slot chore(deps): bump softprops/action-gh-release from 2 to 3 #1.

DSL (packages/config-dsl/)

  • .priority(p) builder removed; RuleJson.priority removed from the wire shape.
  • compile() documents that cfg.rules order is authoritative.

Tests

  • New regression routing::tests::first_match_in_list_order_wins — reproduces the user's exact scenario (Lark/Ask + github/Chrome) and asserts that swapping the list positions flips the winning decision.
  • New migration test config::store::tests::migrates_legacy_priority_to_list_order — writes a legacy JSON with "priority": 10 and "priority": 100 rules in the wrong order, loads it, verifies the prio-100 rule moves to slot 1 and that the priority key is gone from disk after persistence.
  • New DSL test rule array order is preserved (list-order priority) plus an updated RouteBuilder modifiers stick that asserts no priority field is emitted.

Local verification

  • cargo test -p linkpilot-core — 32 passed (incl. the two new tests above)
  • cargo test -p linkpilot-cli — 7 passed
  • cargo check --workspace incl. linkpilot-desktop — clean
  • cargo clippy --workspace — no new warnings (only pre-existing tray.rs needless_borrows)
  • npm run build in apps/desktop (tsc + vite) — clean
  • bun test in packages/config-dsl — 13 passed
  • npm run bundle:mac — produces LinkPilot.app + LinkPilot_0.2.0_aarch64.dmg; Info.plist patch applies cleanly

Backwards compatibility

The Rule struct doesn't use serde(deny_unknown_fields), so existing configs that still carry priority deserialize without error. The migration runs on the next load — sorts by priority desc (stable), drops the field, persists once. Idempotent: subsequent loads see no priority and skip migration. User-authored priority intent is preserved.

Test plan

  • First launch on an existing install: confirm linkpilot.config.json no longer contains "priority": after the daemon boots once.
  • Repro user's case: configure source-app = Lark → Ask and url-host = github.com → Open Chrome; drag the github rule to the top in the Rules page; click a github link inside Lark → opens directly in Chrome with no picker.
  • Drag the lark rule above github → same click → picker appears.
  • lpt rules list prints rules in list order with #N slots.
  • lpt rules move <github-id> top raises that rule to slot 1.
  • Onboarding: toggle two templates → both land at the top in declared order.

🤖 Generated with Claude Code

jackerjay added 3 commits May 19, 2026 21:01
Two priority sources used to coexist: a numeric `Rule.priority: i32`
field and the position of each rule in `config.rules`. They drifted
in practice — the GUI editor stamped 100 on every new rule, the CLI
defaulted to 10, the demo used 10, drag-reorder restamped (N-idx)*10.
A user with `lark→ask` + `github.com→Chrome` both at priority 100
hit a stable-sort tie that silently favored whichever rule was added
first, masking what felt like a working list order.

Collapse the two sources into one: list order in `config.rules` IS
priority, top of list wins. Duplicates are now structurally
impossible.

- core/rules: drop `Rule.priority`
- core/routing: walk rules in declared order, return the first
  matching enabled rule (no sort) — plus a regression test that
  proves swapping list positions flips the winner
- core/config/store: one-shot migration on load — if any rule on
  disk still has a legacy `priority` field, sort the array by it
  desc (stable) and strip the field, then persist
- cli: drop `--priority` from `rules add`, replace `set-priority`
  with `rules move <id> <top|up|down|bottom>`, render `rules list`
  with 1-based slot numbers instead of a priority column
- frontend: drop the Priority input from the rule editor, show the
  rule's slot (`#N`) instead of its old numeric priority, simplify
  drag-reorder to a pure list rewrite, and prepend onboarding
  templates so the first one ends up at slot #1
- config-dsl: drop the `.priority()` builder + the `priority` field
  from `RuleJson`; document that array order is authoritative
When a rule targets a browser that's missing or mistyped, the launcher
returns `PlatformError::Other("browser 'foo' is not installed on this
Mac")`. The dispatcher used to surface that as `LaunchOutcome::Failed`
and log it — from the user's perspective: click a link, nothing
happens.

Wrap the launcher call in `open_with_default_fallback`: on a failure
that names a browser id different from the config's `default_target`,
retry once with `default_target`. If primary and default share a
browser id (e.g. user broke their default too), skip the retry rather
than thrash the same missing binary. Both failure cases compose into
one error string so logs / Tauri callers still see what went wrong.

Applies to both the `Open` arm and the `Ask` arm (after the picker
resolves to a target), so a stale picker pick gets the same safety
net.

Adds four `dispatch::tests` cases against a scripted launcher: the
happy path, the fallback success path, the both-failed path, and the
"primary == default, no point retrying" path.
@jackerjay jackerjay merged commit 7c74696 into main May 19, 2026
3 checks passed
@jackerjay jackerjay deleted the fix/list-order-priority branch May 19, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant