Make numan completions install by default

[tonythethompson]

Summary

• numan completions <shell> now installs to the canonical path and creates parent directories (mkdir + write), so redirects no longer fail on a missing completion dir
• --print keeps the previous stdout behavior for piping and custom redirects
• Docs, changelog, and tests updated for the new default

Test plan

• cargo test completions
• cargo clippy -- -D warnings
• Run numan completions bash and confirm ~/.local/share/bash-completion/completions/numan is created
• Run numan completions bash --print | head and confirm script-only stdout

Made with Cursor

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[sourcery-ai]

Sorry @tonythethompson, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Review limit reached

@tonythethompson, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b1f9b4a8-5348-4ca0-add9-5038d256bfb5

📥 Commits

Reviewing files that changed from the base of the PR and between d5b599a and e881232.

📒 Files selected for processing (6)

• AGENTS.md
• CHANGELOG.md
• README.md
• src/cli.rs
• src/cmd/completions.rs
• tests/completions_test.rs

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/completions-default-install

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch feature/completions-default-install

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Install numan shell completions by default; keep --print for piping

✨ Enhancement 📝 Documentation 🧪 Tests 🕐 40+ Minutes

AI Description

• Make numan completions  install to canonical locations by default.
• Add --print to preserve stdout-only script output for piping/redirects.
• Update docs, changelog, and tests to reflect the new default behavior.

Diagram

