CLI tweaks: fix benchmarks symlink, add link command, grouped help#28
Merged
CLI tweaks: fix benchmarks symlink, add link command, grouped help#28
Conversation
…help The benchmarks binary alias was passing None to run_with_command instead of parsing args via clap, so `benchmarks list` just showed help. Now mirrors the agents pattern with a proper run() entry point. Also adds an examples section to `models -h` showing that bare `models` launches the TUI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add `models link` to create/remove/check symlinks for `agents` and `benchmarks` binary aliases. Supports --dir for custom target, --remove to clean up, and --status to inspect existing symlinks. Reorganize top-level help into grouped sections (Commands, Setup, Additional) using a custom help_template for better scannability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add after_help examples to the benchmarks show subcommand and to the BenchmarksCli struct (used by the benchmarks symlink entry point). Also add descriptions to existing show examples in the parent help. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a5d32c903
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+9
to
+11
| let binary = binary | ||
| .canonicalize() | ||
| .context("could not resolve binary path")?; |
There was a problem hiding this comment.
Avoid canonicalizing the executable before creating aliases
Because binary is canonicalized here and then reused both as the symlink target and for the default target_dir, running models link from a symlinked install (for example Homebrew’s /opt/homebrew/bin/models) will create agents/benchmarks in the versioned Cellar directory instead of the PATH entry, and the aliases stay pinned to that version. After an upgrade or package-manager cleanup, those links can disappear or break even though models itself still works.
Useful? React with 👍 / 👎.
Comment on lines
+120
to
+124
| match std::fs::read_link(&link_path) { | ||
| Ok(_) => { | ||
| std::fs::remove_file(&link_path).with_context(|| { | ||
| format!("failed to remove symlink: {}", link_path.display()) | ||
| })?; |
There was a problem hiding this comment.
Remove only symlinks that point to this models binary
models link --remove currently deletes any agents or benchmarks symlink it finds in the target directory. In shared PATH directories like ~/.local/bin or /usr/local/bin, that can remove an unrelated command or an alias that intentionally points at a different models install, even though the command is documented as removing previously created symlinks.
Useful? React with 👍 / 👎.
arimxyer
added a commit
that referenced
this pull request
Mar 23, 2026
Add user-configurable alias names for symlink commands via [aliases] section in config.toml. Default aliases: agents, benchmarks, mstatus. Users can override any alias name in their config. Also adds status as a third symlink target (default name mstatus to avoid collision with system commands). - Add AliasesConfig struct with serde defaults for backward compat - Add AliasKind enum for routing, match_alias() and alias_names() helpers - Refactor link.rs to read alias names from config instead of hardcoded const - Update main.rs argv[0] detection to use config-driven matching - Add cli::status::run() for standalone status alias invocation - Fix link canonicalization bug (PR #28 review) - Fix link removal safety (verify symlink target before removing) - Add non-TTY fallback for status sources command (PR #29 review) - Add detail state metadata to status show --json output (PR #29 review) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
arimxyer
added a commit
that referenced
this pull request
Mar 31, 2026
* fix: parse args for benchmarks symlink and add examples to top-level help The benchmarks binary alias was passing None to run_with_command instead of parsing args via clap, so `benchmarks list` just showed help. Now mirrors the agents pattern with a proper run() entry point. Also adds an examples section to `models -h` showing that bare `models` launches the TUI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add `models link` command and grouped help sections Add `models link` to create/remove/check symlinks for `agents` and `benchmarks` binary aliases. Supports --dir for custom target, --remove to clean up, and --status to inspect existing symlinks. Reorganize top-level help into grouped sections (Commands, Setup, Additional) using a custom help_template for better scannability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add examples to benchmarks show and benchmarks help text Add after_help examples to the benchmarks show subcommand and to the BenchmarksCli struct (used by the benchmarks symlink entry point). Also add descriptions to existing show examples in the parent help. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
arimxyer
added a commit
that referenced
this pull request
Mar 31, 2026
Add user-configurable alias names for symlink commands via [aliases] section in config.toml. Default aliases: agents, benchmarks, mstatus. Users can override any alias name in their config. Also adds status as a third symlink target (default name mstatus to avoid collision with system commands). - Add AliasesConfig struct with serde defaults for backward compat - Add AliasKind enum for routing, match_alias() and alias_names() helpers - Refactor link.rs to read alias names from config instead of hardcoded const - Update main.rs argv[0] detection to use config-driven matching - Add cli::status::run() for standalone status alias invocation - Fix link canonicalization bug (PR #28 review) - Fix link removal safety (verify symlink target before removing) - Add non-TTY fallback for status sources command (PR #29 review) - Add detail state metadata to status show --json output (PR #29 review) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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
benchmarkssymlink not parsing args (was passingNoneinstead of parsing via clap, unlikeagents)models linkcommand to create/remove/check symlinks foragentsandbenchmarksbinary aliases (--status,--remove,--dir)models -hinto grouped sections (Commands, Setup, Additional)benchmarks showandbenchmarks -hhelp textTest plan
benchmarks listworks via symlink (was broken, now parses args)models linkcreates symlinks, detects existing, removes cleanlymodels link --statusreports symlink state correctlymodels -hshows grouped sectionsmodels benchmarks show -hshows examples🤖 Generated with Claude Code