Add completion spec: pkill - #300
Conversation
`pkill` had no command signature, so Warp's argument completer fell back to filesystem paths instead of offering running process names (GH #10924). Add a `pkill` spec whose pattern argument is driven by a process-name generator, and move that generator (plus the signal-name generator `pkill` reuses for `--signal`) into `common.rs`, per the repo's generator-reuse convention. The shared process-name generator suppresses the `ps` header, reduces macOS's absolute executable paths to basenames, keeps Linux's bare names, and de-duplicates, so `killall` now also produces suggestions on Linux. 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
This PR adds a pkill command signature, registers dynamic generators for process names, signal names, and users, and refactors the existing kill/killall generator helpers into shared code. The tests cover the new signature wiring and the cross-platform process-name parser behavior.
Concerns
No blocking correctness, security, or spec-alignment concerns were found in the annotated diff. The attached spec context states that no approved or repository spec context was found for this PR.
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
## Description Dependency version bump: pinned `warp-command-signatures` rev `a2ad4bfb…` → `4990fa1d4f5a9c74251c7330484a7ab6f3d38ac1`. That rev is the merge commit of [warpdotdev/command-signatures#300](warpdotdev/command-signatures#300), which adds the `pkill` completion spec and its process-name generator. `pkill` previously had no signature, so the argument completer fell through to `CompletionsFallbackStrategy::FilePaths` and offered filesystem paths (#10924). The bump also picks up the sibling `killall` generator fix from the same PR. The diff is `Cargo.toml` + `Cargo.lock` only. The `Cargo.lock` change is limited to the two `warp-command-signatures` / `warp-completion-metadata` source lines — a plain `cargo update -p` additionally re-resolved unrelated `windows-sys`/`base64` pins, so those two lines were applied directly instead. ## Linked Issue - GitHub: #10924 - Linear: [APP-5192](https://linear.app/warpdotdev/issue/APP-5192/pkill-tab-completes-filesystem-paths-instead-of-process-names-gh-10924) - Picks up (merged): [warpdotdev/command-signatures#300](warpdotdev/command-signatures#300) - **Originating thread**: https://warpdev.slack.com/archives/C0BDQDW8V5E/p1785965967514419 ## Testing No test is added here by request — this is a version bump, and the `pkill` completion behavior is covered by command-signatures#300's own tests in that repo (a signature-level regression test plus generator parser tests, run under its `script/presubmit`). Checks run against the bumped rev in a Linux runner: - `cargo metadata --locked` — **pass**; the lock is consistent with `Cargo.toml` and the new rev resolves. - `cargo check -p warp_completer --locked` — **pass**; the crate that consumes `warp-command-signatures` builds against it. - `cargo fmt -p warp_completer -- --check` — **pass**. Not run: the full workspace build/test suite, and any GUI run. CI is the backstop. ### Screenshots / Videos No visual capture — scoped out with the test at the requester's direction. - [ ] I have manually tested my changes locally with `./script/run` ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode CHANGELOG-BUG-FIX: `pkill <TAB>` now suggests running process names instead of filesystem paths. <!-- factory-agent: {"source":"factory-agent","task_id":"APP-5192","task_source":"linear","task_url":"https://linear.app/warpdotdev/issue/APP-5192/pkill-tab-completes-filesystem-paths-instead-of-process-names-gh-10924","oz_run_id":"019fd3e8-c360-7c50-8764-c317eb92d2e5","repo":"warpdotdev/warp"} --> _Conversation: https://staging.warp.dev/conversation/e5efc7bf-3937-4b34-b5d4-19bf26d4890d_ _Run: https://oz.staging.warp.dev/runs/019fd3e8-c360-7c50-8764-c317eb92d2e5_ _This PR was generated with [Oz](https://warp.dev/oz)._ Co-authored-by: Warp Agent <agent@warp.dev>
Problem
pkill <TAB>in Warp offers filesystem paths instead of running process names — reported in warpdotdev/warp#10924.pkillhas no command signature, so Warp's argument completer has nothing to complete against and falls back toCompletionsFallbackStrategy::FilePaths. Every other process-signalling command in this repo (kill,killall) already has a spec, which is why they behave correctly.Fix
command-signatures/json/pkill.json— new spec. The positionalpatternargument is driven by aprocess_namegenerator, and the option set covers both procps-ng (Linux) and BSD/macOSpkill, with platform-only flags marked in their descriptions.--signalcompletes signal names and-u/-Ucomplete user names;-F/--pidfileuses thefilepathstemplate (the one place a path really is wanted).command-signatures/src/generators/pkill.rs— registersprocess_name,signal_name, anduser_nameforpkill.command-signatures/src/generators/common.rs—killall's process-name generator andkill's signal-name generator move here, sincepkillnow shares both.AGENTS.md/ theadd-command-specskill require generators used by more than one command to live incommon.rs.The shared
process_namesparser is stricter than the one it replaces: it suppresses thepsheader (ps -A -o comm=, with a header filter for thepsimplementations that print one anyway), reduces macOS's absolute executable paths to basenames, keeps Linux's already-bare names, and de-duplicates by name. The oldkillallparser only emitted lines containing/, so it produced no suggestions on Linux;killallpicks that fix up here.pgrephas the same missing-signature problem but is out of scope for this ticket.Verification
script/presubmit(prettier check,cargo fmt --check,cargo clippy -D warnings,cargo test) — passes, 82 tests, 0 failures.Regression tests in
command-signatures/src/generators/pkill_tests.rs:test_pkill_pattern_argument_completes_process_names— asserts the bundledpkillsignature's first positional argument uses theprocess_namegenerator and offers no path template. Confirmed failing before the fix: withjson/pkill.jsonremoved the test fails withpkill signature should be bundled, and it passes with the spec in place.test_pkill_registers_the_generators_its_spec_references, plus parser tests for macOS-style paths, Linux-style bare names, header/blank/duplicate filtering, and empty output.The generator pipeline was also run directly in the verification environment (
ps -A -o comm= | sort -u), which returns bare process names with no header row.Visual (UI) proof is not attached and is outstanding. Exercising the completion menu requires a full Warp client build against a path override of this crate, and this task scopes the client repo to the follow-up PR below. The rendered
pkill <TAB>menu should be captured on that PR, where the change actually becomes visible in the client.Follow-up: this PR must merge first
A companion PR in
warpdotdev/warpwill bump the pinnedwarp-command-signaturesgitrevinCargo.toml(currentlya2ad4bfbb641eabc05412ebce6798f3c6530db5b) to this PR's merge commit. Merge this PR first; the client-side bump depends on the commit landing here.Links
Conversation: https://staging.warp.dev/conversation/e5efc7bf-3937-4b34-b5d4-19bf26d4890d
Run: https://oz.staging.warp.dev/runs/019fd3e8-c360-7c50-8764-c317eb92d2e5
This PR was generated with Oz.