Fix ip rule/tunnel/xfrm/monitor completions: model bare keywords as subcommands - #311
Conversation
#310 (merged as 6e46d29) added rich rule/tunnel/xfrm/monitor completions for ip, but modeled every bare (non-dash) iproute2 keyword -- iif, oif, dev, mode, table, from, to, src, dst, proto, spi, enc, and ~90 others -- as an "options" entry. Warp's completer only ever treats a token as an option/flag when it starts with a '-' (crates/warp_completer/src/completer/engine/flag/v2.rs: short_hand_flag_suggestions bails to iter::empty() for any non-dash, non-empty token; crates/warp_completer/src/parsers/v2.rs: get_flag_signature_spec bails out immediately unless the token starts with '-'). Since none of these keywords have a leading dash, none of them were ever reachable: the completions dropdown never appeared and the attached generators never fired. Confirmed live: built and ran a local Warp client against unmodified main and against this fix. On main, `ip rule add iif <Tab>` and `ip tunnel add dev <Tab>` produce nothing. With this fix, both show a real dropdown of network interfaces via the existing network_interfaces generator. Fix: every bare-keyword entry is now modeled as a "subcommands" entry instead of "options", with its value carried on that entry's own positional "args" (generators/suggestions preserved as-is). This matches how Warp's resolver actually walks tokens (deepest_matching_subcommand_signature in crates/warp_completer/src/signatures/v2/lookup.rs matches subcommand names by exact string, independent of any dash). No vocabulary was removed or regressed; only the "options" -> "subcommands" modeling changed. ip.rs is untouched -- the existing network_interfaces generator did not need any changes. Also fixes two remaining gaps found in the merged spec: - ip xfrm state add|update encap now takes its full grammar (ENCAP-TYPE SPORT DPORT OADDR) instead of just the encap type. - ip tunnel prl/6rd's dev argument now uses the network_interfaces generator (it previously had no generator attached). Known, engine-level limitation, not fixable generically here: because iproute2 lets these keywords appear in any order and combination, and Warp only carries forward one resolved command node's own arguments once a subcommand match is picked, only the first bare keyword typed after a verb reliably gets a completions dropdown. A second chained keyword (e.g. "iif eth0 from ...") does not itself complete, though the resulting command is still valid to type manually. Supersedes #309, which predates this merge and no longer applies cleanly. Co-Authored-By: Warp Agent <agent@warp.dev>
Warp's completer never suggests a bare (non-dash) token modeled as an 'options' entry - short_hand_flag_suggestions() in crates/warp_completer/src/completer/engine/flag/v2.rs requires a leading '-'. Every bare iproute2 keyword under 'ip address' (peer/broadcast/scope/dev/label/... for add/change/replace/del, and to/scope/up/down/... for show/flush/save) must be a 'subcommands' entry instead, carrying its value on that entry's own positional args. Matches the structure landed in #311 for rule/tunnel/xfrm/monitor. Co-Authored-By: Warp Agent <agent@warp.dev>
Warp's completer only ever suggests dash-prefixed strings for `options` entries (short_hand_flag_suggestions() requires a leading '-'), so bare keywords like 'target-nsid' and 'nsid' were completely unreachable. Subcommand matching has no dash requirement, so these now live under 'subcommands' instead, matching the same fix applied to ip rule/tunnel/xfrm/monitor in #311. Co-Authored-By: Warp Agent <agent@warp.dev>
Bare (non-dash) keywords are unreachable in Warp's completer when modeled as 'options' entries: short_hand_flag_suggestions() in crates/warp_completer/src/completer/engine/flag/v2.rs only ever suggests names starting with '-', so a bare option name and any generator attached to it never surface in the dropdown. Every keyword under 'ip neighbour' (lladdr, nud, proxy, router, use, managed, extern_learn, extern_valid, dev, protocol, to, master, vrf, nomaster, unused) is bare, so every 'options' array in this subtree becomes 'subcommands', matching the fix applied to ip rule/tunnel/xfrm in #311. The vocabulary and descriptions are unchanged; only the container key changes. Co-Authored-By: Warp Agent <agent@warp.dev>
There was a problem hiding this comment.
Overview
Remodels ~90 bare iproute2 keywords from options to subcommands so they are reachable at all, which is correct and verified. Posting one decision for a human rather than a verdict, because merging this accepts a completion-engine limitation the spec cannot fix.
Concerns
- The fix makes the first bare keyword after a verb complete, but not subsequent ones: once a keyword resolves to a subcommand, lookup carries only that child's positional args and its siblings stop being candidates, so
ip rule add iif eth0 from <Tab>still offers nothing. This is strictly better than the previous model, which surfaced none of these keywords or their generators, and it does not affect whether a manually typed command is valid — but it means arbitrary-order keyword construction stays incomplete. The JSON shape cannot solve it; a real fix would retain the parent's sibling subcommands after resolving a keyword, indeepest_matching_subcommand_signature(crates/warp_completer/src/signatures/v2/lookup.rsinwarpdotdev/warp). - The decision matters beyond this PR: five sibling specs for
ip netns,neigh,route,link, andaddrare built on this same pattern. Either accept the limitation for all six and avoid claiming arbitrary-order keyword completion, or fix the engine first.
Verdict
Checks: build pass, tests pass (133 + 10), CI green at 9c67ddb, visual proof missing — session captures are being attached to the description separately.
Found: 0 critical, 0 important, 0 suggestions, 0 nits
Independently verified and not repeated as findings: the remodel is vocabulary-neutral (307 named nodes before and after, zero added, zero removed, with descriptions, argument shapes, and generator bindings preserved); the earlier corrections to ip rule save, ip tunnel prl, and 6rd all survived; and the two substantive additions — the ENCAP-TYPE SPORT DPORT OADDR grammar on xfrm state add|update encap, and network_interfaces on PRL/6rd dev — both match current upstream.
Responding as wilson: Open session · View factory task
* Add completions: ip neighbour (neigh) Models add/change/replace/del(ete), get, show/list/lst, flush, and help for `ip neighbour` (and its `neigh` abbreviation), grounded in upstream ip/ipneigh.c and ip-neighbour(8): - add/change/replace/del share ipneigh_modify's option set (lladdr, nud, proxy, router, use, managed, extern_learn, extern_valid, dev, protocol) with a positional ADDRESS argument; del keeps lladdr/nud but notes they are ignored, matching the man page. - show/list/lst and flush share do_show_or_flush's filter set (to, dev, master, vrf, nomaster, proxy, unused, nud, protocol), modeled separately from add/change/replace since the filter and modify option sets differ (e.g. nud/protocol accept an 'all' filter value only here). - get takes ADDRESS/dev/proxy per ipneigh_get. - managed/use are tagged iproute2 5.16+ and extern_valid iproute2 6.17+ per their upstream commit history. - dev reuses the network_interfaces generator from #310 since it accepts any interface; vrf is left without a generator since it only accepts VRF devices and no narrower generator exists. Ref: warpdotdev/warp#9766 (APP-3981) Co-Authored-By: Warp Agent <agent@warp.dev> * Model bare neighbour keywords as subcommands, not options Bare (non-dash) keywords are unreachable in Warp's completer when modeled as 'options' entries: short_hand_flag_suggestions() in crates/warp_completer/src/completer/engine/flag/v2.rs only ever suggests names starting with '-', so a bare option name and any generator attached to it never surface in the dropdown. Every keyword under 'ip neighbour' (lladdr, nud, proxy, router, use, managed, extern_learn, extern_valid, dev, protocol, to, master, vrf, nomaster, unused) is bare, so every 'options' array in this subtree becomes 'subcommands', matching the fix applied to ip rule/tunnel/xfrm in #311. The vocabulary and descriptions are unchanged; only the container key changes. Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
Add nested completions for `ip route`, covering the actions (add, change/chg, replace, prepend, append, del/delete, show/list/lst, flush, get, save, restore, showdump, help) and their attribute surface (via, dev/oif, src, metric, table, scope, proto, mtu, advmss, nexthop, onlink, route types, lockable metrics, etc.), modeled against upstream iproute2 ip/iproute.c and man/man8/ip-route.8.in. Bare (non-dash) keywords are modeled as `subcommands`, not `options`, per the fix landing in #311 -- Warp's completer only suggests dash-prefixed tokens from `options` arrays, so a bare keyword there is unreachable in the dropdown. show/flush/save share one option set (the SELECTOR filter grammar); add/change/replace/prepend/append/del share a different, larger set (the route attribute grammar); get has its own distinct set. These three grammars are verified as genuinely different parsers in upstream source, not copy-paste of one onto the others. dev/oif reuse the network_interfaces generator from #310 since they accept any interface. vrf and other narrower positions are left without a generator rather than reusing network_interfaces incorrectly. Co-Authored-By: Warp Agent <agent@warp.dev>
* Add nested completions for ip netns Covers the full ip/ipnetns.c action set: list/show/lst, add, attach, set, delete/del, identify, pids, exec, monitor, and list-id, plus help. Existing namespace-name positions reuse the existing "netns" generator (ip netns list); a new "processes" generator (ps -eo pid,comm) backs the PID positions in attach/identify, since those accept any running process, not just namespace-linked ones. New-name positions (add, attach's NAME) are left without a generator. Verified against iproute2's ip/ipnetns.c and man/man8/ip-netns.8.in (iproute2/iproute2 main branch). Co-Authored-By: Warp Agent <agent@warp.dev> * Fix ip netns list-id: bare keywords must be subcommands, not options Warp's completer only ever suggests dash-prefixed strings for `options` entries (short_hand_flag_suggestions() requires a leading '-'), so bare keywords like 'target-nsid' and 'nsid' were completely unreachable. Subcommand matching has no dash requirement, so these now live under 'subcommands' instead, matching the same fix applied to ip rule/tunnel/xfrm/monitor in #311. Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
* Add completions: ip address (addr, a) Models the ip address subcommand tree from upstream ip/ipaddress.c: add/change(chg)/replace/del(delete) address attributes (peer, broadcast/ brd, anycast, scope, dev, label, metric/priority/preference, valid_lft, preferred_lft, and the home/nodad/optimistic/mngtmpaddr/noprefixroute/ autojoin flags, proto), plus the separate show(list,lst)/flush/save filter branch (to, scope, up/down, label, the read-only address state flags, group/master/vrf/nomaster/type/proto/novf) and the argument-less showdump/restore/help leaves. Reuses the network_interfaces generator from #310 for the genuinely any-interface dev positions; master/vrf are left without a generator since they only accept a narrower set of devices. Refs warpdotdev/warp#9769 (APP-3978). Stacked on factory/ip-rule-tunnel-xfrm-monitor (#310, already merged into main). Co-Authored-By: Warp Agent <agent@warp.dev> * Fix ip address completions: model bare keywords as subcommands Warp's completer never suggests a bare (non-dash) token modeled as an 'options' entry - short_hand_flag_suggestions() in crates/warp_completer/src/completer/engine/flag/v2.rs requires a leading '-'. Every bare iproute2 keyword under 'ip address' (peer/broadcast/scope/dev/label/... for add/change/replace/del, and to/scope/up/down/... for show/flush/save) must be a 'subcommands' entry instead, carrying its value on that entry's own positional args. Matches the structure landed in #311 for rule/tunnel/xfrm/monitor. Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
… l2tp, macsec, maddress, mptcp, mroute, mrule, netconf, nexthop, ntable, sr, tap/tuntap, tcpmetrics, token, vrf) Completes the `ip` completion work started in #310 (warpdotdev/warp#9764) and the #9765-#9769 series, by filling in every remaining bare-leaf `ip` subcommand called out in warpdotdev/warp#15047. - Models every bare (non-dash) iproute2 keyword as a `subcommands` entry with the value on that entry's own `args`, per the fix established in the in-flight #311 (`ip rule`/`tunnel`/ `xfrm`/`monitor` completions were unreachable because Warp's completer only treats `-`-prefixed tokens as flag candidates). - Reuses the `network_interfaces` generator only where a position genuinely accepts any local interface. - Adds three new, narrower generators (`vrf_interfaces`, `macsec_interfaces`, `tuntap_interfaces`) that reuse the already-tested `parse_interfaces` parser against `ip -o link show type <kind>`, for positions that must be an existing device of that specific kind. - Merges `tap` into `tuntap` as an alias, since they are the same command upstream. Co-Authored-By: Warp Agent <agent@warp.dev>
…erators - Rebase onto current main to pick up #311's fix (bare iproute2 keywords modeled as `subcommands`, not `options`) instead of carrying the pre-#311 shape back into `ip rule`/`tunnel`/`xfrm`/`monitor`. Verified no dashless keyword remains under `options` anywhere in the file; the only remaining `options` entries are legitimate dash-prefixed flags. - Model `tcp_metrics` as an alias of `tcpmetrics` (upstream `ip/ip.c` dispatches both to `do_tcp_metrics`), matching the existing tap/tuntap alias convention. - Add targeted unit tests for the three new generators (vrf_interfaces, macsec_interfaces, tuntap_interfaces): each asserts the exact type-filtered shell command it runs, and that its output is parsed correctly through the shared `parse_interfaces` function using representative filtered `ip -o link show type <kind>` output. Co-Authored-By: Warp Agent <agent@warp.dev>
* Add completions: ip route Add nested completions for `ip route`, covering the actions (add, change/chg, replace, prepend, append, del/delete, show/list/lst, flush, get, save, restore, showdump, help) and their attribute surface (via, dev/oif, src, metric, table, scope, proto, mtu, advmss, nexthop, onlink, route types, lockable metrics, etc.), modeled against upstream iproute2 ip/iproute.c and man/man8/ip-route.8.in. Bare (non-dash) keywords are modeled as `subcommands`, not `options`, per the fix landing in #311 -- Warp's completer only suggests dash-prefixed tokens from `options` arrays, so a bare keyword there is unreachable in the dropdown. show/flush/save share one option set (the SELECTOR filter grammar); add/change/replace/prepend/append/del share a different, larger set (the route attribute grammar); get has its own distinct set. These three grammars are verified as genuinely different parsers in upstream source, not copy-paste of one onto the others. dev/oif reuse the network_interfaces generator from #310 since they accept any interface. vrf and other narrower positions are left without a generator rather than reusing network_interfaces incorrectly. Co-Authored-By: Warp Agent <agent@warp.dev> * Fix nexthop grammar and add missing ip route test action Reviewer found two gaps: 1. 'nexthop' was modeled as an opaque NH positional, so nothing completed after it. Model it as nested subcommands (via [FAMILY] ADDRESS, dev STRING, weight NUMBER, onlink, realms REALM, encap ENCAPTYPE, as [to] ADDRESS), matching parse_one_nh() in ip/iproute.c. Applied across all seven modify-grammar action blocks (add, change, replace, prepend, append, del, test). The inner 'dev' stays unbound from network_interfaces, as intended. 2. 'ip route test' was missing. It is undocumented in usage()/the man page, but do_iproute() dispatches it straight into the same iproute_modify() parser as add/change/replace/prepend/append/del, so it gets the identical grammar. Co-Authored-By: Warp Agent <agent@warp.dev> * Mark 'features' isVariadic on all route-modifying actions Upstream's parse_features() (ip/iproute.c) consumes a space-separated sequence of feature tokens, e.g. 'features ecn tcp_usec_ts', not a single scalar value. Marks the FEATURES argument isVariadic across add/change/replace/prepend/append/del/test so completions keep offering the remaining feature name(s) after the first is typed. Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
* Add completions for remaining ip subcommands Model the 19 previously-bare-leaf ip subcommands as full completion subtrees: addrlabel, fou, ila, ioam, l2tp, macsec, maddress, mptcp, mroute, mrule, netconf, nexthop, ntable, sr, tuntap (with tap merged in as an alias), tcpmetrics (with tcp_metrics as an alias), token, and vrf. Follows the corrected modeling shape established in #311: bare (non-dash) iproute2 keywords are modeled as 'subcommands' entries, not 'options', since Warp's completer only treats dash-prefixed tokens as options. The network_interfaces generator is reused only for positions that genuinely accept any local interface; narrower device positions (e.g. macsec/vrf/tuntap device names) are left without a generator. Verified against upstream iproute2 source under ip/ (ipaddrlabel.c, ipfou.c, ipila.c, ipioam6.c, ipl2tp.c, ipmacsec.c, ipmaddr.c, ipmptcp.c, ipmroute.c, iprule.c (do_multirule), ipnetconf.c, ipnexthop.c, ipntable.c, ipseg6.c, iptuntap.c, tcp_metrics.c, iptoken.c, ipvrf.c). Fixes warpdotdev/warp#15047 Co-Authored-By: Warp Agent <agent@warp.dev> * Fix review findings: ioam del alias, mrule parity, nexthop master generator - ip ioam namespace/schema: remove the invented 'delete' alias; upstream ipioam6.c only accepts the literal 'del' keyword (strcmp, not a matches()-style prefix check). - ip mrule: bring add/delete/show/flush to full grammar parity with ip rule, since do_multirule (iprule.c) only changes preferred_family before delegating to do_iprule. Adds dscp, ipproto, sport, dport, tun_id, flowlabel, nat/map-to, realms, suppress_prefixlength, and suppress_ifgroup that were previously missing. - ip nexthop: add the network_interfaces generator to 'master DEV' under list/flush/bucket-list (ipnexthop.c calls ll_name_to_index with no type restriction there, unlike vrf), and add the missing 'vrf NAME' filter under 'nexthop bucket list'. - ip mptcp: correct the laminar endpoint flag's version annotation to iproute2 6.19+ (commit 2ee7ec0d), not 6.13+. Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
|
/oz-review |
Part 1 - defects confirmed live on main: - ip ntable: add missing 'ntbl' alias (upstream ip/ip.c maps both ntable and ntbl to do_ipntable; lookup is exact so ntbl reached nothing). Review flagged on #321. - ip tcpmetrics: add missing 'addr' alias to the address selector on show, delete, and flush (tcp_metrics.c accepts addr and address interchangeably). Review flagged on #323. - ip tuntap delete: remove user/group (do_del() passes null user/gid pointers to parse_args, which silently skips those branches) and add pi/one_queue/vnet_hdr/multi_queue, which parse_args does accept on delete. Review flagged on #313 and #321. - ip tunnel change: add seq/iseq/oseq/csum/icsum/ocsum, mirroring 'tunnel add' (iptunnel.c's parse_args accepts these for both add and change). Review flagged on #310/#311. Part 2 - salvaged from PR #322 (factory/ip-remaining-subcommands-v2), verified against upstream before porting: - ip nexthop add|replace group: nest the resilient-group grammar (type with buckets/idle_timer/unbalanced_timer, plus fdb) under 'group', matching 'ip nexthop add id N group G type resilient buckets B' from ip-nexthop(8) and ipnexthop.c. Left the existing top-level type/hw_stats siblings untouched. - ip mrule save / ip mrule restore: ip/iprule.c's do_iprule (compiled for RTNL_FAMILY_IPMR to back ip mrule) supports flush/save/restore identically to ip rule. - ip macsec add|set rx sci: nest on/off/sa (with the same pn/xpn/ salt/ssci/key/on/off options already modeled on the sibling top-level sa) under sci, matching 'ip macsec add DEV rx SCI sa {0..3} ...' from ip-macsec(8). ip macsec del rx sci: nest sa (AN only). Left the existing port/address/sa siblings untouched. Rejected from #322: the macsec offload value suggestions (mac/off/phy) are already present on main via #321's args array, so nothing to port there. Co-Authored-By: Warp Agent <agent@warp.dev>
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR updates command-signatures/json/ip.json so bare iproute2 keywords under ip rule, ip tunnel, ip xfrm, and ip monitor are modeled as subcommands rather than dash-style options. It also preserves the monitor object arguments, expands xfrm state add|update encap positional arguments, and adds interface generators to the tunnel prl/6rd dev arguments.
Concerns
- No blocking correctness, security, comment-quality, test-quality, or spec-alignment concerns found in the annotated diff. The known chained-keyword completion limitation is disclosed in the PR description and appears to be an existing engine limitation rather than a regression introduced by this data-only fix.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
* Add completions: ip link Model ip link's actions and common device attributes for completions: add/delete/set/change/replace/show/xstats/afstats/property/help, plus the common ip link set attributes (up/down, mtu, name, address, arp, multicast, promisc, allmulticast, txqueuelen, netns, alias, master/nomaster, vrf, group, VF sub-options) and the type name list. Every bare (non-dash) keyword is modeled as a 'subcommands' entry with its value carried on that entry's own positional args, per #311's fix for the same class of defect in rule/tunnel/xfrm/monitor -- Warp's completer never suggests a bare keyword modeled as an 'options' entry. dev/link positions reuse the existing network_interfaces generator; netns/link-netns reuse the existing netns generator. No new generators were needed. Co-Authored-By: Warp Agent <agent@warp.dev> * Address review: creation-time GSO/GRO, xdp variant grammar, down version note - ip link add/replace: add the five gso_*/gro_* creation attributes that were already modeled under set/change but missing from the creation trees, per iplink_parse()/the man page accepting them on both paths. - ip link set/change: xdpgeneric/xdpdrv/xdpoffload now nest the same object [section|program NAME] [verbose] / pinned FILE subtree as xdp, since the parser accepts identical grammar on all four and the first-keyword-only completer limitation does not apply within a single resolved node. - ip link show: annotate the 'down' filter with '(iproute2 6.13+)' (confirmed absent in v6.12.0's ip/ipaddress.c, present in v6.13.0's); left set/change's longstanding 'down' entry unchanged. Co-Authored-By: Warp Agent <agent@warp.dev> * Transplant verified items from competing PR: del/qlen aliases, batadv/vti6, inet grammar - 'delete' now also matches 'del' (do_iplink's matches() performs prefix comparison against "delete", and "del" is the idiomatic abbreviation; confirmed via lib/utils.c's matches()). - txqueuelen/txqlen gained the 'qlen' alias, confirmed via iplink.c's three-way check (matches(txqueuelen) || strcmp(qlen) || matches(txqlen)). - Added 'batadv' (iplink_batadv.c) and 'vti6' (link_vti6.c) to every type suggestion list; both are real, dlopen-registered link kinds missing from the man page's TYPE/ETYPE grammar, same class of gap as team/wwan already modeled. - Added the 'inet [...]' IPv4 devconf grammar under set/change as a nested 'inet' subcommand with all 32 sysctl-equivalent sub-keywords from iplink_parse_inet(), reversing the earlier decision to omit it. Co-Authored-By: Warp Agent <agent@warp.dev> * Address review bot findings: narrow xstats type list, add show dev keyword - xstats' type suggestions were the full link-type list, but iplink_ifla_xstats() rejects any type without a print_ifla_xstats callback. Checked every iplink_<type>.c/link_<type>.c upstream: only bond, bond_slave, bridge, and bridge_slave register one. Narrowed the list to those four (previously missing bridge_slave entirely). - ip link show accepts an explicit 'dev DEVICE' keyword form (the shared ipaddr_list_flush_or_save() parser's final branch), not just the bare positional. Added it as its own keyword subcommand with the network_interfaces generator, alongside the existing positional. Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
## Description Updates `warp-command-signatures` from `fe352669` to `77c4a9a7`. This is the bump that actually delivers the `ip` completions work to users. Before it, every `ip` subcommand in the pinned spec was a bare leaf with no nested actions, options, or argument completions; after it, all 29 are filled in. ### Merged PRs - Add git read-tree completions (warpdotdev/command-signatures#296) - Add completions: ip rule, tunnel, xfrm, monitor (warpdotdev/command-signatures#310) - Fix ip rule/tunnel/xfrm/monitor completions: model bare keywords as subcommands (warpdotdev/command-signatures#311) - Add completions: ip neighbour (neigh) (warpdotdev/command-signatures#312) - Add completions: ip netns (warpdotdev/command-signatures#313) - Add completion spec: ip address (addr, a) (warpdotdev/command-signatures#315) - Add completion spec: ip route (warpdotdev/command-signatures#317) - Add completions: ip link (warpdotdev/command-signatures#318) - Add the neighbor (US spelling) alias to the ip neigh spec (warpdotdev/command-signatures#320) - Add completions for remaining ip subcommands (warpdotdev/command-signatures#321) - Fix ip neighbour completions: add 'neighbor' alias and full rt_protos vocabulary (warpdotdev/command-signatures#323) - Fix ip address completions: alias addr/a, local/dev keywords, nowhere scope, negated flags, accurate save (warpdotdev/command-signatures#324) - ip: fix macsec rx SCI chaining, ntable/tcp_metrics aliases, tuntap delete flags (warpdotdev/command-signatures#325) - ip tunnel: add interface generator to 'tunnel show NAME' (warpdotdev/command-signatures#326) - ip: add remaining nexthop/mrule/tunnel gaps, fix tunnel-show generator regression from #326 (warpdotdev/command-signatures#327) ### Worth knowing for review Two behaviors of this client's completer shaped the spec, and are worth being aware of when reading it: - Bare non-dash keywords must be modeled as `subcommands`, not `options`. `short_hand_flag_suggestions()` in `crates/warp_completer/src/completer/engine/flag/v2.rs` returns empty for any non-dash token, and only emits names satisfying `is_short_hand_flag_name()` or `is_long_hand_flag_name()`, both of which require a leading `-`. iproute2's grammar is almost entirely bare keywords, so an early version of this work was silently invisible until #311 remodeled it. - Only the first bare keyword after a verb completes. `deepest_matching_subcommand_signature()` in `crates/warp_completer/src/signatures/v2/lookup.rs` returns the matched subcommand alone, so its siblings stop being candidates — `ip rule add iif eth0 from <Tab>` offers nothing. That is an engine limitation rather than a spec one, tracked separately in #15048. It is not a regression: before this work none of these keywords completed at all. ## Linked Issue Delivers #9764, #9765, #9766, #9767, #9768, #9769. Follow-ups tracked in #15046 (IPv6 tunnel modes) and #15048 (chained-keyword completion). - [x] The linked issue is labeled `ready-to-implement`. - [x] Where appropriate, screenshots or a short video of the implementation are included below. ## Testing - [ ] I have manually tested my changes locally with `./script/run` This change is a dependency rev bump: two lines in `Cargo.toml` and `Cargo.lock`, no source changes. `cargo metadata` resolves cleanly against the new rev. The completions themselves were verified end to end against a locally built client earlier in this work, by pointing `warp-command-signatures` at a local checkout and exercising the dropdown. That is what caught the modeling bug fixed by #311. Evidence, including the before/after comparison on one build with only the spec crate swapped, is attached to warpdotdev/command-signatures#311. Being explicit about the gap: I did not run a full local `./script/run` or `cargo clippy` for this bump. The sandbox this was prepared in has 3 GB of RAM and the OOM killer terminates rustc partway through compiling the `warp` crate. Since the diff is a dependency rev with no source changes, CI is the meaningful gate here — but a reviewer who wants a local sanity check should run one before merging. ### Screenshots / Videos Completions dropdown verification is on warpdotdev/command-signatures#311. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode CHANGELOG-IMPROVEMENT: Added completions for `ip` — `address`, `link`, `route`, `neighbour`, `netns`, `rule`, `tunnel`, `xfrm`, `monitor` and the remaining subcommands — plus `git read-tree`. CHANGELOG-BUG-FIX: Fixed `ip` completions not appearing for iproute2's bare keyword arguments, along with missing `ip addr`/`ip a`/`ip neighbor` aliases and several suggestions that iproute2 rejects. Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
## Description Updates `warp-command-signatures` from `77c4a9a7` to `32a7fd56`. Follow-on to #15101, which merged pinned at `77c4a9a7` — a rev that predates warpdotdev/command-signatures#330. Without this, #330's corrections sit in `main` but do not reach users until some later bump. ### Merged PRs - ip rule/mrule/nexthop: complete flush selector sets, numeric-only nexthop protocol filter (warpdotdev/command-signatures#330) ### What #330 corrects - `ip nexthop list|flush protocol` no longer suggests protocol *names*. The filter path uses `get_unsigned()` (`ip/ipnexthop.c:1234`), so only numeric IDs are accepted there; names work solely on `add`/`replace`, which use `rtnl_rtprot_a2n()` (`:1128`). The named suggestions were producing invalid commands. - `ip rule flush` and `ip mrule flush` gain the full selector set, matching what `show` already carried. - `ip rule save` and `ip mrule save` correctly take no selectors. `ip/iprule.c:754` guards this explicitly: `if (action == IPRULE_SAVE && argc > 0) { fprintf(stderr, "\"ip rule save\" does not take any arguments.\n"); return -1; }`. The guard is specific to `IPRULE_SAVE`; `FLUSH` genuinely does take the filters. ## Linked Issue Completes the `ip` completions work delivered by #15101 (#9764, #9765, #9766, #9767, #9768, #9769). - [x] The linked issue is labeled `ready-to-implement`. - [x] Where appropriate, screenshots or a short video of the implementation are included below. ## Testing - [ ] I have manually tested my changes locally with `./script/run` A dependency rev bump: two lines across `Cargo.toml` and `Cargo.lock`, no source changes. `cargo metadata` resolves cleanly against the new rev. The upstream claims above were verified against a fresh iproute2 checkout rather than its man pages, which diverge from the parser in this area. #330's own checks were green (7/7) before merge. Same caveat as #15101: I did not run a full local `./script/run` or `cargo clippy` for this, because the sandbox it was prepared in OOMs compiling the `warp` crate. With no source changes, CI is the meaningful gate. ### Screenshots / Videos Completions dropdown verification is on warpdotdev/command-signatures#311. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode CHANGELOG-BUG-FIX: Fixed `ip nexthop` filter completions suggesting protocol names that iproute2 rejects, and completed the `ip rule`/`ip mrule` flush selector sets. Co-authored-by: Oz <oz-agent@warp.dev> Co-authored-by: Warp Agent <agent@warp.dev>
Summary
#310 merged rich
ip rule/tunnel/xfrm/monitorcompletions tomain, but modeled every bare (non-dash) iproute2 keyword --iif,oif,dev,mode,table,from,to,src,dst,proto,spi,enc, and roughly 90 others -- as anoptionsentry. Warp's completer only ever treats a token as an option/flag when it starts with-, so none of these bare keywords were actually reachable: the completions dropdown never appeared and their attached generators (e.g.network_interfaces) never fired, regardless of the JSON shape.This PR fixes that by modeling every bare keyword as a
subcommandsentry instead, with the value carried on that entry's own positionalargs. No vocabulary from #310 was removed; only the modeling changed.References
warpdotdev/warp#9764andAPP-3983. Supersedes #309 (opened before #310 merged, no longer applies cleanly against the newmain).Root cause
Warp's completer (
crates/warp_completer/src/completer/engine/flag/v2.rs,crates/warp_completer/src/parsers/v2.rsinwarpdotdev/warp) requires a leading-before a token is even considered as a flag candidate:short_hand_flag_suggestionsreturnsiter::empty()for any non-empty token that does not start with-.get_flag_signature_specbails immediately with "It's not a flag, so don't bother with it" unless the token starts with-.Since iproute2's keyword syntax (
ip rule add iif eth0,ip tunnel add mode gre, etc.) has no dash, every one of theseoptionsentries in the merged spec was silently unreachable. This was confirmed live: on unmodifiedmain,ip rule add iif <Tab>produces no completions at all.Changes
json/ip.json: converted every bare-keywordoptionsentry (acrossip rule,ip tunnel,ip xfrm state/policy/monitor, andip monitor) into asubcommandsentry, matching how Warp's resolver (deepest_matching_subcommand_signatureincrates/warp_completer/src/signatures/v2/lookup.rs) actually walks tokens -- by exact string match, independent of any dash. Dash-prefixed top-level options (-family,-json, etc.) are untouched.ip xfrm state add|update encapnow takes its full grammar (ENCAP-TYPE SPORT DPORT OADDR) instead of just the encap type.ip tunnel prl/6rd'sdevargument now uses thenetwork_interfacesgenerator (previously had none attached).src/generators/ip.rs: unchanged -- the existingnetwork_interfacesgenerator from Add completions: ip rule, tunnel, xfrm, monitor #310 did not need any changes.Known, engine-level limitation
Because iproute2 lets these keywords appear in any order and combination, and Warp only carries forward one resolved command node's own arguments once a subcommand match is picked, only the first bare keyword typed after a verb reliably gets a completions dropdown. A second chained keyword (e.g.
ip rule add iif eth0 from <Tab>) does not itself show a dropdown, though the resulting command is still valid to type manually. This is a constraint of Warp's completion engine for non-dash keyword grammars generally, not specific to this spec.Verification
npm run format:check-- Prettier cleancargo fmt -p warp-command-signatures -p warp-completion-metadata --checkcargo clippy -p warp-command-signatures -p warp-completion-metadata --all-targets --all-features -- -D warnings-- no warningscargo test-- 133 + 10 tests passed, 0 failed (includes the invariant tests that every JSON spec deserializes and everygeneratorNameexists, plus the pre-existingnetwork_interfacesgenerator unit tests)mainand again on this branch:main(6e46d29):ip rule add iif <Tab>shows no completions dropdown -- confirms the bug.ip rule add iif <Tab>,ip tunnel add dev <Tab>,ip xfrm policy add dev <Tab>, andip monitor dev <Tab>all correctly show a dropdown of real network interfaces (lo, dummy0, eth0, teql0, tunl0, ip6tnl0) via thenetwork_interfacesgenerator.Captures from the implementation run (view links):
main, showing the bug: https://oz.staging.warp.dev/artifacts/019ff8bf-b87d-7945-b3c2-de8bda341403ip rule add iif+ Tab: https://oz.staging.warp.dev/artifacts/019ff8c2-b6bb-7b6d-95c5-e30dc9a73887ip tunnel add dev+ Tab: https://oz.staging.warp.dev/artifacts/019ff8c3-21da-7233-88d7-d3597c6991c5ip xfrm policy add dev+ Tab: https://oz.staging.warp.dev/artifacts/019ff8c3-7826-7474-a261-91794b4e3fa0ip monitor dev+ Tab: https://oz.staging.warp.dev/artifacts/019ff8c3-d76d-7363-9bf2-434b8299e1dcIndependent verification (separate run)
A second, independent run reproduced the before/after on one machine (8 vCPU Linux, Xvfb) against a single Warp build. Only the
warp-command-signaturescrate was swapped between the two passes --warpitself was untouched -- so before and after differ in exactly one variable. Pre-fix pass is #310 at3eac838; post-fix pass is this PR's merge commitac413a5. Relative to3eac838, onlyjson/ip.jsonchanged (+87 -71);src/generators/ip.rsis byte-identical, so the generator code is a constant across the comparison.Ground truth on the test machine, from
ip -o link show:lo,dummy0,eth0,teql0,tunl0,ip6tnl0,docker0.Before (pre-fix, #310 at
3eac838)ip rule add iif+ Tab -- offers a directory listing of the working directory (about.hbs,about.toml,agents/,AGENTS.md,app/,Cargo.lock), never an interface name. The generator does not fire.ip rule add+ Tab -- no menu at all, after two Tab presses. No text inserted, no cursor movement, no error.ip tunnel add mode+ Tab -- likewise no menu after two Tab presses.Controls (pre-fix pass) -- why this is the engine, not the spec
These are the checks that separate "the spec is wrong" from "this position never completes". Both were run in the same pre-fix session:
git checkout+ Tab lists real branches, so script-backed generators do run in this build. The failure above is not a broken generator or a sandbox artifact.ip -n+ Tab offers dash-prefixed global flags rather than namespace names, so the pre-existingnetnsgenerator -- which predates #310 -- fails in the same way at the same position shape. The problem is older than #310 and was not introduced by it.After (post-fix,
ac413a5)ip rule add iif+ Tab -- a 7-entry dropdown of interface names, each described by its operational state. Every name matchesip -o link showexactly, with none missing and none extra:lo(UNKNOWN),dummy0(DOWN),eth0(UP),teql0(DOWN),tunl0(DOWN),ip6tnl0(DOWN),docker0(DOWN). The@NONEparent suffix ontunl0andip6tnl0is stripped, as the generator intends.ip rule add+ Tab -- 30 selector and action keywords, alphabetically ordered, each with a description.ip tunnel add mode+ Tab -- 5 mode values (gre,ipip,isatap,sit,vti).ip monitor dev+ Tab andip rule add oif+ Tab both produce the same interface dropdown, so the generator is reachable from more than one position.The chained-keyword limitation, shown
The "Known, engine-level limitation" section above is real and reproduces.
ip rule add iif eth0 from+ Tab offers working-directory files and folders (about.hbsFile,agents/Directory, ...) rather than address prefixes -- the sibling keyword vocabulary is gone onceiifhas resolved.ip rule add iif eth0+ Tab (no third keyword typed) opens no menu at all.Cosmetic observations, not blockers
Noted while capturing; none affect reachability, and all are visible in the screenshots in the collapsed section below:
ip xfrm stateshowsgetdescribed as "Delete an existing security association, or query its parameters" andlistas "Delete all matching security associations, or list them". These come from the alias groupings["delete","get"]and["deleteall","list"]inip.jsonsharing one description, which the dropdown then repeats per alias. Reads oddly on the second alias of each pair.ip tunnel add modeentries (gre,ipip,isatap,sit,vti) carry no descriptions.ip monitorlists 18 entries where the first 14 object types have no descriptions and only the trailing 4 options (all-nsid,dev,file,label) do.Computer-use video recordings (3)
Warp autocomplete verification: Running `ip -o link show` in Warp, then typing seven `ip ...` command prefixes and pressing Tab to open each completion dropdown, pausing on each so entries are readable.
Warp autocomplete diagnostic pass: Diagnostic inspection of Tab autocomplete menus in a locally-built Warp terminal: Part A git completions, Part B baseline ip positions, Part C ip subcommand retries, and Part D ip -o link show / which ip outputs.
Warp ip autocomplete cases: Running ip -o link show, then typing nine ip subcommand prefixes and pressing Tab to inspect autocomplete dropdowns in a freshly built Warp terminal.
Computer-use screenshots (10)
Verbatim output of `ip -o link show` in the Warp terminal, listing interfaces lo, dummy0, eth0, teql0, tunl0, ip6tnl0, docker0.
Case 1: `ip rule add iif ` + Tab shows a dropdown of interface names (lo, dummy0, eth0, teql0, tunl0, ip6tnl0, and docker0 on scroll) each labeled "Interface (STATE)".
Case 2: `ip rule add ` + Tab shows a scrollable dropdown of 30 ip rule keywords (blackhole, dport, dscp, dsfield, flowlabel, from, ... unreachable) each with a description.
Case 3: `ip rule add oif ` + Tab shows the same interface-name dropdown (lo, dummy0, eth0, teql0, tunl0, ip6tnl0, docker0 on scroll) each labeled "Interface (STATE)".
Case 4: `ip tunnel add mode ` + Tab shows a dropdown of 5 tunnel modes (gre, ipip, isatap, sit, vti) with icons but no descriptions.
Case 5: `ip monitor dev ` + Tab shows the interface-name dropdown (lo, dummy0, eth0, teql0, tunl0, ip6tnl0, scrollable to docker0) each labeled "Interface (STATE)".
Case 6: `ip rule add iif eth0 from ` + Tab shows a file/directory completion dropdown from the CWD (about.hbs File, about.toml File, agents/ Directory, AGENTS.md File, app/ Directory, Cargo.lock File), not IP prefixes.
Case 7: `ip rule add iif eth0 ` + Tab (twice) produced no dropdown; the command line remains unchanged with the cursor after the trailing space.
Case 8: `ip xfrm state ` + Tab shows a scrollable dropdown of xfrm state subcommands (add, allocspi, count, delete, deleteall, flush, get, list, update) with descriptions.
Case 9: `ip monitor ` + Tab shows a scrollable 18-entry dropdown (acaddress, address, all, link, maddress, mroute, ... rule, stats, all-nsid, dev, file, label); the first 14 have no descriptions and only the last 4 have descriptions.
Co-Authored-By: Warp Agent agent@warp.dev