diff --git a/Makefile b/Makefile index 1369ae1..ec39ea5 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ BINDIR = $(PREFIX)/bin BINARY = ivaldi TARGET = target/release/$(BINARY) -.PHONY: build test install install-extras uninstall clean help +.PHONY: build test install install-extras install-raven-completions uninstall clean help ## Build release binary build: @@ -35,6 +35,14 @@ install-extras: build @$(TARGET) completions fish > $(PREFIX)/share/fish/vendor_completions.d/ivaldi.fish @echo "Installed man pages and completions under $(PREFIX)/share" +## Install the RavenShell completion spec to ~/.config/ravenshell/completions +## (per-user config, so no sudo — kept separate from install-extras) +install-raven-completions: build + @echo "Installing RavenShell completion spec..." + @install -d $(HOME)/.config/ravenshell/completions + @$(TARGET) completions raven > $(HOME)/.config/ravenshell/completions/ivaldi.json + @echo "Installed $(HOME)/.config/ravenshell/completions/ivaldi.json" + ## Uninstall ivaldi from $(PREFIX)/bin uninstall: @echo "Removing $(BINDIR)/$(BINARY)..." @@ -53,6 +61,7 @@ help: @echo " make test Run all tests" @echo " make install Install to $(BINDIR) (may need sudo)" @echo " make install-extras Install man pages and bash/zsh/fish completions (may need sudo)" + @echo " make install-raven-completions Install RavenShell completion spec (no sudo)" @echo " make uninstall Remove from $(BINDIR) (may need sudo)" @echo " make clean Clean build artifacts" @echo "" diff --git a/docs/auth.md b/docs/auth.md index 579ab4e..175f7cd 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -22,12 +22,63 @@ For the GitLab device flow specifically, see [`gitlab.md`](gitlab.md). 1. **Ivaldi OAuth token** (`~/.config/ivaldi/auth.json`) — highest priority 2. **Environment variable** (`GITHUB_TOKEN` / `GITLAB_TOKEN`) 3. **`.netrc` file** -4. **Platform CLI** (`gh auth login` / `glab auth login`) +4. **Platform CLI** (`gh` / `glab`) + +For GitHub, the platform-CLI source asks `gh auth token` directly rather than +parsing `~/.config/gh/hosts.yml`. By default `gh` stores its token in the OS +keyring (macOS Keychain / libsecret) and only writes `hosts.yml` with +`--insecure-storage`, so the old file-parse silently missed most installs — +which pushed Ivaldi into minting its own competing OAuth token. `hosts.yml` +remains a fallback for older `gh` versions. + +## `ivaldi auth login` (GitHub) + +The login command is **reuse-aware** so it does not pile up tokens: + +1. `--with-token` — read a Personal Access Token from stdin and store it, + skipping the browser device flow. **This is the recommended choice for + multi-device use** (see below). +2. Otherwise, if a usable credential already resolves (a valid `gh` / env / + `.netrc` token, or a prior ivaldi token that still validates against + `GET /user`), Ivaldi reuses it and does **not** mint a new token. A stored + ivaldi token that GitHub now rejects is dropped first, so login falls back + to `gh` instead of adding another token. +3. Only when nothing usable exists does Ivaldi run the OAuth device flow. +4. `--force` skips steps 2–3 and always mints a fresh ivaldi token. + +## Multi-device behavior & the 10-token cap + +GitHub limits an OAuth App to **10 tokens per user / application / scope**; +minting an 11th **silently revokes the oldest** +([docs](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps)). +Because Ivaldi's device flow uses the GitHub CLI's public OAuth App with a +fixed scope, every `ivaldi auth login` (and every `gh auth login` / refresh) +spends a slot — so after enough logins across machines, an *older* device gets +logged out. That is the "cross-device auth ping-pong", and refresh tokens do +**not** fix it (OAuth Apps can't issue them, and refreshing rotates one +device's own chain without freeing a slot). + +Mitigations, in order of robustness: + +- **Personal Access Token** (`ivaldi auth login --with-token`): a PAT is + independent of the OAuth-App cap. Paste the **same** fine-grained or classic + PAT (with `repo` scope) on every device and none of them ever evict another. +- **Reuse `gh`**: on machines where `gh` is logged in, Ivaldi now reuses that + one token instead of minting its own, so it stops contributing to the churn. +- **Your own GitHub App** (advanced): register a GitHub App with the device + flow enabled and point Ivaldi at it via `IVALDI_GITHUB_CLIENT_ID`. GitHub + Apps issue expiring access tokens + refresh tokens (no client secret needed + for the device flow), giving each device an independent, self-refreshing + credential — though the 10-token cap still applies beyond 10 devices. ## Token Storage Location: `~/.config/ivaldi/auth.json` (permissions: 0600) +The file is written **atomically with 0600 from creation** (temp file created +mode 0600, then renamed over the target), so the token is never momentarily +world-readable as a plain write-then-chmod would leave it. + ```json { "github": { diff --git a/src/auth.rs b/src/auth.rs index 27c220c..3ab4435 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -162,16 +162,10 @@ impl TokenStore { let data = serde_json::to_string_pretty(&storage).map_err(AuthError::Json)?; - // Write with restricted permissions - fs::write(&self.config_path, &data).map_err(AuthError::Io)?; - - // Set file permissions to 0600 (owner read/write only) - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let perms = fs::Permissions::from_mode(0o600); - fs::set_permissions(&self.config_path, perms).map_err(AuthError::Io)?; - } + // Write atomically with 0600 perms from creation, so the token (and any + // long-lived refresh token) is never world-readable, not even in the + // brief window between create and chmod that a plain write+chmod leaves. + write_secret_file(&self.config_path, data.as_bytes()).map_err(AuthError::Io)?; Ok(()) } @@ -292,6 +286,47 @@ fn home_dir() -> Option { std::env::var("HOME").ok().map(PathBuf::from) } +/// Write `bytes` to `path` atomically with owner-only (0600) permissions. +/// +/// The bytes go to a temp file in the same directory, created with mode 0600 +/// up front (so the secret is never briefly world-readable as a plain +/// `write` + later `chmod` would allow), then renamed over `path`. Readers see +/// either the old or the new contents, never a partial file. +fn write_secret_file(path: &Path, bytes: &[u8]) -> std::io::Result<()> { + use std::io::Write; + + let parent = path.parent().unwrap_or_else(|| Path::new(".")); + let file_name = path.file_name().and_then(|n| n.to_str()).unwrap_or("auth"); + let tmp = parent.join(format!(".{}.tmp.{}", file_name, std::process::id())); + + // A leftover temp from a crashed run could have the wrong perms; start clean + // so the `create_new` below always makes a fresh 0600 file. + let _ = fs::remove_file(&tmp); + + let result = (|| -> std::io::Result<()> { + #[cfg(unix)] + let mut f = { + use std::os::unix::fs::OpenOptionsExt; + fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&tmp)? + }; + #[cfg(not(unix))] + let mut f = fs::File::create(&tmp)?; + + f.write_all(bytes)?; + f.sync_all()?; + fs::rename(&tmp, path) + })(); + + if result.is_err() { + let _ = fs::remove_file(&tmp); + } + result +} + fn read_netrc_token(machine: &str) -> Option { let content = fs::read_to_string(home_dir()?.join(".netrc")).ok()?; let mut in_machine = false; @@ -309,6 +344,17 @@ fn read_netrc_token(machine: &str) -> Option { } fn read_gh_cli_token() -> Option { + // Prefer asking the gh CLI itself. By default gh stores its token in the OS + // keyring (macOS Keychain / libsecret), and only writes it to hosts.yml + // with `--insecure-storage`. Reading the file therefore misses most real + // installs, which previously left ivaldi unable to reuse gh's login and + // pushed it to mint its own competing OAuth token. `gh auth token` prints + // the active token regardless of where gh stored it. + if let Some(token) = gh_auth_token() { + return Some(token); + } + + // Fallback for older gh versions / `--insecure-storage`: parse hosts.yml. let content = fs::read_to_string(home_dir()?.join(".config/gh/hosts.yml")).ok()?; let lines: Vec<&str> = content.lines().collect(); for (i, line) in lines.iter().enumerate() { @@ -322,6 +368,21 @@ fn read_gh_cli_token() -> Option { None } +/// Ask the gh CLI for the active GitHub token, if gh is installed and +/// authenticated. Returns `None` on any failure (gh missing, not logged in, +/// non-zero exit), so callers fall through to other credential sources. +fn gh_auth_token() -> Option { + let output = std::process::Command::new("gh") + .args(["auth", "token", "--hostname", "github.com"]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let token = String::from_utf8(output.stdout).ok()?.trim().to_string(); + if token.is_empty() { None } else { Some(token) } +} + fn read_glab_cli_token() -> Option { let content = fs::read_to_string(home_dir()?.join(".config/glab-cli/config.yml")).ok()?; let lines: Vec<&str> = content.lines().collect(); diff --git a/src/cli/commands.rs b/src/cli/commands.rs index d76de46..9f20461 100644 --- a/src/cli/commands.rs +++ b/src/cli/commands.rs @@ -600,12 +600,96 @@ fn run_patch_session( Ok(staged_paths) } -fn cmd_seal(args: SealArgs, quiet: bool) -> Result<(), String> { - let message = args - .get_message() - .ok_or("seal message required. Usage: ivaldi seal \"your message\"")? - .to_string(); +/// Resolve which editor to launch for composing a seal message. Honors +/// `$VISUAL`, then `$EDITOR`, and falls back to `vim` when neither is set +/// (or is set to an empty/whitespace-only value). +fn resolve_editor() -> String { + std::env::var("VISUAL") + .ok() + .filter(|s| !s.trim().is_empty()) + .or_else(|| std::env::var("EDITOR").ok().filter(|s| !s.trim().is_empty())) + .unwrap_or_else(|| "vim".to_string()) +} + +/// Strip comment lines (those starting with `#`) and surrounding blank lines +/// from an editor buffer, yielding the seal message body. Internal newlines +/// between paragraphs are preserved. +fn strip_message_comments(raw: &str) -> String { + raw.lines() + .filter(|line| !line.starts_with('#')) + .collect::>() + .join("\n") + .trim() + .to_string() +} + +/// Open the user's editor so they can compose a multi-line seal message, the +/// way `git commit` does when no `-m` is given. Writes a template listing the +/// staged changes to `.ivaldi/SEAL_EDITMSG`, launches the editor on it, then +/// reads the result back with comment lines stripped. An empty message aborts +/// the seal. +fn compose_message_via_editor( + ivaldi_dir: &std::path::Path, + ws: &Workspace, +) -> Result { + use std::io::IsTerminal; + + // Launching a full-screen editor only makes sense with a real terminal. + // In a pipe or script, fail loudly so the user reaches for -m instead of + // hanging on an editor that can't draw. + if !std::io::stdin().is_terminal() { + return Err( + "no seal message given and no terminal to open an editor. \ + Pass one with: ivaldi seal -m \"your message\"" + .into(), + ); + } + + let editmsg_path = ivaldi_dir.join("SEAL_EDITMSG"); + // First line is left blank for the message; the rest is commented guidance + // listing what will be sealed. Everything commented is dropped on read. + let mut template = String::from( + "\n# Please enter the message for your seal. Lines starting with '#'\n\ + # will be ignored, and an empty message aborts the seal.\n#\n\ + # Changes to be sealed:\n", + ); + for path in ws.staging.staged_files().keys() { + template.push_str(&format!("#\tchanged: {}\n", path)); + } + for path in ws.staging.staged_deletions() { + template.push_str(&format!("#\tdeleted: {}\n", path)); + } + + std::fs::write(&editmsg_path, &template) + .map_err(|e| format!("could not write seal message template: {e}"))?; + + let editor = resolve_editor(); + let mut parts = editor.split_whitespace(); + let program = parts.next().unwrap_or("vim"); + let status = process::Command::new(program) + .args(parts) + .arg(&editmsg_path) + .status() + .map_err(|e| format!("could not launch editor '{editor}': {e}"))?; + + if !status.success() { + let _ = std::fs::remove_file(&editmsg_path); + return Err(format!("editor '{editor}' exited abnormally; seal aborted")); + } + + let raw = std::fs::read_to_string(&editmsg_path) + .map_err(|e| format!("could not read seal message: {e}"))?; + let _ = std::fs::remove_file(&editmsg_path); + + let message = strip_message_comments(&raw); + if message.is_empty() { + return Err("aborting seal due to empty message".into()); + } + Ok(message) +} + +fn cmd_seal(args: SealArgs, quiet: bool) -> Result<(), String> { let ctx = find_repo()?; let cas = FileCas::new(ctx.ivaldi_dir.join("objects")).map_err(|e| e.to_string())?; let ws = Workspace::new(&cas, &ctx.work_dir, &ctx.ivaldi_dir); @@ -618,6 +702,13 @@ fn cmd_seal(args: SealArgs, quiet: bool) -> Result<(), String> { ); } + // Message comes from the -m/positional arg, or—when omitted—an editor + // session over the staged changes (like `git commit` with no -m). + let message = match args.get_message() { + Some(m) => m.to_string(), + None => compose_message_via_editor(&ctx.ivaldi_dir, &ws)?, + }; + // Open persistent repo first so we can resolve the current timeline's // parent tree, then build the seal tree as parent + staging. let mut repo = open_repo()?; @@ -2952,6 +3043,88 @@ fn cmd_auth(args: AuthArgs) -> Result<(), String> { return Ok(()); } + // PAT path: store a Personal Access Token read from stdin instead of + // running the device flow. PATs are independent per device and are + // immune to GitHub's 10-token-per-OAuth-app eviction, so this is the + // most reliable option when ivaldi is used on many machines. + if login_args.with_token { + use std::io::BufRead; + eprintln!( + "Paste a GitHub Personal Access Token (needs 'repo' scope) and press Enter:" + ); + let mut input = String::new(); + std::io::stdin() + .lock() + .read_line(&mut input) + .map_err(|e| e.to_string())?; + let pat = input.trim().to_string(); + if pat.is_empty() { + return Err("no token provided on stdin".into()); + } + if GitHubClient::with_token(pat.clone()).verify_token() == Some(false) { + return Err( + "GitHub rejected that token. Check it is valid and has 'repo' scope.".into(), + ); + } + let token = crate::auth::Token { + access_token: pat, + token_type: "pat".to_string(), + scope: String::new(), + created_at: std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0), + }; + let store = TokenStore::new().map_err(|e| e.to_string())?; + store + .save_token(Platform::GitHub, token) + .map_err(|e| e.to_string())?; + println!("Stored your GitHub Personal Access Token for this device."); + return Ok(()); + } + + // Reuse-aware: don't mint another OAuth token if a usable one + // already resolves. Every extra token counts against GitHub's + // 10-per-OAuth-app/scope limit and can silently evict an older + // device, which is the root cause of the cross-device logouts. + // `--force` skips this entirely. + if !login_args.force { + // If our own stored token is no longer accepted by GitHub, drop + // it so we can fall back to gh / env / .netrc instead of piling + // on yet another token. Only delete on a definitive rejection, + // never on a transient network error (verify_token -> None). + if let Some(existing) = crate::auth::resolve_auth(Platform::GitHub) + && existing.name == "ivaldi" + && GitHubClient::with_token(existing.token.clone()).verify_token() + == Some(false) + && let Ok(store) = TokenStore::new() + { + let _ = store.delete_token(Platform::GitHub); + } + + // A surviving credential (a valid ivaldi token, or a gh / env / + // .netrc one) means there is nothing to do. + if let Some(existing) = crate::auth::resolve_auth(Platform::GitHub) { + let trustworthy = existing.name != "ivaldi" + || GitHubClient::with_token(existing.token.clone()).verify_token() + != Some(false); + if trustworthy { + println!("Already authenticated with GitHub via {}.", existing.name); + println!(" {}", existing.description); + println!( + "Ivaldi will use this credential automatically — no new login needed." + ); + println!( + "(Run 'ivaldi auth login --force' to mint a separate ivaldi token, or" + ); + println!( + " 'ivaldi auth login --with-token' to paste a Personal Access Token.)" + ); + return Ok(()); + } + } + } + println!("Initiating GitHub authentication..."); let device_code = GitHubClient::request_device_code().map_err(|e| e.to_string())?; @@ -3846,15 +4019,87 @@ fn cmd_peer(args: PeerArgs, _quiet: bool) -> Result<(), String> { /// `ivaldi completions ` — write a completion script to stdout. /// Requires no repository and mutates nothing. fn cmd_completions(args: CompletionsArgs) -> Result<(), String> { - clap_complete::generate( - args.shell, - &mut ::command(), - "ivaldi", - &mut std::io::stdout(), - ); + let mut cmd = ::command(); + match args.shell.clap_shell() { + Some(shell) => { + clap_complete::generate(shell, &mut cmd, "ivaldi", &mut std::io::stdout()); + } + None => { + // RavenShell consumes a JSON completion spec rather than a shell + // script. Build it from the same command tree the other shells use. + let spec = render_raven_spec(&cmd); + let json = serde_json::to_string_pretty(&spec) + .map_err(|e| format!("encode raven completions: {e}"))?; + println!("{json}"); + } + } Ok(()) } +/// Build a RavenShell completion spec from the clap command tree. The result is +/// the JSON that belongs at `~/.config/ravenshell/completions/ivaldi.json`. +/// +/// RavenShell's file format (see RavenShell `completion/spec_file.go`) supports +/// one level of subcommands, each with its own flags and an argument source. +/// Ivaldi's grouped commands (`timeline`, `review`, …) carry a second level, so +/// their sub-subcommand names are emitted as that subcommand's static argument +/// candidates — enough for RavenShell to offer `ivaldi timeline ` → +/// create/switch/…. File completion is suppressed there since those slots take +/// a sub-subcommand, not a path. +fn render_raven_spec(cmd: &clap::Command) -> serde_json::Value { + use serde_json::json; + + let mut subcommands = Vec::new(); + for sub in cmd.get_subcommands().filter(|c| !c.is_hide_set()) { + let mut entry = json!({ "name": sub.get_name() }); + if let Some(about) = sub.get_about() { + entry["desc"] = json!(first_line(&about.to_string())); + } + let flags = raven_flags(sub); + if !flags.is_empty() { + entry["flags"] = json!(flags); + } + let nested: Vec = sub + .get_subcommands() + .filter(|c| !c.is_hide_set()) + .map(|c| { + let desc = c.get_about().map(|a| first_line(&a.to_string())); + json!({ "text": c.get_name(), "desc": desc.unwrap_or_default() }) + }) + .collect(); + if !nested.is_empty() { + entry["args"] = json!({ "static": nested, "noFiles": true }); + } + subcommands.push(entry); + } + + let mut spec = json!({ "subcommands": subcommands }); + let flags = raven_flags(cmd); + if !flags.is_empty() { + spec["flags"] = json!(flags); + } + spec +} + +/// Collect a command's optional `--long` flags as RavenShell `{text, desc}` +/// items. Positional arguments and hidden flags are skipped. +fn raven_flags(cmd: &clap::Command) -> Vec { + use serde_json::json; + cmd.get_arguments() + .filter(|arg| !arg.is_hide_set()) + .filter_map(|arg| { + let long = arg.get_long()?; + let desc = arg.get_help().map(|h| first_line(&h.to_string())); + Some(json!({ "text": format!("--{long}"), "desc": desc.unwrap_or_default() })) + }) + .collect() +} + +/// First line of a (possibly multi-line) help string, for tidy one-line descs. +fn first_line(s: &str) -> String { + s.lines().next().unwrap_or("").to_string() +} + /// `ivaldi man --out DIR` — render `ivaldi.1` plus one `ivaldi-.1` /// page per subcommand into DIR. Requires no repository and mutates nothing. fn cmd_man(args: ManArgs, quiet: bool) -> Result<(), String> { @@ -3892,6 +4137,64 @@ mod tests { use crate::fsmerkle::ChangeKind; use crate::workspace::FileState; + mod editor_message { + use super::super::*; + + #[test] + fn strips_comment_lines_and_trims_blanks() { + let raw = "\nFix the parser\n\nHandle empty input\n# Please enter the message\n#\tchanged: src/parse.rs\n"; + assert_eq!( + strip_message_comments(raw), + "Fix the parser\n\nHandle empty input" + ); + } + + #[test] + fn keeps_hash_not_at_line_start() { + // A literal '#' mid-line (e.g. an issue ref) is not a comment. + assert_eq!(strip_message_comments("Closes issue #42"), "Closes issue #42"); + } + + #[test] + fn all_comments_yields_empty() { + assert_eq!(strip_message_comments("# only\n# comments\n"), ""); + } + + #[test] + fn editor_prefers_visual_then_editor_then_vim() { + // SAFETY: single-threaded test; we restore the env afterward. + let saved_visual = std::env::var_os("VISUAL"); + let saved_editor = std::env::var_os("EDITOR"); + + unsafe { + std::env::remove_var("VISUAL"); + std::env::remove_var("EDITOR"); + } + assert_eq!(resolve_editor(), "vim"); + + unsafe { std::env::set_var("EDITOR", "nano") }; + assert_eq!(resolve_editor(), "nano"); + + unsafe { std::env::set_var("VISUAL", "code --wait") }; + assert_eq!(resolve_editor(), "code --wait"); + + // Whitespace-only values are ignored in favor of the next source. + unsafe { std::env::set_var("VISUAL", " ") }; + assert_eq!(resolve_editor(), "nano"); + + unsafe { + match saved_visual { + Some(v) => std::env::set_var("VISUAL", v), + None => std::env::remove_var("VISUAL"), + } + match saved_editor { + Some(v) => std::env::set_var("EDITOR", v), + None => std::env::remove_var("EDITOR"), + } + } + } + } + mod patch_session { use super::super::*; use crate::cas::Cas; diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 6089a31..c430ed5 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -40,12 +40,15 @@ pub enum Commands { Gather(GatherArgs), /// Create a sealed commit from staged files + #[command(alias = "se")] Seal(SealArgs), /// Redo the most recent seal, folding in staged changes and/or a new message + #[command(alias = "rs")] Reseal(ResealArgs), /// Show repository status + #[command(alias = "st")] Status(StatusArgs), /// Show current timeline and position @@ -53,16 +56,19 @@ pub enum Commands { Whereami, /// View commit history + #[command(alias = "lg")] Log(LogArgs), /// Show, line-by-line, which seal last touched each line of a file - #[command(alias = "blame")] + #[command(aliases = ["blame", "wd"])] Whodidit(WhodiditArgs), /// Compare changes + #[command(alias = "df")] Diff(DiffArgs), /// Remove files from the gathered set (unstage) + #[command(alias = "dc")] Discard(DiscardArgs), /// Throw away all uncommitted changes and restore the working @@ -70,13 +76,15 @@ pub enum Commands { Reverse(ReverseArgs), /// Move the timeline head back to an earlier seal + #[command(alias = "rw")] Rewind(RewindArgs), /// Create a seal that undoes an earlier seal's changes + #[command(alias = "ud")] Undo(UndoArgs), /// Apply another seal's changes to the current timeline - #[command(alias = "cherry-pick")] + #[command(aliases = ["cherry-pick", "pl"])] Pluck(PluckArgs), /// Manage timelines (branches) @@ -84,9 +92,11 @@ pub enum Commands { Timeline(TimelineArgs), /// Merge timelines together + #[command(alias = "fu")] Fuse(FuseArgs), /// Interactive time travel through history + #[command(alias = "tv")] Travel(TravelArgs), /// Combine a range of seals into a single seal (linear history) @@ -94,31 +104,39 @@ pub enum Commands { Weld(WeldArgs), /// View and modify configuration (bare `config` opens an interactive form) - #[command(alias = "configure")] + #[command(aliases = ["configure", "cf"])] Config(ConfigArgs), /// Add patterns to .ivaldiignore + #[command(alias = "ex")] Exclude(ExcludeArgs), /// Manage GitHub/GitLab repository connections + #[command(alias = "pt")] Portal(PortalArgs), /// Authenticate with GitHub/GitLab + #[command(alias = "au")] Auth(AuthArgs), /// Clone a repository from GitHub/GitLab + #[command(alias = "dl")] Download(DownloadArgs), /// Push commits to GitHub/GitLab + #[command(alias = "up")] Upload(UploadArgs), /// Discover remote timelines + #[command(alias = "sc")] Scout(ScoutArgs), /// Download specific remote timelines + #[command(alias = "hv")] Harvest(HarvestArgs), /// Sync current timeline with remote (delta only) + #[command(alias = "sy")] Sync(SyncArgs), /// Local code review system @@ -126,15 +144,19 @@ pub enum Commands { Review(ReviewArgs), /// Open interactive TUI dashboard + #[command(alias = "ui")] Tui, /// Serve the current repo to authorized peers over `ivaldi://` + #[command(alias = "sv")] Serve(ServeArgs), /// Manage authorized peers for `ivaldi serve` + #[command(alias = "pr")] Peer(PeerArgs), /// Generate shell completion script to stdout + #[command(alias = "cmp")] Completions(CompletionsArgs), /// Generate man pages into a directory @@ -165,7 +187,44 @@ pub struct PluckArgs { #[derive(clap::Args, Debug)] pub struct CompletionsArgs { /// Shell to generate completions for - pub shell: clap_complete::Shell, + pub shell: CompletionShell, +} + +/// Shells that `ivaldi completions` can target. +/// +/// The standard shells are rendered by `clap_complete`; `raven` emits a +/// RavenShell JSON completion spec (the file that belongs at +/// `~/.config/ravenshell/completions/ivaldi.json`). +#[derive(Clone, Copy, Debug, PartialEq, Eq, clap::ValueEnum)] +pub enum CompletionShell { + /// Bourne Again SHell + Bash, + /// Elvish shell + Elvish, + /// Friendly Interactive SHell + Fish, + /// PowerShell + #[value(name = "powershell")] + PowerShell, + /// Z SHell + Zsh, + /// RavenShell (JSON completion spec) + Raven, +} + +impl CompletionShell { + /// The matching `clap_complete` shell, or `None` for RavenShell, whose + /// spec is generated separately. + pub fn clap_shell(self) -> Option { + match self { + CompletionShell::Bash => Some(clap_complete::Shell::Bash), + CompletionShell::Elvish => Some(clap_complete::Shell::Elvish), + CompletionShell::Fish => Some(clap_complete::Shell::Fish), + CompletionShell::PowerShell => Some(clap_complete::Shell::PowerShell), + CompletionShell::Zsh => Some(clap_complete::Shell::Zsh), + CompletionShell::Raven => None, + } + } } #[derive(clap::Args, Debug)] @@ -191,14 +250,19 @@ pub struct PeerArgs { #[derive(Subcommand, Debug)] pub enum PeerCommands { /// Trust a peer's pubkey for incoming `ivaldi serve` connections + #[command(alias = "tr")] Trust(PeerTrustArgs), /// List trusted peers + #[command(alias = "ls")] List, /// Remove a trusted peer by pubkey-hex prefix + #[command(alias = "fg")] Forget(PeerForgetArgs), /// Print this user's own pubkey + #[command(alias = "wi")] Whoami, /// Manage TOFU `~/.ivaldi/known_peers` (servers we connect to) + #[command(alias = "kn")] Known(PeerKnownArgs), } @@ -211,8 +275,10 @@ pub struct PeerKnownArgs { #[derive(Subcommand, Debug)] pub enum PeerKnownCommands { /// List known peers (host:port → pubkey) + #[command(alias = "ls")] List, /// Forget a known peer by host[:port] + #[command(alias = "fg")] Forget(PeerKnownForgetArgs), } @@ -459,6 +525,7 @@ pub struct ButterflyArgs { #[derive(Subcommand, Debug)] pub enum ButterflyCommands { /// Create a new butterfly timeline + #[command(alias = "cr")] Create(ButterflyCreateArgs), /// Sync changes up to parent timeline @@ -596,12 +663,15 @@ pub struct PortalArgs { #[derive(Subcommand, Debug)] pub enum PortalCommands { /// Add a remote repository connection + #[command(alias = "ad")] Add(PortalAddArgs), /// List configured portals + #[command(alias = "ls")] List(PortalListArgs), /// Remove a portal + #[command(alias = "rm")] Remove(PortalRemoveArgs), } @@ -641,12 +711,15 @@ pub struct AuthArgs { #[derive(Subcommand, Debug)] pub enum AuthCommands { /// Authenticate with a platform + #[command(alias = "li")] Login(AuthLoginArgs), /// Show authentication status + #[command(alias = "st")] Status, /// Remove stored credentials + #[command(alias = "lo")] Logout(AuthLogoutArgs), } @@ -660,6 +733,18 @@ pub struct AuthLoginArgs { /// Only meaningful with `--gitlab`. #[arg(long)] pub gitlab_host: Option, + + /// Read a Personal Access Token from stdin and store it, instead of + /// running the browser device flow. PATs are independent per device and + /// are NOT subject to GitHub's 10-token-per-OAuth-app eviction, so this is + /// the most reliable choice when you use ivaldi across many machines. + #[arg(long)] + pub with_token: bool, + + /// Authenticate even when a usable credential (gh CLI / env var / an + /// existing login) is already present, minting a separate ivaldi token. + #[arg(long)] + pub force: bool, } #[derive(clap::Args, Debug)] @@ -759,27 +844,35 @@ pub enum ReviewCommands { List(ReviewListArgs), /// Show review details + #[command(alias = "sh")] Show(ReviewShowArgs), /// Show diff for a review + #[command(alias = "df")] Diff(ReviewDiffArgs), /// Add a comment to a review + #[command(alias = "cm")] Comment(ReviewCommentArgs), /// Approve a review + #[command(alias = "ap")] Approve(ReviewApproveArgs), /// Request changes on a review + #[command(alias = "rc")] RequestChanges(ReviewRequestChangesArgs), /// Merge an approved review + #[command(alias = "mg")] Merge(ReviewMergeArgs), /// Close a review without merging + #[command(alias = "cl")] Close(ReviewCloseArgs), /// Reopen a closed review + #[command(alias = "ro")] Reopen(ReviewReopenArgs), } @@ -1459,12 +1552,34 @@ mod tests { let cli = Cli::try_parse_from(["ivaldi", "completions", "fish"]).unwrap(); match cli.command.unwrap() { Commands::Completions(args) => { - assert_eq!(args.shell, clap_complete::Shell::Fish); + assert_eq!(args.shell, CompletionShell::Fish); } _ => panic!("expected Completions"), } } + #[test] + fn parse_completions_raven() { + let cli = Cli::try_parse_from(["ivaldi", "completions", "raven"]).unwrap(); + match cli.command.unwrap() { + Commands::Completions(args) => { + assert_eq!(args.shell, CompletionShell::Raven); + assert!(args.shell.clap_shell().is_none()); + } + _ => panic!("expected Completions"), + } + } + + #[test] + fn parse_completions_powershell_keeps_one_word_value() { + // clap_complete spells it `powershell`; the derived ValueEnum must too. + let cli = Cli::try_parse_from(["ivaldi", "completions", "powershell"]).unwrap(); + match cli.command.unwrap() { + Commands::Completions(args) => assert_eq!(args.shell, CompletionShell::PowerShell), + _ => panic!("expected Completions"), + } + } + #[test] fn completions_fish_generates_nonempty_output() { let mut buf = Vec::new(); @@ -1490,4 +1605,116 @@ mod tests { _ => panic!("expected Review"), } } + + // ---- New short command aliases ---- + + #[test] + fn parse_top_level_short_aliases() { + // A representative sample of the new 2-char aliases. + assert!(matches!( + Cli::try_parse_from(["ivaldi", "se"]).unwrap().command.unwrap(), + Commands::Seal(_) + )); + assert!(matches!( + Cli::try_parse_from(["ivaldi", "st"]).unwrap().command.unwrap(), + Commands::Status(_) + )); + assert!(matches!( + Cli::try_parse_from(["ivaldi", "up"]).unwrap().command.unwrap(), + Commands::Upload(_) + )); + assert!(matches!( + Cli::try_parse_from(["ivaldi", "dl", "owner/repo"]) + .unwrap() + .command + .unwrap(), + Commands::Download(_) + )); + assert!(matches!( + Cli::try_parse_from(["ivaldi", "sy"]).unwrap().command.unwrap(), + Commands::Sync(_) + )); + assert!(matches!( + Cli::try_parse_from(["ivaldi", "lg"]).unwrap().command.unwrap(), + Commands::Log(_) + )); + } + + #[test] + fn short_aliases_do_not_collide_at_top_level() { + // `up` is Upload at the top level; the same string is a Butterfly + // subcommand alias, so they must not interfere across levels. + assert!(matches!( + Cli::try_parse_from(["ivaldi", "up"]).unwrap().command.unwrap(), + Commands::Upload(_) + )); + match Cli::try_parse_from(["ivaldi", "tl", "bf", "up"]) + .unwrap() + .command + .unwrap() + { + Commands::Timeline(t) => match t.command { + TimelineCommands::Butterfly(b) => { + assert!(matches!(b.command, ButterflyCommands::Up)) + } + _ => panic!("expected Butterfly"), + }, + _ => panic!("expected Timeline"), + } + } + + #[test] + fn parse_subcommand_short_aliases() { + // review merge -> `mg` + match Cli::try_parse_from(["ivaldi", "rv", "mg", "5"]) + .unwrap() + .command + .unwrap() + { + Commands::Review(args) => assert!(matches!(args.command, ReviewCommands::Merge(_))), + _ => panic!("expected Review"), + } + // auth status -> `au st` + match Cli::try_parse_from(["ivaldi", "au", "st"]) + .unwrap() + .command + .unwrap() + { + Commands::Auth(args) => assert!(matches!(args.command, AuthCommands::Status)), + _ => panic!("expected Auth"), + } + } + + #[test] + fn parse_auth_login_with_token_and_force() { + match Cli::try_parse_from(["ivaldi", "auth", "login", "--with-token"]) + .unwrap() + .command + .unwrap() + { + Commands::Auth(args) => match args.command { + AuthCommands::Login(l) => { + assert!(l.with_token); + assert!(!l.force); + } + _ => panic!("expected Login"), + }, + _ => panic!("expected Auth"), + } + + match Cli::try_parse_from(["ivaldi", "auth", "login", "--force"]) + .unwrap() + .command + .unwrap() + { + Commands::Auth(args) => match args.command { + AuthCommands::Login(l) => { + assert!(l.force); + assert!(!l.with_token); + } + _ => panic!("expected Login"), + }, + _ => panic!("expected Auth"), + } + } } diff --git a/src/github.rs b/src/github.rs index 6a14be8..cbb1674 100644 --- a/src/github.rs +++ b/src/github.rs @@ -106,6 +106,19 @@ impl GitHubClient { self.token.as_deref() } + /// Best-effort check that GitHub accepts the active token, by calling + /// `GET /user`. Returns `Some(true)` if accepted, `Some(false)` if GitHub + /// rejected it (401), and `None` on a network/other error — so callers can + /// avoid treating a transient outage as a bad token. + pub fn verify_token(&self) -> Option { + self.token.as_ref()?; + match self.get("/user") { + Ok(_) => Some(true), + Err(GitHubError::AuthRequired) => Some(false), + Err(_) => None, + } + } + fn url(&self, path: &str) -> String { if path.starts_with("https://") { path.to_string() diff --git a/src/workspace.rs b/src/workspace.rs index 534e6ee..ed9bf84 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -261,6 +261,8 @@ impl<'a> Workspace<'a> { ) -> Result { let mut gathered = Vec::new(); let mut needs_confirmation = Vec::new(); + // Loaded lazily the first time a directory argument needs expanding. + let mut ignore: Option = None; for &path in paths { // Hard block: security-pattern files can never be staged @@ -273,8 +275,14 @@ impl<'a> Workspace<'a> { continue; } - // Dotfiles need explicit confirmation unless already allowed - let basename = path.rsplit('/').next().unwrap_or(path); + // Dotfiles and dot-directories need explicit confirmation unless + // already allowed. The trailing slash a user may type on a + // directory is trimmed before extracting the basename. + let basename = path + .trim_end_matches('/') + .rsplit('/') + .next() + .unwrap_or(path); if basename.starts_with('.') && basename != ".ivaldiignore" && !allowlist.is_allowed(path) @@ -283,15 +291,20 @@ impl<'a> Workspace<'a> { continue; } - let content = fs::read(&full_path).map_err(WorkspaceError::Io)?; - let canonical = BlobNode::canonical_bytes(&content); - let hash = B3Hash::digest(&canonical); - self.cas - .put(hash, &canonical) - .map_err(WorkspaceError::Cas)?; - on(path); - - self.staging.stage(path, hash); + // Directory arguments expand into the (non-ignored) files beneath + // them, mirroring `gather .`. Without this, the fs::read in + // stage_path would fail with "Is a directory (os error 21)". + if full_path.is_dir() { + let cache = &*ignore + .get_or_insert_with(|| crate::ignore::load_pattern_cache(&self.work_dir)); + for rel in self.expand_dir(path, cache)? { + self.stage_path(&rel, on)?; + gathered.push(rel); + } + continue; + } + + self.stage_path(path, on)?; gathered.push(path.to_string()); } @@ -301,10 +314,45 @@ impl<'a> Workspace<'a> { }) } + /// Expand a directory argument into the workspace-relative paths of the + /// non-ignored files beneath it, sorted. Mirrors `scan`'s traversal so + /// ignore rules and dotfile exclusion apply exactly as for `gather .`. + /// Returns an empty vec when the directory itself is ignored. + fn expand_dir( + &self, + rel_dir: &str, + cache: &PatternCache, + ) -> Result, WorkspaceError> { + let rel_dir = rel_dir.trim_end_matches('/'); + if cache.is_dir_ignored(rel_dir) { + return Ok(Vec::new()); + } + let mut files = Vec::new(); + self.scan_dir(&self.work_dir.join(rel_dir), rel_dir, cache, &mut files)?; + files.sort(); + Ok(files) + } + + /// Read a single file at workspace-relative `rel`, store its canonical + /// blob in the CAS, and record it in the staging area. `on` is invoked + /// with `rel` right after the blob is stored (drives progress bars). + fn stage_path(&mut self, rel: &str, on: &mut dyn FnMut(&str)) -> Result<(), WorkspaceError> { + let full_path = self.work_dir.join(rel); + let content = fs::read(&full_path).map_err(WorkspaceError::Io)?; + let canonical = BlobNode::canonical_bytes(&content); + let hash = B3Hash::digest(&canonical); + self.cas.put(hash, &canonical).map_err(WorkspaceError::Cas)?; + on(rel); + self.staging.stage(rel, hash); + Ok(()) + } + /// Stage specific files unconditionally (used after user confirms dotfiles). /// Still rejects security-blocked files. pub fn gather_confirmed(&mut self, paths: &[&str]) -> Result, WorkspaceError> { let mut gathered = Vec::new(); + // Loaded lazily the first time a confirmed directory needs expanding. + let mut ignore: Option = None; for &path in paths { if crate::ignore::is_security_blocked(path) { return Err(WorkspaceError::SecurityBlocked(path.to_string())); @@ -315,14 +363,19 @@ impl<'a> Workspace<'a> { continue; } - let content = fs::read(&full_path).map_err(WorkspaceError::Io)?; - let canonical = BlobNode::canonical_bytes(&content); - let hash = B3Hash::digest(&canonical); - self.cas - .put(hash, &canonical) - .map_err(WorkspaceError::Cas)?; + // A confirmed directory (e.g. a dot-directory the user approved at + // the prompt) expands the same way as in `gather_with_progress`. + if full_path.is_dir() { + let cache = &*ignore + .get_or_insert_with(|| crate::ignore::load_pattern_cache(&self.work_dir)); + for rel in self.expand_dir(path, cache)? { + self.stage_path(&rel, &mut |_| {})?; + gathered.push(rel); + } + continue; + } - self.staging.stage(path, hash); + self.stage_path(path, &mut |_| {})?; gathered.push(path.to_string()); } Ok(gathered) @@ -1077,6 +1130,50 @@ mod tests { assert_eq!(cas.len(), 1); // Content stored in CAS } + #[test] + fn gather_expands_directory_argument() { + let (dir, cas) = setup_workspace(); + fs::create_dir_all(dir.path().join("solutions/01_variables")).unwrap(); + fs::write(dir.path().join("solutions/a.oxi"), "a").unwrap(); + fs::write(dir.path().join("solutions/01_variables/b.oxi"), "b").unwrap(); + // A sibling file outside the directory must not be gathered. + fs::write(dir.path().join("outside.oxi"), "x").unwrap(); + + let allowlist = empty_allowlist(&dir); + let mut ws = Workspace::new(&cas, dir.path(), dir.path().join(".ivaldi")); + // Trailing slash, as a user would type it, must not corrupt paths. + let result = ws.gather(&["solutions/"], &allowlist).unwrap(); + + assert_eq!( + result.gathered, + vec!["solutions/01_variables/b.oxi", "solutions/a.oxi"] + ); + assert!(ws.staging.is_staged("solutions/a.oxi")); + assert!(ws.staging.is_staged("solutions/01_variables/b.oxi")); + assert!(!ws.staging.is_staged("outside.oxi")); + } + + #[test] + fn gather_dot_directory_needs_confirmation() { + let (dir, cas) = setup_workspace(); + fs::create_dir_all(dir.path().join(".github/workflows")).unwrap(); + fs::write(dir.path().join(".github/workflows/ci.yml"), "ci").unwrap(); + + let allowlist = empty_allowlist(&dir); + let mut ws = Workspace::new(&cas, dir.path(), dir.path().join(".ivaldi")); + let result = ws.gather(&[".github/"], &allowlist).unwrap(); + + // A dot-directory is held for confirmation, not expanded outright. + assert!(result.gathered.is_empty()); + assert_eq!(result.needs_confirmation, vec![".github/"]); + assert!(!ws.staging.is_staged(".github/workflows/ci.yml")); + + // Confirming expands it, staging the non-dot files beneath. + let confirmed = ws.gather_confirmed(&[".github/"]).unwrap(); + assert_eq!(confirmed, vec![".github/workflows/ci.yml"]); + assert!(ws.staging.is_staged(".github/workflows/ci.yml")); + } + #[test] fn gather_with_progress_fires_callback_per_file() { let (dir, cas) = setup_workspace();