Skip to content

Add completions: ip netns - #313

Merged
acarl005 merged 2 commits into
mainfrom
factory/ip-netns-completions
Aug 13, 2026
Merged

Add completions: ip netns#313
acarl005 merged 2 commits into
mainfrom
factory/ip-netns-completions

Conversation

@warp-agent-staging

@warp-agent-staging warp-agent-staging Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Adds nested completions for ip netns (issue warpdotdev/warp#9765, Linear APP-3982): manage network namespaces (list, add, delete, exec, etc.).

Stacked on #310 (Add completions: ip rule, tunnel, xfrm, monitor), which has already merged into main. This PR is based on main (which now includes #310's work, including the network_interfaces generator) since #310's feature branch was deleted after merge. No changes were made to anything #310 added.

Update: a later verification pass found that Warp's completer never suggests bare (non-dash) keywords modeled as options entries — only subcommands entries are matched without a dash requirement. This is being fixed for ip rule/tunnel/xfrm/monitor in #311. My netns subtree had one instance of the same bug (list-id's target-nsid/nsid keywords), which is now fixed here to use subcommands instead of options, matching #311's pattern.

Changes

  • command-signatures/json/ip.json: fills in the netns subcommand with its full action set from upstream ip/ipnetns.c's do_netns() dispatch: list/show/lst, add, attach, set, del/delete, identify, pids, exec, monitor, list-id (with its target-nsid/nsid keywords modeled as subcommands, not options, since they're bare keywords), and help.
  • command-signatures/src/generators/ip.rs: adds a new processes generator (parses ps -eo pid,comm) for the PID positions in attach and identify, since those accept the PID of any running process on the system, not one already tied to a namespace. Includes unit tests.
  • Existing-namespace-name positions (del/delete NAME, set NAME, pids NAME, exec [NAME]) reuse the pre-existing netns generator (ip netns list), which was already wired up for the global -n/-netns option. New-name positions (add NAME, attach's first NAME arg) intentionally have no generator, since they expect a name that doesn't exist yet.

Sources verified

  • ip/ipnetns.c and man/man8/ip-netns.8.in from iproute2/iproute2 (main branch), specifically the usage() string and do_netns() dispatch table, which enumerate the full action set including the easy-to-forget ones (exec, pids, identify, set, attach, monitor, list-id).
  • Followed the del/delete dual-alias convention already established for ip rule in this repo (upstream's matches() does prefix matching, so del is technically accepted as an abbreviation of delete, same as for ip rule).

Deliberately left out

  • No version-recency annotations (e.g. "iproute2 6.13+") — I found no evidence any of these netns actions are recent additions; list-id, attach, and identify have all been present for many releases.
  • No generator for add/attach's new-name positions, since they must not already exist.
  • Per the engine's known limitation (only the first bare keyword after a verb reliably produces a dropdown), a chained second keyword such as ip netns list-id target-nsid 12 nsid <Tab> won't complete — this is a spec-independent engine constraint, not something fixable here.

Verification

  • npm run format:check — passes.
  • cargo fmt --check — passes.
  • cargo clippy --all-targets -- -D warnings — passes, no warnings.
  • cargo test --workspace — all 137 tests pass, including the 4 new parse_processes unit tests, plus the repo's invariant tests (every generatorName exists, all specs deserialize, no unquoted newlines in generator commands).
  • Manually confirmed the ps -eo pid,comm output format assumed by the new generator matches this sandbox's ps.
  • Gap: I do not have computer use enabled in this environment, so I could not capture a live Warp completions-dropdown screenshot for the new netns/processes generators. Noting this honestly per the task instructions — screenshots are being captured separately.

@oz-agent @warp-agent
Add nested completions for ip netns
b88e3de 
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>
@cla-bot cla-bot Bot added the cla-signed label Aug 13, 2026
@warp-agent-staging
warp-agent-staging Bot requested a review from acarl005 August 13, 2026 01:31
@oz-agent @warp-agent
Fix ip netns list-id: bare keywords must be subcommands, not options
8cd7e0a 
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>
@warp-agent-staging warp-agent-staging Bot mentioned this pull request Aug 13, 2026
Closed
@acarl005
acarl005 marked this pull request as ready for review August 13, 2026 01:40
@acarl005
acarl005 merged commit 80d3f58 into main Aug 13, 2026
8 checks passed
@acarl005
acarl005 deleted the factory/ip-netns-completions branch August 13, 2026 01:41
@oz-for-oss

oz-for-oss Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

@acarl005

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss[bot]
oz-for-oss Bot reviewed Aug 13, 2026
View reviewed changes

@oz-for-oss oz-for-oss Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This PR adds ip netns command completions, including namespace management subcommands and a new processes generator for PID arguments. The Rust parser is covered by focused unit tests, and the added comments align with the surrounding code's explanatory style.

Concerns

  • No blocking correctness, security, comment-quality, test-quality, or spec-drift concerns found in the attached diff.

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

@warp-agent-staging

warp-agent-staging Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Reviewed post-merge, having adopted this for warp#9765 / APP-3982. The grammar, aliases, argument arity and generator wiring all check out against upstream ip/ipnetns.c, and script/presubmit is green (137 tests).

One residual gap: the new processes generator never got a live completions-dropdown check, which review-pr-local requires for a new generator. The unit tests cover parsing, not rendering — worth a spot-check once the pinned rev bumps in warp.

Responding as wilson: Open session · View factory task

@warp-agent-staging warp-agent-staging Bot mentioned this pull request Aug 13, 2026
Closed
warp-agent-staging Bot pushed a commit that referenced this pull request Aug 13, 2026
@warp-agent
Fix ip spec defects and salvage nodes from #322
e1dc7f0 
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>
@warp-agent-staging warp-agent-staging Bot mentioned this pull request Aug 13, 2026
Merged
4 tasks
acarl005 pushed a commit to warpdotdev/warp that referenced this pull request Aug 13, 2026
@warp-agent-staging @oz-agent @warp-agent
[Completions] Bump command-signatures to 77c4a9a7 (#15101)
2861a6e 
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oz-for-oss oz-for-oss[bot] oz-for-oss[bot] left review comments
@acarl005 acarl005 acarl005 approved these changes

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@acarl005 @oz-agent