Add completion spec: tcpdump - #305
Merged
Merged
Conversation
Adds command-signatures/json/tcpdump.json covering tcpdump's option surface, plus an `interfaces` generator that lists capture interfaces from `tcpdump --list-interfaces` and reuses the shared users generator for -Z. APP-3966 Co-Authored-By: Warp <agent@warp.dev> Co-Authored-By: Warp Agent <agent@warp.dev>
|
@warp-agent-staging[bot] 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 Powered by Oz |
There was a problem hiding this comment.
Overview
Adds a tcpdump command signature covering documented options, pcap-filter positional suggestions, dynamic interface/user generators, and parser tests for representative tcpdump interface-list output.
Concerns
- No blocking concerns found in the changed lines.
spec_context.mdreports no approved or repository spec context, so there is no spec drift to flag.
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
Matches the repo's dominant convention of a #[cfg(test)] mod in the generator's own file. All six tests are unchanged. Co-Authored-By: Warp <agent@warp.dev> Co-Authored-By: Warp Agent <agent@warp.dev>
acarl005
approved these changes
Aug 6, 2026
acarl005
pushed a commit
to warpdotdev/warp
that referenced
this pull request
Aug 6, 2026
## Description Updates `warp-command-signatures` from `5e08807c` to `fe352669` so the client picks up the new `vagrant` completion spec. This is the client-side half of APP-3965; the spec itself merged in warpdotdev/command-signatures#306. Closes #9781 ### Merged PRs picked up by this bump - Add completion spec: Vagrant (vagrant) (warpdotdev/command-signatures#306) — the reason for this bump - Complete just completions: Justfile recipe generator (warpdotdev/command-signatures#307) - Add completion spec: tcpdump (warpdotdev/command-signatures#305) `fe352669` is the current `main` head of `warpdotdev/command-signatures` and is #306's merge commit. The diff is 3 lines — the `rev` in `Cargo.toml` and the two matching `source` lines in `Cargo.lock`. The two revs have **identical dependency manifests** (verified with `git diff 5e08807c fe352669 -- Cargo.toml '*/Cargo.toml'`, empty), so no other lockfile entry legitimately changes and no unrelated dependency churn rides along. `cargo metadata --locked` passes, confirming the lockfile is consistent. ## Verification **Testing-exempt category: dependency/version bump.** No regression test is added — the change is a git rev plus its lockfile consequence, so a unit test here could only assert the new rev string rather than detect a logic defect. The completion data itself is tested in `command-signatures`. The client-side resolution and UI checks below are the meaningful proof, and they are the whole point of this PR. ### Visual proof: `vagrant` completions in the running client Captured against a `warp-oss` build of **this PR's branch** (so the completions come from the pinned rev `fe352669`, not a local path override), running on an Xvfb display. The Vagrant CLI is not installed on the capture machine and is not needed — both generators are filesystem reads, so a fixture project (`.vagrant/machines/{web/virtualbox,db/libvirt,worker/docker}`) and a fixture box store were seeded. - **`vagrant <TAB>`** — the full static subcommand list with descriptions, in spec order (`autocomplete`, `box`, `cloud`, `destroy`, `docker-exec`, `docker-logs`, `docker-run`, `global-status`, `halt`, `help`, `init`, `list-commands`, `login`, `package`, `plugin`, `port`, `powershell`, `provider`, `provision`, `push`, `rdp`, `reload`, `resume`, `rsync`, `rsync-auto`, `snapshot`, `ssh`, `ssh-config`, `status`, `suspend`, `up`, `upload`, `validate`, `version`, `winrm`, `winrm-config`), followed by the global flags. - **`vagrant up <TAB>`** — the `vagrant_machines` generator resolving live against the fixture project: `db` described as `libvirt`, `web` as `virtualbox`, `worker` as `docker`, followed by `up`'s own flags (`--provision`/`--no-provision`, `--provider`, `--destroy-on-error`, …). - **`vagrant box remove <TAB>`** — the `vagrant_boxes` generator, **with the `-VAGRANTSLASH-` / `-VAGRANTCOLON-` un-escaping applied**: the on-disk directories `hashicorp-VAGRANTSLASH-bionic64`, `ubuntu-VAGRANTSLASH-focal64`, `generic-VAGRANTSLASH-alpine318`, and `registry.example.com-VAGRANTCOLON-8080-VAGRANTSLASH-internal-base` render as `hashicorp/bionic64`, `ubuntu/focal64`, `generic/alpine318`, and `registry.example.com:8080/internal-base`. No literal `VAGRANTSLASH`/`VAGRANTCOLON` text appears anywhere in the menu. One cosmetic note from the capture, not caused by this PR: the longest fixture entry (`registry.example.com:8080/internal-base`, an artificially long name invented for the un-escaping test) renders with its `Installed box` description column squeezed to empty, while the shorter names show it. The generator attaches that description unconditionally, so this is dropdown column truncation on an unusually long name rather than missing data. ### Deterministic checks - **`vagrant` resolves through warp's own completer registry.** Ran an ad-hoc (uncommitted) test against `CommandRegistry::global_instance()` — the same registry `crates/warp_completer/src/signatures/legacy/mod.rs` builds from the embedded signatures. Result: `registry.signature("vagrant")` returns `Some`, description `"Build and manage reproducible virtual machine environments"`, **36 subcommands**, and the nested `box` group resolves to `add, list, outdated, prune, remove, repackage, update, help`. - **Crate-level check against the exact feature set `warp_completer` enables.** Exercised `warp_command_signatures::signature_by_name("vagrant")` and `dynamic_command_signature_data()` with `default-features = false, features = ["embed-signatures"]`, matching what `warp_completer` enables on non-wasm. Result: signature resolves with all 36 subcommands, nested groups populated (`cloud`: auth/box/provider/publish/search/version; `plugin`: install/license/list/uninstall/update/repair/expunge; `snapshot`: delete/list/pop/push/restore/save), 19 options on `vagrant up`, and both `vagrant_machines` and `vagrant_boxes` generators registered in `dynamic_command_signature_data()`. An unknown command still resolves to `None`, confirming embedded data is genuinely being read. - **Before/after.** The same check against the *old* pin `5e08807c` returns `NOT FOUND` for `vagrant` with no generators registered, so this bump is what delivers the signature. - **`./script/format --check`** — clean. - **`cargo clippy -p warp_completer --all-targets --locked -- -D warnings`** — clean. - **`cargo check -p warp_completer --locked`** — clean. - **`cargo metadata --locked`** — passes. - **`cargo test -p warp_completer --locked`** — 138 passed, 25 failed. Those 25 failures are **pre-existing and unrelated**: the identical command on a clean `origin/master` checkout without this change produces the byte-identical `138 passed; 25 failed`. They all panic in `warp_features` with "Tried to check FeatureFlag::CloudEnvironments before feature flags were initialized", which happens because a single-crate test run does not pull in `warp_features/test-util` the way a workspace run does. Not introduced here. - The workspace-wide `./script/presubmit` was not run — disproportionate to a manifest-only pin, and CI covers it. Worth flagging for reviewers: the spec-surface regression tests in `command-signatures` cover the JSON there, but **nothing in this repo will fail if the `vagrant` signature is later dropped or renamed upstream** — the checks above are one-time confirmations, not standing guards. CHANGELOG-IMPROVEMENT: Added completions for `vagrant`, including its nested `box`, `cloud`, `plugin`, and `snapshot` command groups plus dynamic machine and box names. Originating thread: https://warpdev.slack.com/archives/C0BDQDW8V5E/p1785980828616589 <!-- oz:computer-use-videos start --> ### Computer-use video recordings [](https://oz.staging.warp.dev/artifacts/019fd53d-2cda-72e8-b3b4-75eb7b11a7c9) **Warp vagrant command-completion dropdowns**: Demonstrating Warp's completion dropdown for three command lines: "vagrant ", "vagrant up ", and "vagrant box remove ", pausing on each dropdown. <!-- oz:computer-use-videos end --> <!-- oz:computer-use-screenshots start --> <details> <summary>Computer-use screenshots (3)</summary>  Warp completion dropdown after typing "vagrant " showing autocomplete, box, cloud, destroy, docker-exec, docker-logs.  Warp completion dropdown after typing "vagrant up " showing db (libvirt), web (virtualbox), worker (docker), then flags --color, --debug, --debug-timestamp.  Warp completion dropdown after typing "vagrant box remove " showing box names with literal slashes/colon: generic/alpine318, hashicorp/bionic64, registry.example.com:8080/internal-base, ubuntu/focal64, then flags --all, --all-architectures. </details> <!-- oz:computer-use-screenshots end --> ### Rework changes - **[IMPORTANT] `Cargo.toml` — visual proof of `vagrant <TAB>` required.** Addressed. Built `warp-oss` from this PR's branch (not a local signatures path override, so the capture exercises the pinned rev) and captured a video plus three screenshots of `vagrant <TAB>`, `vagrant up <TAB>` (the `vagrant_machines` generator), and `vagrant box remove <TAB>` (the `vagrant_boxes` generator including `-VAGRANTSLASH-`/`-VAGRANTCOLON-` un-escaping). Embedded above and posted to the Linear ticket. The previous "no GUI/computer-use proof" caveat has been removed rather than left beside the new evidence. - No code changes in this cycle — the diff is unchanged at 3 lines across `Cargo.toml` and `Cargo.lock`. <!-- factory-agent: {"source":"factory-agent","task_id":"APP-3965","task_source":"linear","task_url":"https://linear.app/warpdotdev/issue/APP-3965/add-vagrant-completions","oz_run_id":"019fd50c-bdf9-71f9-98f4-305716f44f2f","repo":"warpdotdev/warp","review_rework_attempts":1} --> _This PR was generated with [Oz](https://warp.dev/oz)._ Co-authored-by: Warp Agent <agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a completion spec for
tcpdump, which Warp did not ship a signature for — typingtcpdump <tab>fell back to generic path completions. Resolves APP-3966 / warpdotdev/warp#9780.command-signatures/json/tcpdump.json— 64 options transcribed from the fish completion and cross-checked againsttcpdump --helpand the tcpdump(1) man page (4.99.4): capture selection (-i,-c,-s,-p,-Q,--immediate-mode), savefiles (-w,-r,-V,-C,-G,-W,-z), output control (-A/-x/-xx/-X/-XX,-e,-q,-v/-vv/-vvv,-n/-nn/-N/-f), timestamps (-tthrough-ttttt,-j,--time-stamp-precision,--micro,--nano), the-d/-dd/-dddBPF dumps, and the listing flags (-D,-J,-L). Static enumerations come from the tool itself: the-Tpacket types were each probed againsttcpdump -T <type>so only the ones it accepts are offered, and-Q/-j/--time-stamp-precisioncarry their documented value sets. Path-taking options usefilepaths, and-wusesfilepathsMayNotExistsince it creates its savefile. The positional argument is the variadic pcap-filter expression, seeded with the common primitives and combinators (host,net,port,src,dst,tcp,and,not, …).command-signatures/src/generators/tcpdump.rs— two generators.interfaces(for-i/--interface) parsestcpdump --list-interfaces;user_name(for-Z) iscommon::users_generator(), already shared bypkill,killallandlsof.Why
-idoes not reuse nmap'snetwork_interfacesnmap.rshas anetwork_interfacesgenerator that lists kernel netdevs from/sys/class/netorifconfig. Fortcpdumpthat list is strictly worse:tcpdump -Dis the authority on what libpcap can actually capture on, and it additionally reports the pseudo-devices (any,nflog,nfqueue,dbus-system, …) plus a human description and link status per interface — none of which appear in a netdev listing.anyin particular is one of the most commonly typed values for-i. The two generators answer different questions from different sources, so this is not the shared-generator case thatadd-command-specasks to hoist intocommon.rs;-Zis, and it reuses the shared one.Missing tools fail soft: the generator runs through
single_command_and_ignore_stderr, so a host withouttcpdumponPATH(or one where the capture devices are unreadable) yields no suggestions rather than error text, and nothing blocks or hangs.Verification
./script/presubmitis green —npm run format:check,cargo fmt --check,cargo clippy --all-targets --all-features -D warnings, andcargo test(99 passed, 0 failed).The
#[cfg(test)] mod testsat the bottom ofcommand-signatures/src/generators/tcpdump.rsadds 6 parser tests over real--list-interfacesoutput: index ordering is preserved, descriptions combine the parenthesized text with the link status, nested parentheses survive, a bare[none]status falls back to a generic label, the older name-only output shape works, and non-index lines and empty output produce nothing.Exercised against the registry and a real
tcpdump(4.99.4 / libpcap 1.10.4) with a scratch harness (not committed) that resolves the signature throughsignature_by_namethe same way the client does:That live run is what caught the nested-parenthesis bug now covered by a test — the first parser truncated
nflogtoLinux netfilter log (NFLOG.Fail-soft was checked directly: with
tcpdumpremoved fromPATH,tcpdump --list-interfaces 2>/dev/nullexits 127 immediately with empty stdout, so the generator returns no suggestions.Visual proof — not produced here
.agents/skills/add-command-specasks for a Warp dropdown screenshot per generator. This cloud agent runs in a headless container with no display, so a Warp GUI build could not be exercised; the same exception was accepted on the journalctl spec (#299). The ticket's Testing section calls registry/unit-level proof sufficient for a signature-data change, and that is what the evidence above is. Anyone wanting the dropdowns can reproduce them with.agents/skills/test-local-warp— point warp'swarp-command-signaturesdependency at this checkout,cargo run --features fast_dev, thentcpdump -i <tab>andtcpdump -Z <tab>.Follow-up
warpdotdev/warpneeds awarp-command-signaturesgitrevbump after this merges for the client to pick the signature up, the same way warp#14742 did for journalctl. No warp PR is opened here.Originating thread: https://warpdev.slack.com/archives/C0BDQDW8V5E/p1785980450.964609
Conversation: https://staging.warp.dev/conversation/2c02c743-e687-4176-9a7d-77cab1072d4b
Run: https://oz.staging.warp.dev/runs/019fd4c2-8186-77cb-9fc2-607b21a615d4
This PR was generated with Oz.