graph TD
  A["completions cmd"] --> B{"--print?"}
  B -- "yes" --> C["generate script"] --> D["stdout output"] --> E["stderr hints"]
  B -- "no" --> F["default path"] --> G["mkdir + write"] --> H[("completion file")] --> D

  subgraph Legend
    direction LR
    _p["Process"] ~~~ _d{"Decision"} ~~~ _f[("File")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep printing by default; add `--install` flag

• ➕ Avoids any behavior change for users relying on stdout redirects/pipes without flags
• ➕ Keeps the command “pure” (no filesystem side effects) unless explicitly requested
• ➖ Worse first-run UX: users still hit missing-directory redirect failures
• ➖ Requires extra docs and flag teaching for the most common use case

2. Add `--path ` override (optional)

• ➕ Supports nonstandard layouts (custom zsh/fish paths, managed dotfiles) without manual redirects
• ➕ Pairs well with the new default for power users
• ➖ Expands CLI surface area and testing matrix
• ➖ Needs careful validation (file vs dir, permissions, Windows path quirks)

3. Delegate install locations to a dedicated “completion installer” crate

• ➕ Centralizes canonical per-shell paths and edge cases
• ➕ Potentially reduces maintenance burden long-term
• ➖ Adds dependency and constrains behavior to crate decisions
• ➖ May not match project-specific UX needs (like --print hints on stderr)

Recommendation: The chosen approach (install by default + --print for stdout-only) is the best UX tradeoff: it eliminates the common “redirect fails because dir doesn’t exist” footgun while preserving pipe/redirect workflows with an explicit flag. Consider a follow-up --path override only if users request custom install destinations.

Files changed (5) +207 / -56

Enhancement (1) +162 / -28

completions.rsImplement default installation paths, mkdir+write install, and '--print' flag +162/-28

Implement default installation paths, mkdir+write install, and '--print' flag

• Changes 'numan completions' to install completion scripts to canonical per-shell locations by default, creating parent directories and writing the script file. Adds '--print' to emit the script on stdout while keeping redirect/pipe hints on stderr, and introduces helpers for path resolution and installation with expanded unit tests.

src/cmd/completions.rs

Tests (1) +28 / -3

completions_test.rsExtend integration tests for 'print_hint' and install directory creation +28/-3

Extend integration tests for 'print_hint' and install directory creation

• Renames tests to match 'print_hint' and verifies the hint includes the '--print' usage. Adds coverage ensuring 'install_to' creates parent directories and writes a non-empty script for each supported shell.

tests/completions_test.rs

Documentation (3) +17 / -25

AGENTS.mdUpdate command reference for completions default install + '--print' +1/-1

Update command reference for completions default install + '--print'

• Adjusts the agent-facing command map to reflect that 'numan completions <shell>' now installs by default and reserves stdout printing for '--print'. This keeps internal documentation aligned with the new UX.

AGENTS.md

CHANGELOG.mdDocument completions default-install behavior change +4/-0

Document completions default-install behavior change

• Adds a changelog entry noting that completions now install by default and that '--print' preserves stdout emission for piping or custom redirects.

CHANGELOG.md

README.mdRevise shell completions docs for default install and advanced '--print' usage +12/-24

Revise shell completions docs for default install and advanced '--print' usage

• Updates the README to describe the new default installation behavior and simplifies the quick-start examples. Adds an advanced section showing '--print' with redirects/pipes and clarifies PowerShell guidance.

README.md

[Copilot]

Pull request overview

This PR changes numan completions <shell> from “print-to-stdout by default” to “install-to-canonical-path by default”, adding a --print flag to preserve the prior pipe/redirect workflow while keeping copy-ready hints on stderr.

Changes:

• Add --print mode and default install behavior (mkdir -p + write) for completion scripts.
• Introduce canonical per-shell install paths and shared install_to(...) helper.
• Update documentation, changelog, and tests to match the new UX.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/cmd/completions.rs	Implements default install flow, --print behavior, canonical paths, and updated hint logic.
tests/completions_test.rs	Updates hint test and adds coverage for install-to-path behavior across shells.
README.md	Documents new default install behavior and --print usage.
CHANGELOG.md	Notes the default behavior change for numan completions.
AGENTS.md	Updates command description to reflect install-by-default + --print.

[greptile-apps]

Greptile Summary

The PR changes numan completions <shell> to install completion scripts by default while retaining script-only stdout through --print.

• Resolves Fish installation paths through XDG_CONFIG_HOME, with ~/.config as the fallback.
• Creates missing parent directories and atomically replaces Numan-owned completion files.
• Updates command help, documentation, changelog entries, and completion tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/cmd/completions.rs	Implements default completion installation, XDG-aware Fish paths, ownership validation, and atomic replacement; both previously reported issues are resolved.
tests/completions_test.rs	Adds coverage for parent-directory creation across supported shells and updates print-hint assertions.
src/cli.rs	Updates completion-command help to describe installation-by-default and the --print escape hatch.
README.md	Documents the new default installation workflow and explicit printing or redirection behavior.

_{Reviews (2): Last reviewed commit: "Honor XDG_CONFIG_HOME for Fish completio..." | Re-trigger Greptile}

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. install_to overwrites without ownership ✓ Resolved 📘 Rule violation ≡ Correctness

Description

install_to() writes completion files via fs::write without asserting that an existing target
file is Numan-managed and without prepending the required ownership header. This can overwrite
user-managed completion files and violates the managed-file ownership requirement.

Code

src/cmd/completions.rs[R111-112]

+    fs::write(path, script.as_bytes())
+        .with_context(|| format!("Failed to write completions to {}", path.display()))?;

Relevance

●●● Strong

Ownership assertion before overwrite is an established safety requirement and has been accepted
previously.

PR-#66

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2436707 requires an OWNERSHIP_MARKER header in Numan-managed files and an ownership
assertion before overwriting. The new install_to() implementation creates directories and then
directly calls fs::write(path, ...), which will overwrite existing files regardless of ownership
and writes the raw generated script without any ownership marker header.

Rule 2436707: Enforce OWNERSHIP_MARKER header and ownership assertion in Numan-managed files
src/cmd/completions.rs[95-113]
src/util/fs_safety.rs[145-328]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/cmd/completions.rs::install_to()` overwrites the target completion file directly via `fs::write` with no ownership assertion and no `OWNERSHIP_MARKER` header in the written content.

## Issue Context
The project requires Numan-managed files to include an ownership header and to refuse overwriting files that are not owned by Numan.

## Fix Focus Areas
- src/cmd/completions.rs[95-113]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unquoted PowerShell dot-source ✓ Resolved 🐞 Bug ≡ Correctness

Description

execute() prints a PowerShell dot-source instruction using the raw path display, which breaks when
the install path contains spaces (common on Windows user profiles). Users will copy/paste a command
that fails even though installation succeeded.

Code

src/cmd/completions.rs[R52-54]

+    if matches!(args.shell, CompletionShell::PowerShell) {
+        println!("Add to $PROFILE (once): . {}", path.display());
+    }

Relevance

●●● Strong

Repo recently accepted quoting/escaping printed command paths to handle spaces/Windows shells.

PR-#95

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new PowerShell message prints . {path.display()} unquoted, so a path like `C:\Users\John
Doe\.numan\completions.ps1` becomes multiple tokens in PowerShell. This repo has already accepted a
similar bug pattern where unescaped paths in printed shell instructions break when paths contain
spaces.

src/cmd/completions.rs[36-55]
PR-#95

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`numan completions powershell` prints `Add to $PROFILE (once): . {path}` without quoting/escaping, producing an invalid PowerShell command when the path contains spaces or quotes.

## Issue Context
The command now installs to a file path under the user home directory and prints a follow-up instruction for PowerShell users. That instruction should be copy/paste safe.

## Fix Focus Areas
- src/cmd/completions.rs[52-54]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Non-atomic completions write ✓ Resolved 🐞 Bug ☼ Reliability

Description

install_to() overwrites the destination using fs::write, which can leave a truncated/partial
completions file if the process is interrupted or the write fails mid-stream. The repo already
provides write_bytes_atomic() for atomic replacement but it isn’t used here.

Code

src/cmd/completions.rs[R111-112]

+    fs::write(path, script.as_bytes())
+        .with_context(|| format!("Failed to write completions to {}", path.display()))?;

Relevance

●● Moderate

Atomic write helper exists and was adopted elsewhere, but may be viewed as optional for completions
files.

PR-#94

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

install_to() currently performs a direct overwrite via fs::write. The repository already
includes an atomic byte-write helper designed for exactly this use case (tempfile + flush + atomic
persist/rename), which would avoid partially-written completion scripts.

src/cmd/completions.rs[95-113]
src/util/atomic.rs[6-32]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`install_to()` writes the completions file directly via `fs::write`, which is not atomic and can leave a corrupted destination on partial writes.

## Issue Context
The codebase already has `crate::util::atomic::write_bytes_atomic()` which writes to a temp file in the same directory, flushes, and persists/renames atomically.

## Fix Focus Areas
- src/cmd/completions.rs[95-113]
- src/util/atomic.rs[15-31]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Completions help text stale ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The clap help for the completions subcommand still says it "generates" scripts, but the command
now installs by default and requires --print to emit scripts. This mismatch will confuse users
relying on numan --help.

Code

src/cmd/completions.rs[R45-47]

+    let path = default_install_path(args.shell)?;
+    install_to(args.shell, &path)?;
+    println!(

Relevance

●●● Strong

Team often fixes user-facing CLI/help text drift to match actual behavior.

PR-#82
PR-#35

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

src/cli.rs still advertises the completions subcommand as script generation, while the updated
execute() path installs by default unless --print is used.

src/cli.rs[52-57]
src/cmd/completions.rs[36-47]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`numan --help` describes `completions` as generation-only, but the implementation now installs by default.

## Issue Context
The behavioral change is in `src/cmd/completions.rs`, while the clap help string lives in the `Commands` enum.

## Fix Focus Areas
- src/cli.rs[55-56]
- src/cmd/completions.rs[36-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Eager home_dir lookup ✓ Resolved 🐞 Bug ☼ Reliability

Description

default_install_path() resolves dirs::home_dir() before matching on shell, so the Nushell case
can fail early even though it only needs dirs::data_dir(). This is an avoidable dependency that
can make the command fail in environments where home_dir() resolution is unavailable but
data_dir() is.

Code

src/cmd/completions.rs[R69-71]

+pub fn default_install_path(shell: CompletionShell) -> Result<PathBuf> {
+    let home = dirs::home_dir().context("Could not resolve home directory")?;
+    Ok(match shell {

Relevance

●● Moderate

Small reliability refactor; similar “avoid unconditional guard” patterns accepted, but not a clear
precedent here.

PR-#67

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The function calls dirs::home_dir() unconditionally, then the Nushell arm separately calls
dirs::data_dir(). Moving the home_dir() call into only the shells that actually use it removes
an unnecessary failure path.

src/cmd/completions.rs[68-92]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`default_install_path()` eagerly resolves `dirs::home_dir()` even for `CompletionShell::Nushell`, which only uses `dirs::data_dir()`.

## Issue Context
This is a small separation-of-concerns bug: it increases the number of ways `numan completions nushell` can fail without providing any benefit.

## Fix Focus Areas
- src/cmd/completions.rs[69-92]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 26 rules

✅ REVIEW.md

Review mode: ⚖️ Balanced: This changes runtime installation behavior across multiple shell-specific paths and filesystem operations, with enough independent logic sites for a careful single-pass review but not an unusually dense multi-pass case.

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[qodo-code-review]

Qodo Fixer

No findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page.

[linear-code]

NUM-78