Skip to content

fix: create output dir (if not exists) for gen_completions command#3519

Open
inteist wants to merge 2 commits into
atuinsh:mainfrom
inteist:main
Open

fix: create output dir (if not exists) for gen_completions command#3519
inteist wants to merge 2 commits into
atuinsh:mainfrom
inteist:main

Conversation

@inteist

@inteist inteist commented Jun 6, 2026

Copy link
Copy Markdown

this follows standard *nix behavior. i.e. if the folder doesn't exist usually in *nix environments in this case, the folder is created automatically.

  • If the directory already exists, it succeeds and does nothing.
  • If parent directories are missing, it creates them.
  • If the path exists but is a file, it returns an error.
  • If permissions prevent creating/accessing it, it returns an error.

Checks

  • [ x] I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • [ x] I have checked that there are no existing pull requests for the same thing

this follows standard *nix behavior. i.e. if the folder doesn't exist
usually in *nix environments in this case, the folder is created
automatically.

 - If the directory already exists, it succeeds and does nothing.
 - If parent directories are missing, it creates them.
 - If the path exists but is a file, it returns an error.
 - If permissions prevent creating/accessing it, it returns an error.
@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Fossier: Manual Review Requested

@inteist is a new contributor. A maintainer should review this PR before merging.

Score Breakdown

Total Score: 65.0/100 | Confidence: 100% | Outcome: REVIEW

Signal Value Score Weight
account_age 6345 1.00 0.09
public_repos 61 1.00 0.05
contribution_history 68 0.34 0.05
follower_ratio 12.0 1.00 0.05
bot_signals False 0.50 0.04
open_prs_elsewhere 1 0.88 0.09
closed_prs_elsewhere 9 0.10 0.10
merged_prs_elsewhere 3 0.68 0.08
prior_interaction 1 0.33 0.08
activity_velocity 0 1.00 0.08
pr_content ... 0.85 0.08
commit_email max@inteist.com 0.80 0.04
pr_description ... 0.70 0.05
repo_stars 30385 0.30 0.04
org_membership 0 0.20 0.03
commit_verification ... 0.65 0.04
contributor_stars 17 0.34 0.04

@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes completion generation create the requested output directory first. The main changes are:

  • Adds fs_err::create_dir_all before writing completion files.
  • Keeps the existing generate_to path for shell-specific completion output.
  • Leaves stdout generation unchanged when no output directory is provided.

Confidence Score: 4/5

This is close, but the directory mode issue should be fixed before merging.

  • The changed path now creates directories while Atuin's restrictive umask is active.

  • Shared completion directories can be created as root-only and become unreadable to normal users.

  • The stdout path and existing-directory path remain unchanged.

  • crates/atuin/src/command/gen_completions.rs

Important Files Changed

Filename Overview
crates/atuin/src/command/gen_completions.rs Adds automatic output directory creation before writing generated shell completions.

Reviews (1): Last reviewed commit: "fix: create output dir (if not exists) f..." | Re-trigger Greptile


match out_dir {
Some(out_dir) => {
fs_err::create_dir_all(&out_dir)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Creates private completion dirs

AtuinCmd::run sets the process umask to 077 before dispatching commands. With this new create_dir_all, atuin gen-completions --out-dir /usr/local/share/bash-completion/completions can create missing shared completion directories as 0700 when run under sudo, so normal users cannot traverse them and shell completion loading breaks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm...

The use case seems a little forced to me.
Most use cases would be to generate completions to STDOUT
We generate completions into a folder to then move it. So IMO this is a non issue.

potential adjustment would be

adding if !matches!(&self, Self::GenCompletions(_)) {

into

impl AtuinCmd {
    pub fn run(self) -> Result<()> {
        #[cfg(not(windows))]
        if !matches!(&self, Self::GenCompletions(_)) {

this needs to be reviewed by someone who better understands various use cases for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant