[codex] Default bare plinks invocation to manage#5
Merged
Conversation
📝 WalkthroughWalkthroughCLI now allows no subcommand (defaults to interactive Manage). Clap metadata and help text expanded for commands and args. run() dispatch uses an unwrap-or default to Manage. Tests added for help output and default parsing. README updated with build/install/development/packaging instructions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "CLI Parser\n(src/cli.rs)"
participant Runner as "run()\n(src/lib.rs)"
participant TUI as "tui::run()\n(src/tui)"
User->>CLI: invoke binary (with or without args)
CLI-->>Runner: returns Cli { command: Option<Command> }
Runner->>Runner: command = cli.command.unwrap_or(Command::Manage)
alt command == Manage
Runner->>Runner: resolve_config_path(cwd)?
Runner->>TUI: call tui::run(resolved, opener)
TUI-->>User: start interactive TUI
else other command
Runner->>Runner: dispatch to matching command handler
Runner-->>User: perform requested action (add/open/list/remove)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
adambuchweitz
force-pushed
the
codex/default-manage-command
branch
2 times, most recently
from
April 13, 2026 14:28
195134f to
b8be804
Compare
adambuchweitz
marked this pull request as ready for review
April 13, 2026 16:23
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/cli.rs (1)
57-58: Improve failure diagnostics inhelp_output.If the subprocess fails, include stderr in the assertion message to make CI failures actionable.
Suggested tweak
- assert!(output.status.success()); + assert!( + output.status.success(), + "plinks help command failed: {}", + String::from_utf8_lossy(&output.stderr) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli.rs` around lines 57 - 58, The test helper help_output should include the subprocess stderr when asserting success: update the assertion that currently uses output.status.success() to include the stderr (e.g. String::from_utf8_lossy(&output.stderr)) in the assertion message so CI failures show actionable error output; keep returning the stdout as String::from_utf8(output.stdout).unwrap() after the improved assert.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/cli.rs`:
- Around line 57-58: The test helper help_output should include the subprocess
stderr when asserting success: update the assertion that currently uses
output.status.success() to include the stderr (e.g.
String::from_utf8_lossy(&output.stderr)) in the assertion message so CI failures
show actionable error output; keep returning the stdout as
String::from_utf8(output.stdout).unwrap() after the improved assert.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d544cc7-7b93-4ec0-be59-6c3bf9a3bb15
📒 Files selected for processing (3)
src/cli.rssrc/lib.rstests/cli.rs
adambuchweitz
force-pushed
the
codex/default-manage-command
branch
from
April 13, 2026 17:54
b8be804 to
af8382f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/cli.rs (2)
51-59: Improve assertion diagnostics inhelp_output.When subprocess help fails, the current assert loses stderr context, making triage slower.
Diff suggestion
fn help_output(args: &[&str]) -> String { let output = ProcessCommand::new(env!("CARGO_BIN_EXE_plinks")) .args(args) .output() .unwrap(); - assert!(output.status.success()); - String::from_utf8(output.stdout).unwrap() + assert!( + output.status.success(), + "help command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).expect("help output must be valid UTF-8") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli.rs` around lines 51 - 59, The assert in help_output loses stderr when the subprocess fails; change the success check in the help_output function to include stdout/stderr in the failure message (e.g., replace assert!(output.status.success()) with an assertion or panic that formats output.status, String::from_utf8_lossy(&output.stdout) and String::from_utf8_lossy(&output.stderr) so test failures show both stdout and stderr for triage).
314-318: Remove exact spacing from help text assertions to prevent brittleness.Lines 314–318 check help output with spacing-dependent patterns (e.g.,
"open Open..."). These can fail after clap version updates or formatter changes even when functionality is unchanged. Refactor to check only the descriptions:Suggested refactor
- assert!(help.contains("open Open a saved link by primary name, alias, or tag")); - assert!(help.contains("list List saved links")); - assert!(help.contains("add Add a saved link")); - assert!(help.contains("remove Remove a saved link by primary name")); - assert!(help.contains("manage Open the interactive terminal UI")); + assert!(help.contains("Open a saved link by primary name, alias, or tag")); + assert!(help.contains("List saved links")); + assert!(help.contains("Add a saved link")); + assert!(help.contains("Remove a saved link by primary name")); + assert!(help.contains("Open the interactive terminal UI"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli.rs` around lines 314 - 318, The help output assertions currently depend on exact spacing (e.g., assert!(help.contains("open Open a saved link..."))); change them to avoid brittle spacing by asserting the command name and its description exist separately in the help string (use the existing help variable). For each case (open, list, add, remove, manage) replace the spacing-dependent assertion with a pair or single assertion that checks help.contains("open") and help.contains("Open a saved link"), and likewise for "list"/"List saved links", "add"/"Add a saved link", "remove"/"Remove a saved link by primary name", and "manage"/"Open the interactive terminal UI" so formatting changes won't break the tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/cli.rs`:
- Around line 51-59: The assert in help_output loses stderr when the subprocess
fails; change the success check in the help_output function to include
stdout/stderr in the failure message (e.g., replace
assert!(output.status.success()) with an assertion or panic that formats
output.status, String::from_utf8_lossy(&output.stdout) and
String::from_utf8_lossy(&output.stderr) so test failures show both stdout and
stderr for triage).
- Around line 314-318: The help output assertions currently depend on exact
spacing (e.g., assert!(help.contains("open Open a saved link..."))); change
them to avoid brittle spacing by asserting the command name and its description
exist separately in the help string (use the existing help variable). For each
case (open, list, add, remove, manage) replace the spacing-dependent assertion
with a pair or single assertion that checks help.contains("open") and
help.contains("Open a saved link"), and likewise for "list"/"List saved links",
"add"/"Add a saved link", "remove"/"Remove a saved link by primary name", and
"manage"/"Open the interactive terminal UI" so formatting changes won't break
the tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6e9d0159-54d6-41a3-89bb-355419991cd1
📒 Files selected for processing (4)
README.mdsrc/cli.rssrc/lib.rstests/cli.rs
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib.rs
adambuchweitz
force-pushed
the
codex/default-manage-command
branch
2 times, most recently
from
April 13, 2026 18:27
3528bde to
d6aa40b
Compare
adambuchweitz
force-pushed
the
codex/default-manage-command
branch
from
April 13, 2026 18:44
d6aa40b to
4bf6520
Compare
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/cli.rs (1)
343-397: Consider a tiny helper to reduce repeated help-assert boilerplate.The repeated
contains + identical error messagepattern is readable but a bit verbose; a local helper would keep future help tests easier to extend.♻️ Optional refactor
+fn assert_help_contains(help: &str, expected: &str, context: &str) { + assert!( + help.contains(expected), + "unexpected {context} help output (missing: {expected}):\n{help}" + ); +}- assert!( - help.contains("Add a saved link"), - "unexpected add help output:\n{help}" - ); + assert_help_contains(&help, "Add a saved link", "add");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cli.rs` around lines 343 - 397, Add a small local test helper to remove repeated assert!(help.contains(...), "unexpected ...\n{help}") boilerplate: create e.g. assert_help_contains(help: &str, expected: &str, label: &str) that calls assert!(help.contains(expected), "unexpected {label} help output:\n{help}"), then replace the repeated assertions in the add-help checks and the test open_help_explains_name_alias_and_tag_modes (which calls help_output) with calls to assert_help_contains(help, "...", "add" or "open") to make the tests shorter and clearer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/cli.rs`:
- Around line 343-397: Add a small local test helper to remove repeated
assert!(help.contains(...), "unexpected ...\n{help}") boilerplate: create e.g.
assert_help_contains(help: &str, expected: &str, label: &str) that calls
assert!(help.contains(expected), "unexpected {label} help output:\n{help}"),
then replace the repeated assertions in the add-help checks and the test
open_help_explains_name_alias_and_tag_modes (which calls help_output) with calls
to assert_help_contains(help, "...", "add" or "open") to make the tests shorter
and clearer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83c9b3ea-ba08-4c92-a037-066de870b9e2
📒 Files selected for processing (4)
README.mdsrc/cli.rssrc/lib.rstests/cli.rs
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib.rs
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
plinksinvocations to the interactivemanageflowWhy
Running
plinkswith no arguments previously failed during clap parsing because a subcommand was required. For a CLI/TUI entrypoint, the expected behavior is to open the interactive manager when no explicit command is provided.Impact
Users can now launch the TUI with either
plinksorplinks manage, while scripted subcommand usage continues to behave the same.Validation
cargo testSummary by CodeRabbit
New Features
Documentation
Tests