diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a45ced2..ab0e8945 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,48 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 a `default/` directory, so a picker has something to select and a recalled line has a way to say "not the profile I used last time". +- **`dl --claude-profiles` lists those logins with the account behind each one**, because + a profile's name is chosen by a person and verified by nothing. A profile called `work` + holding a personal login reads as correct right up until work is pushed from the wrong + identity, which is the failure profiles exist to prevent; the name is what you type and + the account column is what you get. Read from the three fields of `.claude.json` worth + showing (email, organisation, seat tier), and it distinguishes a profile with no + credential from one whose state file says nothing. + + **Two names for one directory are one login, not a spare copy of one.** The + redundancy footnote says "all but one are spare", so a group has to mean separate + directories you could delete one of. `CLAUDE_CONFIG_DIR=~/.claude-profiles/work` + makes the `default` row and the `work` row the same directory, the same state file + and the same account id, and grouping on the id alone named the only login on the + host; a symlinked profile directory got there the same way, since the walk follows + symlinks. A directory is now counted once, so both rows are still listed with the + same account against each and no deletion is advised. `path` is not in the listing, + which is why nothing on screen let a reader catch this. + + **A profiles directory that cannot be read says so** instead of listing as a host + with no profiles. Only `NotFound` is silent, because a root nothing has created yet + is the ordinary state of most hosts; a root that is unreadable, or a plain file where + the directory should be, told a host with five profiles that it had none. + + **No token is read to build it.** The `authed` column is the credential file's + existence and never its contents, so a listing has not touched a secret. It is also + the way to find a profile created and never logged in to, since a launch naming one of + those refuses. + + It also names the profiles that are **two names for one account**, which is the other + thing a name cannot tell you: two profiles of one account render identically to two + colleagues who share an organisation, so the redundant one is invisible exactly where + you are choosing between them. Grouped on the account's own id and never on a display + field, since a shared organisation is two people; a profile naming no account joins no + group, because two blanks are not the same account. Said once per group as a footnote + rather than per row as a column, because it is a fact about a pair. + + A profile name beginning with a dot is now refused as well as unlisted. A `/*/` + glob matches no dot-directory, so neither the listing nor the completion would ever + show one, and a profile you can launch but never see is a trap. That is marginally + stricter than the `^[A-Za-z0-9._-]+$` the managing tool validates with, and it makes + the resolver, the listing and the completion agree. + **A named profile that holds no credential stops the launch, and that refusal is the feature.** It does not fall back to the default login. Two accounts on one machine is what profiles are for, so a typo that silently forwarded the other one would be worse diff --git a/README.md b/README.md index 03ce007f..b3e47bf9 100644 --- a/README.md +++ b/README.md @@ -258,6 +258,7 @@ instead. [docs/cli.md](docs/cli.md) has the full `--rm` contract, including whic | `dl --purge` | Remove devlaunch's own workspaces and caches | | `dl --install` | Install shell completions, and the `dl-herdr-shell` name a herdr pane opens through | | `dl --refresh` | Rebuild the completion cache now | +| `dl --claude-profiles` | List the Claude logins `--claude-profile` can name, and the account each is signed in as | | `dl --version` | Print the version | | `dl --herdr-shell` | The shell a new [herdr](https://herdr.dev) pane opens: inside the workspace its tab holds, or on this host | | `dl --help`, `-h` | Print help | diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index 639f942f..6de7ed7d 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -150,6 +150,47 @@ next. a `default/` directory. It exists as a word because a picker needs something to select, and a recalled line needs a way to say "not the profile I used last time". +### Seeing which account a profile actually holds + +``` +$ dl --claude-profiles +NAME STATE ACCOUNT +default authed someone@example.com · Someorg · team_tier_1 +work authed someone@work.example · Workorg · team_tier_1 +spare authed someone@example.com · Someorg · team_tier_1 +fresh not logged in - + +'default', 'spare' are the same account, so all but one are spare. +``` + +That last line is the other thing a name cannot tell you. Two profiles of one +account render **identically** to two colleagues who share an organisation, so the +redundant one is invisible exactly where you are choosing between them. It is decided +on the account's own id and never on a display field, because a shared organisation is +two people and a shared name is nothing at all. A profile whose state file names no +account joins no group, so no claim is made about a blank. + +**A profile's name is chosen by you and verified by nothing**, which is the reason +this listing exists. A profile called `work` holding a personal login reads as correct +right up until the work is pushed from the wrong identity, and that is the failure +profiles are meant to prevent. The name is what you type; the account column is what +you get. + +The account is read from `.claude.json`, the state file Claude Code keeps inside each +config directory, and only three of its fields: the email address, the organisation +and the seat tier. Nothing else is read and nothing is written. A `-` is a profile +with no credential and so nobody to name; `unknown` is one that is logged in whose +state file is absent or has moved on from the shape this reads, which stops nothing. + +**No token is read to build this.** The `authed` column is the credential file's +existence, never its contents, so a listing has not touched a secret. +`--claude-profiles` is also the honest way to find a profile created and never logged +in to, since a launch naming one of those refuses. + +A name beginning with a dot is neither offered nor accepted. A `/*/` glob does +not match a dot-directory, so neither this listing nor the shell completion would show +one, and a profile you can launch but never see is a trap rather than a feature. + **A named profile that holds no credential stops the launch.** It does not fall back to your default login, and that refusal is the feature rather than a rough edge. Two accounts on one machine is what profiles are for, so a typo that silently forwarded diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index 8db296f9..db7219d1 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -65,7 +65,7 @@ _dl_completion() { # The retired spellings (--stop, --autorm) are absent by rule rather than by # hand: the grammar marks them `hide = true`, and the test drops every hidden # flag, so a spelling this build only still answers for is never offered. - local global_opts="--ls --install --refresh --prune --reconcile --purge --herdr-shell --rm --devcontainer --claude-profile --help -h --version" + local global_opts="--ls --install --refresh --prune --reconcile --purge --herdr-shell --rm --devcontainer --claude-profile --claude-profiles --help -h --version" if [[ "$cmd" == aid ]]; then global_opts="--claude --codex --gemini --devcontainer --claude-profile --help -h --version" fi @@ -94,12 +94,32 @@ _dl_completion() { # its default directory. `default` is offered because it is a name the resolver # answers for without any directory existing. local profiles_root="${DEVLAUNCH_CLAUDE_PROFILES_DIR:-${CLAUDE_PROFILES_DIR:-$HOME/.claude-profiles}}" - local profiles="default" pdir + local profiles="default" pdir pname if [[ -d "${profiles_root}" ]]; then for pdir in "${profiles_root}"/*/; do [[ -d "$pdir" ]] || continue pdir="${pdir%/}" - profiles+=" ${pdir##*/}" + pname="${pdir##*/}" + # `ProfileName::parse`'s grammar, a second time: one directory + # component of ASCII letters, digits, '.', '_' and '-', not starting + # with '.' or '-'. Offering more than that is offering a completion the + # launch refuses -- press tab, get `-flag` or `my profile`, and the + # refusal is about a name you did not type by hand. + # + # The glob already hides the leading dot (it matches no dot-directory), + # so the visible half of this is the leading '-' and the character set. + # Both are checked anyway rather than relying on the glob, because the + # rule is what has to agree and not the accident that enforces part of + # it. `test_the_completion_offers_only_names_a_launch_accepts` in + # test_bash_completion.py is the diff that keeps the two in step. + # The character set is spelled out rather than written as ranges, + # and that is not fussiness: `[[ =~ ]]` honours LC_COLLATE, so + # `[A-Za-z]` matches `é` in a UTF-8 locale and this offered + # `unicode-é` while `ProfileName::parse` -- which asks + # `is_ascii_alphanumeric` -- refuses it. The test below caught it. + [[ "$pname" =~ ^[abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.-]+$ ]] || continue + [[ "$pname" == [-.]* ]] && continue + profiles+=" ${pname}" done fi COMPREPLY=( $(compgen -W "${profiles}" -- ${cur}) ) diff --git a/rust/devlaunch-core/public-api.rest.txt b/rust/devlaunch-core/public-api.rest.txt index 11c38147..dd3a9f84 100644 --- a/rust/devlaunch-core/public-api.rest.txt +++ b/rust/devlaunch-core/public-api.rest.txt @@ -1093,6 +1093,53 @@ pub fn devlaunch_core::flows::branch_manager::BranchError::eq(&self, &devlaunch_ impl core::fmt::Debug for devlaunch_core::flows::branch_manager::BranchError pub fn devlaunch_core::flows::branch_manager::BranchError::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::branch_manager::BranchError +pub mod devlaunch_core::flows::claude_profiles +pub enum devlaunch_core::flows::claude_profiles::ProfileState +pub devlaunch_core::flows::claude_profiles::ProfileState::Authed +pub devlaunch_core::flows::claude_profiles::ProfileState::NoCredential +impl core::clone::Clone for devlaunch_core::flows::claude_profiles::ProfileState +pub fn devlaunch_core::flows::claude_profiles::ProfileState::clone(&self) -> devlaunch_core::flows::claude_profiles::ProfileState +impl core::cmp::Eq for devlaunch_core::flows::claude_profiles::ProfileState +impl core::cmp::PartialEq for devlaunch_core::flows::claude_profiles::ProfileState +pub fn devlaunch_core::flows::claude_profiles::ProfileState::eq(&self, &devlaunch_core::flows::claude_profiles::ProfileState) -> bool +impl core::fmt::Debug for devlaunch_core::flows::claude_profiles::ProfileState +pub fn devlaunch_core::flows::claude_profiles::ProfileState::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for devlaunch_core::flows::claude_profiles::ProfileState +impl core::marker::StructuralPartialEq for devlaunch_core::flows::claude_profiles::ProfileState +pub struct devlaunch_core::flows::claude_profiles::Account +pub devlaunch_core::flows::claude_profiles::Account::account_uuid: core::option::Option +pub devlaunch_core::flows::claude_profiles::Account::email: core::option::Option +pub devlaunch_core::flows::claude_profiles::Account::organization: core::option::Option +pub devlaunch_core::flows::claude_profiles::Account::seat_tier: core::option::Option +impl devlaunch_core::flows::claude_profiles::Account +pub fn devlaunch_core::flows::claude_profiles::Account::is_empty(&self) -> bool +impl core::clone::Clone for devlaunch_core::flows::claude_profiles::Account +pub fn devlaunch_core::flows::claude_profiles::Account::clone(&self) -> devlaunch_core::flows::claude_profiles::Account +impl core::cmp::Eq for devlaunch_core::flows::claude_profiles::Account +impl core::cmp::PartialEq for devlaunch_core::flows::claude_profiles::Account +pub fn devlaunch_core::flows::claude_profiles::Account::eq(&self, &devlaunch_core::flows::claude_profiles::Account) -> bool +impl core::default::Default for devlaunch_core::flows::claude_profiles::Account +pub fn devlaunch_core::flows::claude_profiles::Account::default() -> devlaunch_core::flows::claude_profiles::Account +impl core::fmt::Debug for devlaunch_core::flows::claude_profiles::Account +pub fn devlaunch_core::flows::claude_profiles::Account::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::claude_profiles::Account +pub struct devlaunch_core::flows::claude_profiles::ProfileSummary +pub devlaunch_core::flows::claude_profiles::ProfileSummary::account: core::option::Option +pub devlaunch_core::flows::claude_profiles::ProfileSummary::name: alloc::string::String +pub devlaunch_core::flows::claude_profiles::ProfileSummary::path: std::path::PathBuf +pub devlaunch_core::flows::claude_profiles::ProfileSummary::shares_account_with: alloc::vec::Vec +pub devlaunch_core::flows::claude_profiles::ProfileSummary::state: devlaunch_core::flows::claude_profiles::ProfileState +impl core::clone::Clone for devlaunch_core::flows::claude_profiles::ProfileSummary +pub fn devlaunch_core::flows::claude_profiles::ProfileSummary::clone(&self) -> devlaunch_core::flows::claude_profiles::ProfileSummary +impl core::cmp::Eq for devlaunch_core::flows::claude_profiles::ProfileSummary +impl core::cmp::PartialEq for devlaunch_core::flows::claude_profiles::ProfileSummary +pub fn devlaunch_core::flows::claude_profiles::ProfileSummary::eq(&self, &devlaunch_core::flows::claude_profiles::ProfileSummary) -> bool +impl core::fmt::Debug for devlaunch_core::flows::claude_profiles::ProfileSummary +pub fn devlaunch_core::flows::claude_profiles::ProfileSummary::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::claude_profiles::ProfileSummary +pub const devlaunch_core::flows::claude_profiles::DEFAULT_PROFILE: &str +pub fn devlaunch_core::flows::claude_profiles::from_process() -> alloc::vec::Vec +pub fn devlaunch_core::flows::claude_profiles::summarise(core::option::Option<&std::path::Path>, core::option::Option<&std::path::Path>) -> alloc::vec::Vec pub mod devlaunch_core::flows::completion pub enum devlaunch_core::flows::completion::FileState pub devlaunch_core::flows::completion::FileState::AlreadyCurrent diff --git a/rust/devlaunch-core/src/clients/claude.rs b/rust/devlaunch-core/src/clients/claude.rs index b3267a3d..b3c59768 100644 --- a/rust/devlaunch-core/src/clients/claude.rs +++ b/rust/devlaunch-core/src/clients/claude.rs @@ -45,13 +45,11 @@ //! symptom: a host that had moved its configuration reported [`NoToken::NotLoggedIn`] //! while holding a perfectly good login. //! -//! The order is opt-out, then any exported `CLAUDE_CODE_OAUTH_TOKEN`, then one -//! credential file: `$CLAUDE_CONFIG_DIR`'s when the variable is set and -//! `$HOME/.claude`'s when it is not. Those last two are one step and not two, which -//! is the whole of [`config_dir`]'s argument and worth spelling here because a list -//! of four reads as a fallback chain. The exported token stays above the variable -//! because both are ambient and that hatch is what lets a `dl` inside a workspace -//! forward what it was handed. +//! The order is opt-out, then any exported `CLAUDE_CODE_OAUTH_TOKEN`, then +//! `$CLAUDE_CONFIG_DIR`, then `$HOME/.claude`. The exported token stays above the +//! variable because both are ambient and that hatch is what lets a `dl` inside a +//! workspace forward what it was handed; see [`config_dir`] for why the variable +//! *replaces* the default rather than being tried ahead of it. //! //! It also means a session devlaunch did not open — `devpod ssh` by hand, VS Code //! through `dl code` — does not get it. Those already have the host's real @@ -99,6 +97,23 @@ const CREDENTIALS_FILENAME: &str = ".credentials.json"; const OAUTH_KEY: &str = "claudeAiOauth"; const ACCESS_TOKEN_KEY: &str = "accessToken"; +/// Claude Code's own account and state file, beside the credential in a config +/// directory. Read for a *label*, never for a credential: nothing in it is secret and +/// nothing in it is forwarded anywhere. +const ACCOUNT_FILENAME: &str = ".claude.json"; + +/// The object in [`ACCOUNT_FILENAME`] describing who is signed in, and the three +/// fields worth showing a person choosing between profiles. +const ACCOUNT_KEY: &str = "oauthAccount"; +const EMAIL_KEY: &str = "emailAddress"; +/// Not shown to anyone: an opaque id is noise in a table. Read so that two profiles +/// holding one account can be *said* to, which no pair of display fields proves -- +/// two logins of one organisation share an `organizationName` and are still different +/// accounts. +const ACCOUNT_UUID_KEY: &str = "accountUuid"; +const ORGANIZATION_KEY: &str = "organizationName"; +const SEAT_KEY: &str = "seatTier"; + /// A value that has the shape a Claude OAuth token has. /// /// Its own type for the reason [`super::gh::Token`] has one: the check belongs at @@ -149,24 +164,27 @@ impl Token { pub(crate) struct ProfileName(String); impl ProfileName { - /// `raw` if it can be a leaf directory name, else nothing. + /// `raw` if it can be a leaf directory name worth offering, else nothing. + /// + /// The same flat-ASCII set [`Token::parse`] accepts, and two leading characters + /// refused outright: /// - /// The same flat-ASCII set [`Token::parse`] accepts, minus anything that is not a - /// plain visible leaf: empty, and a leading `.` or `-`. A separator of either kind - /// and a NUL are excluded by the set itself. + /// - **`-`**, because a name that looks like a flag reads as one everywhere it is + /// later printed or passed on. + /// - **`.`**, which covers `.` and `..` without special-casing them and settles a + /// disagreement three places were having. A glob of `/*/` does not match a + /// dot-directory, so neither the shell completion nor the tool that manages the + /// directory lists one, while this check used to accept it: `--claude-profile + /// .hidden` was a profile you could launch and never see offered. A hidden + /// profile is a trap rather than a feature, and one rule here makes the resolver, + /// the listing and the completion agree. /// - /// A leading `-` goes because a name that looks like a flag reads as one everywhere - /// it is later printed or passed on. A leading `.` goes for two reasons at once: it - /// subsumes `.` and `..`, which are the spellings that would climb out of the root, - /// and it keeps the rule one clause a refusal can state. `.work` was accepted - /// before this and read from `/.work/`, so `--claude-profile 'has space'` was - /// refused with "cannot begin with '.' or '-'" -- a sentence the validator did not - /// enforce. Refusing a hidden directory costs nothing: nothing creates one, and a - /// profile is a thing a person types. + /// Slightly stricter than the `^[A-Za-z0-9._-]+$` the managing tool validates with, + /// which accepts a leading dot it then never lists. Refusing by a named rule beats + /// honouring a name nothing shows you. pub(crate) fn parse(raw: &str) -> Option { let flat = !raw.is_empty() - && !raw.starts_with('-') - && !raw.starts_with('.') + && !raw.starts_with(['-', '.']) && raw .chars() .all(|c| c.is_ascii_alphanumeric() || matches!(c, '_' | '.' | '-')); @@ -202,9 +220,8 @@ pub(crate) struct HostEnv { /// here: a path is bytes, so a directory whose name is not valid UTF-8 would /// arrive with U+FFFD where those bytes were, fail to open, and report /// [`NoToken::NotLoggedIn`] on a host holding a perfectly good login. That is the - /// symptom this whole change removes, so reading it back in would be the same - /// defect wearing the fix's clothes. [`crate::domain::xdg`]'s `resolve` takes an - /// `OsString` for the same reason, and it is the rule this arm already cites. + /// symptom honouring this variable exists to remove. [`crate::domain::xdg`]'s + /// `resolve` takes an `OsString` for the same reason. pub(crate) config_dir: Option, /// `--claude-profile`, if one was typed. /// @@ -386,6 +403,116 @@ fn token_from_credentials(text: &str) -> Option { Token::parse(parsed.get(OAUTH_KEY)?.get(ACCESS_TOKEN_KEY)?.as_str()?) } +/// Who a Claude config directory is signed in as. +/// +/// **A label, not a credential.** The whole reason this type exists is that a profile's +/// directory name is chosen by a person and verified by nothing: a profile called +/// `work` holding a personal login is indistinguishable from a correct one until +/// something reads the account out. Every field is optional because the file is Claude +/// Code's, gains keys on its own schedule, and may be absent entirely in a profile +/// that has been created but never logged in to. +/// +/// Not redacted, unlike [`Token`], and deliberately: an email address is identity +/// rather than a secret, it is what makes the label worth printing, and it never +/// leaves the host. Nothing here is ever forwarded into a container. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct Account { + pub email: Option, + pub organization: Option, + pub seat_tier: Option, + /// The account's own id, for telling two profiles of one account apart from two + /// profiles that merely look alike. Never displayed. + pub account_uuid: Option, +} + +impl Account { + /// Whether anything **worth showing** was learned, so a caller can tell "signed in + /// as somebody I cannot name" from "signed in as nobody". + /// + /// Deliberately blind to [`Self::account_uuid`]: an id alone renders as an empty + /// column, so a file carrying only that is no better than a file carrying nothing, + /// and claiming two such profiles are "the same account" would be a claim about + /// two blanks. + pub fn is_empty(&self) -> bool { + self.email.is_none() && self.organization.is_none() && self.seat_tier.is_none() + } +} + +/// The account a config directory names, if its state file says. +/// +/// `None` covers every ordinary absence at once, because they are one answer to a +/// caller drawing a table: no state file (a profile made and never logged in to), a +/// file that is not JSON, or a file with no `oauthAccount`. Deliberately not a +/// `Deserialize` struct over the whole file, for [`token_from_credentials`]'s reason: +/// the file belongs to Claude Code and has 70-odd keys this does not read. +pub(crate) fn account_at(config_dir: &Path) -> Option { + let text = std::fs::read_to_string(config_dir.join(ACCOUNT_FILENAME)).ok()?; + let parsed: serde_json::Value = serde_json::from_str(&text).ok()?; + let account = parsed.get(ACCOUNT_KEY)?; + let field = |key: &str| { + account + .get(key) + .and_then(serde_json::Value::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_owned) + }; + let found = Account { + email: field(EMAIL_KEY), + organization: field(ORGANIZATION_KEY), + seat_tier: field(SEAT_KEY), + account_uuid: field(ACCOUNT_UUID_KEY), + }; + (!found.is_empty()).then_some(found) +} + +/// The config directory the *unnamed* login resolves to on this machine. +/// +/// `$CLAUDE_CONFIG_DIR`, else `$HOME/.claude`, else nothing on a host that names +/// neither, which is a real state: `dl` runs with `XDG_CACHE_HOME` set and no home. +/// +/// Reads the process environment, unlike everything else here that takes a +/// [`HostEnv`], and the exception is the caller: a listing is a read-only command with +/// no `Host` to hand, and the alternative is exporting `HostEnv` and `config_dir` so a +/// binary can rebuild an answer this module already knows. +pub(crate) fn unnamed_config_dir_from_process() -> Option { + config_dir( + crate::osext::home_dir().as_deref(), + &HostEnv::from_process(), + ) +} + +/// The word a listing offers for the unnamed credential. +/// +/// A reader of [`DEFAULT_PROFILE`] for the one test that diffs it against +/// [`crate::flows::claude_profiles::DEFAULT_PROFILE`]. Test-only on purpose: the +/// listing spells its own row name and this exists so the two cannot drift, not so +/// anything reads the name through here at runtime. +#[cfg(test)] +pub(crate) fn default_profile_name() -> &'static str { + DEFAULT_PROFILE +} + +/// Whether a directory name is one a listing should offer. +/// +/// The listing and the launch must not disagree about what a profile is, so this is +/// asked of the same [`ProfileName`] check the launch applies, plus the one exclusion +/// only a listing needs: `default` is a name the resolver answers for **without** +/// consulting a directory, so a directory of that name is not a profile and offering +/// it would offer a launch that ignores it. +pub(crate) fn profile_name_is_offerable(name: &str) -> bool { + name != DEFAULT_PROFILE && ProfileName::parse(name).is_some() +} + +/// Whether a config directory holds a credential at all. +/// +/// The distinction a listing needs and [`resolve_token`] does not: a profile directory +/// that exists with no credential in it is the ordinary state right after something +/// created it, and saying "not logged in" is more use than saying nothing. +pub(crate) fn has_credential(config_dir: &Path) -> bool { + config_dir.join(CREDENTIALS_FILENAME).is_file() +} + /// Add the Claude login to the flags and environment a session is opened with. /// /// Extends rather than replaces, because a session already carries @@ -685,14 +812,12 @@ mod tests { "/etc", "..\\windows", "-flag", + // A dot-directory is refused rather than being a profile nothing lists: + // a `/*/` glob does not match one, so the completion and the + // managing tool never offer it. + ".hidden", "has space", "n\u{0}ul", - // A hidden directory, refused so that the one sentence a refusal prints - // is true of every name it refuses. `.work` used to be accepted and read - // from `/.work/`, while `dl` told anyone refused for an unrelated - // reason that a name "cannot begin with '.' or '-'". - ".work", - ".hidden", ] { let host = HostEnv { profile: Some(named.to_owned()), @@ -888,8 +1013,8 @@ mod tests { // A path is bytes. Reading this variable through `osext::env_str`, which // decodes lossily on purpose for the switches either side of it, put U+FFFD // where the undecodable byte was and then failed to open the result -- so a - // host with a login reported NotLoggedIn, which is the exact symptom this - // change exists to remove. + // host with a login reported NotLoggedIn, which is the exact symptom honouring + // the variable exists to remove. use std::os::unix::ffi::OsStrExt as _; let parent = tempfile::tempdir().expect("a scratch parent"); diff --git a/rust/devlaunch-core/src/flows/claude_profiles.rs b/rust/devlaunch-core/src/flows/claude_profiles.rs new file mode 100644 index 00000000..1bebe1b1 --- /dev/null +++ b/rust/devlaunch-core/src/flows/claude_profiles.rs @@ -0,0 +1,532 @@ +//! The Claude logins this host can forward, as a person would want them listed. +//! +//! `--claude-profile ` picks one of these +//! ([`crate::clients::claude::resolve_token`]); this is the read side, and it exists +//! for one reason. **A profile's directory name is chosen by a person and verified by +//! nothing.** A profile called `work` holding a personal login is indistinguishable +//! from a correct one until something reads the account out of it, and the failure it +//! causes is the exact failure profiles exist to prevent: work pushed from the wrong +//! identity, found out about later and somewhere else. +//! +//! So a row is a name *and* the account behind it, read from the state file Claude +//! Code keeps beside the credential. The name is what you type; the account is what +//! you get. +//! +//! # What this deliberately does not do +//! +//! **No writer, and none is coming.** Creating a profile, seeding the configuration it +//! shares with the main login, and deleting it belong to whatever manages the +//! directory ([`crate::domain::xdg::claude_profiles_root`] names the arrangement). +//! This enumerates and reads. +//! +//! **No token is read.** [`ProfileState`] answers "is there a credential here" from +//! the file's existence, never from its contents, so nothing in a listing has touched +//! a secret. That is a smaller claim than it sounds and worth keeping: a listing is +//! the surface most likely to grow a `--json` and end up somewhere it should not. + +use std::path::{Path, PathBuf}; + +use crate::clients::claude; + +/// The account behind a profile, at a path a caller outside this crate can name. +/// +/// [`ProfileSummary::account`] is a public field of this type and `clients::claude` is +/// `pub(crate)`, so without the re-export the field is readable while its type is not +/// nameable: a consumer can reach `summary.account.email` and cannot write the type of +/// what they are holding, or a function that takes one. `dl` never noticed because it +/// only ever reaches through the field. +/// +/// Re-exported here rather than by making `clients::claude` public, which would put the +/// token machinery on the same surface for no reason. +pub use crate::clients::claude::Account; + +/// The name that means "the login this host uses anyway". +/// +/// Spelled here as well as in [`crate::clients::claude`] because a listing has to +/// offer it as a row and the resolver has to answer for it, and +/// `the_default_row_is_the_name_the_resolver_answers_for` diffs the two rather than +/// leaving one to drift. +pub const DEFAULT_PROFILE: &str = "default"; + +/// Whether a profile can be launched with. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ProfileState { + /// A credential file is there. Whether it still *works* is a question only the + /// server can answer, and an access token lives hours, so this is deliberately + /// "has one" rather than "has a valid one". + Authed, + /// The directory exists and holds no credential: the ordinary state immediately + /// after something created it and before anyone logged in. Naming it is the whole + /// point, since a launch naming this profile refuses. + NoCredential, +} + +/// One row of the listing. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProfileSummary { + /// What you type after `--claude-profile`. + pub name: String, + /// The config directory it names, for a reader who needs to go and look. + pub path: PathBuf, + pub state: ProfileState, + /// Who it is signed in as, when the state file says. `None` is not an error: a + /// profile never logged in to has none, and neither does one whose file Claude + /// Code has since reshaped. + pub account: Option, + /// The other profiles in this listing signed in as the **same account**, by name + /// and sorted. Empty for a profile that is the only one of its account, which is + /// every profile on a host that has not accumulated any. + /// + /// Worth computing because two profiles of one account is not visible in the + /// columns: they render identically to two genuinely different logins that happen + /// to share an organisation, and the whole point of the listing is that a name + /// proves nothing. Decided on `accountUuid`, never on a display field, for that + /// reason. + pub shares_account_with: Vec, +} + +/// Every profile this host offers, `default` first and the rest by name. +/// +/// `default` leads because it is the login every other row is an alternative to, and +/// it is present even when no profiles directory exists at all, which is the state +/// almost every host is in. +/// +/// Paths rather than reads of the environment, so a test states the machine it means. +/// `unnamed` is the config directory the default login resolves to +/// (`$CLAUDE_CONFIG_DIR`, else `~/.claude`), and `None` is a host that names neither. +pub fn summarise(profiles_root: Option<&Path>, unnamed: Option<&Path>) -> Vec { + let mut rows = Vec::new(); + if let Some(unnamed) = unnamed { + rows.push(row(DEFAULT_PROFILE.to_owned(), unnamed.to_path_buf())); + } + let Some(root) = profiles_root else { + return rows; + }; + let Ok(entries) = std::fs::read_dir(root) else { + // A root that does not exist is a host with no profiles, not a failure: the + // directory is created by whatever manages the profiles, and most hosts never + // have one. + // + // Every other error kind lands here too, and that is a real loss rather than a + // tidy default: a root that is unreadable, or a plain file where the directory + // should be, lists as "no profiles" and tells a host with five of them that it + // has none. This function is pure and has no channel to say so, so the caller + // that does say it -- `render_claude_profiles`, which owns stderr -- checks + // separately. Nothing here can be trusted to have looked. + return rows; + }; + let mut named: Vec = entries + .flatten() + .filter(|entry| entry.path().is_dir()) + .filter_map(|entry| { + let name = entry.file_name().to_string_lossy().into_owned(); + // A directory the resolver would refuse is not offered, so the listing and + // the launch cannot disagree about what a profile is. `default` is excluded + // by the same call, since the resolver never consults a directory of that + // name. + (claude::profile_name_is_offerable(&name)).then(|| row(name, entry.path())) + }) + .collect(); + named.sort_by(|a, b| a.name.cmp(&b.name)); + rows.extend(named); + note_shared_accounts(&mut rows); + rows +} + +/// Fill in [`ProfileSummary::shares_account_with`] across a built listing. +/// +/// Grouped by `accountUuid` and by nothing else. A shared `organizationName` is two +/// colleagues, a shared `emailAddress` would be the same thing said less precisely, +/// and a profile whose file names no id at all joins no group rather than joining the +/// group of blanks. +/// +/// **Two names for one directory are one login, not a spare copy of one.** The group +/// this builds is what `dl --claude-profiles` turns into "all but one are spare", so a +/// group has to mean "these are separate directories you could delete one of". Rows +/// resolving to the same directory fail that: `CLAUDE_CONFIG_DIR=/work` makes the +/// `default` row and the `work` row the same directory, the same `.claude.json` and the +/// same id, and the advice would have named the only login on the host. A symlinked +/// profile directory reaches it the same way, since the walk above follows symlinks. +/// +/// So a directory is counted once, by [`std::fs::canonicalize`] where that answers and +/// by the path as given where it does not: a path that cannot be resolved is not a +/// directory anything else in the listing reached either, and treating it as its own is +/// the reading that cannot invent a spare. This deliberately says nothing about the two +/// names, which is the smaller loss: the listing prints both rows, with the same +/// account against each, and declines to advise a deletion it cannot justify. +fn note_shared_accounts(rows: &mut [ProfileSummary]) { + let directory_of = |row: &ProfileSummary| { + std::fs::canonicalize(&row.path).unwrap_or_else(|_| row.path.clone()) + }; + + // One entry per (account, directory): the first row that reaches a directory + // represents it, and a later row naming the same one is skipped rather than + // counted again. Indices and not names, because only a representative row may be + // told about its neighbours -- a second name for a directory already represented + // must be left with an empty `shares_account_with`, or the group it reconstructs + // downstream would count it as one more thing to delete. + let mut representatives: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + let mut seen: std::collections::BTreeSet<(String, std::path::PathBuf)> = + std::collections::BTreeSet::new(); + for (index, row) in rows.iter().enumerate() { + let Some(uuid) = row.account.as_ref().and_then(|a| a.account_uuid.as_deref()) else { + continue; + }; + if !seen.insert((uuid.to_owned(), directory_of(row))) { + continue; + } + representatives + .entry(uuid.to_owned()) + .or_default() + .push(index); + } + representatives.retain(|_, indices| indices.len() > 1); + + let groups: Vec> = representatives.into_values().collect(); + for group in groups { + for &index in &group { + let names: Vec = group + .iter() + .filter(|&&other| other != index) + .map(|&other| rows[other].name.clone()) + .collect(); + rows[index].shares_account_with = names; + } + } +} + +/// [`summarise`], for this machine. +/// +/// The impure half, kept to one function so everything above it is a function of its +/// inputs: the root through [`crate::domain::xdg::claude_profiles_root`], the unnamed +/// login's directory through the client that owns that decision. Empty means a host +/// naming neither a home directory nor an override, which is the one state with +/// nothing at all to list. +pub fn from_process() -> Vec { + let root = crate::domain::xdg::claude_profiles_root().ok(); + let unnamed = claude::unnamed_config_dir_from_process(); + summarise(root.as_deref(), unnamed.as_deref()) +} + +fn row(name: String, path: PathBuf) -> ProfileSummary { + let authed = claude::has_credential(&path); + let state = if authed { + ProfileState::Authed + } else { + ProfileState::NoCredential + }; + ProfileSummary { + name, + path: path.clone(), + state, + // **Only read when there is a credential.** A profile whose login was removed + // keeps its `.claude.json`, and reading that would put an email on a row that + // cannot launch anything -- against the contract the account column states + // ("a profile with no credential has nobody to name") and, worse, into + // `note_shared_accounts`, which would then report it as a spare copy of a + // working profile. It is neither: it is a directory nobody is logged in to. + // + // Carried by the type rather than filtered at each reader, so the invariant + // holds for `--claude-profiles`, for the grouping, and for anything that reads + // a `ProfileSummary` later. + account: authed.then(|| claude::account_at(&path)).flatten(), + // Filled in by `note_shared_accounts` once the whole listing exists: it is a + // fact about a row's neighbours, so no row can answer it alone. + shares_account_with: Vec::new(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A profile directory, optionally with a credential and an account. + fn profile(root: &Path, name: &str, credential: bool, email: Option<&str>) -> PathBuf { + let dir = root.join(name); + std::fs::create_dir_all(&dir).expect("a profile dir"); + if credential { + std::fs::write( + dir.join(".credentials.json"), + r#"{"claudeAiOauth":{"accessToken":"not-a-real-token"}}"#, + ) + .expect("a credential"); + } + if let Some(email) = email { + // The uuid is derived from the email, so two profiles of one account are + // built by giving them one address and two profiles that merely look alike + // are built by giving them two. + std::fs::write( + dir.join(".claude.json"), + format!( + r#"{{"oauthAccount":{{"emailAddress":"{email}","accountUuid":"uuid-of-{email}", + "organizationName":"Someorg","seatTier":"team"}},"other":1}}"# + ), + ) + .expect("an account file"); + } + dir + } + + /// A login that was removed leaves its state file behind, and the row must not + /// read it. + /// + /// Review catch. `.credentials.json` is what a logout or a cleanup removes; + /// `.claude.json` stays, holding the account that *used* to be signed in. Reading + /// it put an email on a row whose own `authed` column said "no credential", which + /// contradicts the sentence the account column is written around. + #[test] + fn a_profile_with_no_credential_names_nobody_even_with_a_state_file_left_behind() { + let root = tempfile::tempdir().expect("a root"); + profile(root.path(), "stale", false, Some("who@example.com")); + + let rows = summarise(Some(root.path()), None); + let row = rows + .iter() + .find(|row| row.name == "stale") + .expect("the profile is listed"); + assert_eq!(row.state, ProfileState::NoCredential); + assert_eq!( + row.account, None, + "a directory nobody is logged in to has no account to name" + ); + } + + #[test] + fn one_directory_under_two_names_is_not_a_spare_copy_of_itself() { + // `CLAUDE_CONFIG_DIR=/work` makes the unnamed login and the `work` + // profile the same directory: one `.claude.json`, one id, one login. Grouping + // on the id alone printed "'default', 'work' are the same account, so all but + // one are spare" about the only login on the host, and `path` is never in the + // listing, so nothing on screen let a reader notice. + let root = tempfile::tempdir().expect("a scratch root"); + let work = profile(root.path(), "work", true, Some("me@example.com")); + + let rows = summarise(Some(root.path()), Some(&work)); + + let shared = |name: &str| { + rows.iter() + .find(|row| row.name == name) + .map(|row| row.shares_account_with.clone()) + .unwrap_or_else(|| panic!("{name}: {rows:?}")) + }; + // Both rows are still listed, with the same account against each. What is gone + // is the advice to delete one of them. + assert!(shared("default").is_empty(), "{rows:?}"); + assert!(shared("work").is_empty(), "{rows:?}"); + } + + #[test] + fn a_third_name_for_a_counted_directory_does_not_inflate_the_group() { + // Three names, two directories: `default` and `alias` are one directory, and + // `spare` is a genuinely separate copy of the same account. "All but one are + // spare" is true of the pair of *directories*, so the group has to name two + // things and not three -- otherwise the count of what can be deleted is wrong + // in the direction that loses a login. + let root = tempfile::tempdir().expect("a scratch root"); + let alias = profile(root.path(), "alias", true, Some("me@example.com")); + profile(root.path(), "spare", true, Some("me@example.com")); + + let rows = summarise(Some(root.path()), Some(&alias)); + + let shared = |name: &str| { + rows.iter() + .find(|row| row.name == name) + .map(|row| row.shares_account_with.clone()) + .unwrap_or_else(|| panic!("{name}: {rows:?}")) + }; + // `default` represents the directory it shares with `alias`, so it is the one + // told about `spare`; `alias` is the second name for a directory already + // counted and is told nothing. + assert_eq!(shared("default"), ["spare"], "{rows:?}"); + assert_eq!(shared("spare"), ["default"], "{rows:?}"); + assert!(shared("alias").is_empty(), "{rows:?}"); + } + + /// And the half that made it worse than a cosmetic slip. + #[test] + fn a_logged_out_profile_is_not_reported_as_a_spare_copy_of_a_working_one() { + // Same account in both state files, and only one of them has a credential. + // Grouping on the stale one would print "base and stale are the same account, + // so all but one are spare" -- which reads as "delete one of these two", about + // a pair where one is not usable at all and the other is the only login. + let root = tempfile::tempdir().expect("a root"); + let live = profile(root.path(), "base", true, Some("me@example.com")); + let dead = profile(root.path(), "stale", false, Some("me@example.com")); + for dir in [&live, &dead] { + std::fs::write( + dir.join(".claude.json"), + r#"{"oauthAccount":{"emailAddress":"me@example.com","accountUuid":"one-uuid"}}"#, + ) + .expect("a state file"); + } + + let rows = summarise(Some(root.path()), None); + for row in &rows { + assert!( + row.shares_account_with.is_empty(), + "{} was grouped with {:?}", + row.name, + row.shares_account_with + ); + } + } + + #[test] + fn a_profile_is_named_with_the_account_behind_it() { + // The whole point: the name is chosen by a person and verified by nothing, so + // the row carries who it is actually signed in as. + let root = tempfile::tempdir().expect("a scratch root"); + profile(root.path(), "work", true, Some("someone@example.com")); + let rows = summarise(Some(root.path()), None); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].name, "work"); + assert_eq!(rows[0].state, ProfileState::Authed); + let account = rows[0].account.as_ref().expect("an account"); + assert_eq!(account.email.as_deref(), Some("someone@example.com")); + assert_eq!(account.organization.as_deref(), Some("Someorg")); + assert_eq!(account.seat_tier.as_deref(), Some("team")); + } + + #[test] + fn a_profile_made_and_never_logged_in_to_says_so() { + // The ordinary state right after something creates one, and a launch naming it + // refuses, so the listing has to distinguish it rather than omit it. + let root = tempfile::tempdir().expect("a scratch root"); + profile(root.path(), "fresh", false, None); + let rows = summarise(Some(root.path()), None); + assert_eq!(rows[0].state, ProfileState::NoCredential); + assert_eq!(rows[0].account, None); + } + + #[test] + fn an_account_file_that_says_nothing_useful_is_not_an_error() { + // Three absences with one answer, because they read alike to somebody drawing a + // table: no file, not JSON, and no oauthAccount. + let root = tempfile::tempdir().expect("a scratch root"); + let junk = profile(root.path(), "junk", true, None); + std::fs::write(junk.join(".claude.json"), "not json at all").expect("a file"); + let empty = profile(root.path(), "empty", true, None); + std::fs::write(empty.join(".claude.json"), r#"{"somethingElse":1}"#).expect("a file"); + for row in summarise(Some(root.path()), None) { + assert_eq!(row.account, None, "{}", row.name); + // Still launchable: a missing label says nothing about the credential. + assert_eq!(row.state, ProfileState::Authed, "{}", row.name); + } + } + + #[test] + fn the_default_row_leads_and_is_there_with_no_profiles_at_all() { + // Almost every host is in this state, and `default` is the login every other + // row is an alternative to. + let home = tempfile::tempdir().expect("a scratch home"); + let unnamed = home.path().join(".claude"); + std::fs::create_dir_all(&unnamed).expect("a config dir"); + std::fs::write( + unnamed.join(".credentials.json"), + r#"{"claudeAiOauth":{"accessToken":"not-a-real-token"}}"#, + ) + .expect("a credential"); + let rows = summarise(None, Some(&unnamed)); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].name, DEFAULT_PROFILE); + assert_eq!(rows[0].state, ProfileState::Authed); + } + + #[test] + fn the_default_row_is_the_name_the_resolver_answers_for() { + // Two spellings of one fact, diffed rather than left to drift: a listing that + // offered a word the resolver did not answer for would be offering a launch + // that refuses. + assert_eq!(DEFAULT_PROFILE, claude::default_profile_name()); + } + + #[test] + fn the_listing_offers_only_names_a_launch_would_accept() { + // The listing and the launch must not disagree about what a profile is. A + // directory the resolver refuses is not offered, and `default` is excluded + // because the resolver never consults a directory of that name. + let root = tempfile::tempdir().expect("a scratch root"); + for name in ["work", "default", "has space", ".hidden-ish", "-flag"] { + profile(root.path(), name, true, None); + } + let rows = summarise(Some(root.path()), None); + let offered: Vec<&str> = rows.iter().map(|row| row.name.as_str()).collect(); + assert_eq!(offered, ["work"]); + } + + #[test] + fn two_profiles_of_one_account_say_so_about_each_other() { + // The case the columns cannot show: same account under two names renders + // identically to two different logins sharing an organisation, so one of the + // two is dead weight and nothing else would say which. + let root = tempfile::tempdir().expect("a scratch root"); + profile(root.path(), "one", true, Some("same@example.com")); + profile(root.path(), "two", true, Some("same@example.com")); + profile(root.path(), "other", true, Some("different@example.com")); + let rows = summarise(Some(root.path()), None); + let shared = |name: &str| { + rows.iter() + .find(|row| row.name == name) + .map(|row| row.shares_account_with.clone()) + .expect(name) + }; + assert_eq!(shared("one"), ["two"]); + assert_eq!(shared("two"), ["one"]); + assert!(shared("other").is_empty()); + } + + #[test] + fn a_shared_organisation_is_not_a_shared_account() { + // Every profile the fixture builds carries `organizationName: Someorg`, which + // is exactly the trap: colleagues share an organisation and are different + // accounts. Grouping is by `accountUuid` and nothing else. + let root = tempfile::tempdir().expect("a scratch root"); + profile(root.path(), "mine", true, Some("me@example.com")); + profile(root.path(), "theirs", true, Some("them@example.com")); + for row in summarise(Some(root.path()), None) { + assert!(row.shares_account_with.is_empty(), "{}", row.name); + } + } + + #[test] + fn a_profile_naming_no_account_joins_no_group() { + // Two blanks are not "the same account", and saying so would be a claim about + // nothing. A profile with no state file has no id to group on. + let root = tempfile::tempdir().expect("a scratch root"); + profile(root.path(), "quiet", true, None); + profile(root.path(), "silent", true, None); + for row in summarise(Some(root.path()), None) { + assert!(row.shares_account_with.is_empty(), "{}", row.name); + } + } + + #[test] + fn a_profile_that_duplicates_the_default_login_is_named_too() { + // The most pointless profile there is: a second name for the login you would + // have got anyway. `default` is in the grouping for exactly this. + let home = tempfile::tempdir().expect("a scratch home"); + let unnamed = home.path().join(".claude"); + std::fs::create_dir_all(unnamed.parent().expect("a parent")).expect("a home"); + profile(home.path(), ".claude", true, Some("me@example.com")); + let root = tempfile::tempdir().expect("a scratch root"); + profile(root.path(), "spare", true, Some("me@example.com")); + let rows = summarise(Some(root.path()), Some(&unnamed)); + assert_eq!(rows[0].name, DEFAULT_PROFILE); + assert_eq!(rows[0].shares_account_with, ["spare"]); + assert_eq!(rows[1].shares_account_with, [DEFAULT_PROFILE]); + } + + #[test] + fn rows_are_sorted_so_a_listing_does_not_reorder_itself() { + // `read_dir` order is the filesystem's and varies between machines and between + // runs on one machine. + let root = tempfile::tempdir().expect("a scratch root"); + for name in ["zeta", "alpha", "mid"] { + profile(root.path(), name, true, None); + } + let rows = summarise(Some(root.path()), None); + let offered: Vec<&str> = rows.iter().map(|row| row.name.as_str()).collect(); + assert_eq!(offered, ["alpha", "mid", "zeta"]); + } +} diff --git a/rust/devlaunch-core/src/flows/mod.rs b/rust/devlaunch-core/src/flows/mod.rs index 4c677295..28a73e87 100644 --- a/rust/devlaunch-core/src/flows/mod.rs +++ b/rust/devlaunch-core/src/flows/mod.rs @@ -4,6 +4,9 @@ pub mod agent_worktrees; // refusal travels inside a launch refusal, and the words for it are the binary's. pub mod branch_manager; // binary surface — not part of the frozen wf API (#251 §7) +/// The Claude logins this host can forward, with the account behind each name. +pub mod claude_profiles; + pub mod completion; // binary surface — not part of the frozen wf API (#251 §7) pub mod completion_cache; diff --git a/rust/dl/src/cli.rs b/rust/dl/src/cli.rs index fa74da4a..054e92f0 100644 --- a/rust/dl/src/cli.rs +++ b/rust/dl/src/cli.rs @@ -386,6 +386,8 @@ pub(crate) enum Command { List { output: ListOutput, sizes: Sizes }, /// `dl --repos` — the known `owner/repo` strings, for completion. Repos, + /// `dl --claude-profiles` — the Claude logins `--claude-profile` can name. + ClaudeProfiles, /// `dl --completion-data` — the whole completion cache, as one JSON line. CompletionData, /// `dl --update-cache [--force]` — the silent background refresh. @@ -582,6 +584,11 @@ pub(crate) struct Cli { #[arg(long, hide = true)] autorm: bool, + /// List the Claude logins `--claude-profile` can name, and the account each is + /// signed in as. Reads them; never writes. + #[arg(long = "claude-profiles", group = "what")] + claude_profiles: bool, + /// The known `owner/repo` strings, one per line (for shell completion). #[arg(long, group = "what", hide = true)] repos: bool, @@ -751,6 +758,7 @@ enum Chosen { Purge, Version, Repos, + ClaudeProfiles, CompletionData, UpdateCache, HerdrShell, @@ -769,6 +777,7 @@ impl Cli { (self.purge, Chosen::Purge), (self.version, Chosen::Version), (self.repos, Chosen::Repos), + (self.claude_profiles, Chosen::ClaudeProfiles), (self.completion_data, Chosen::CompletionData), (self.update_cache, Chosen::UpdateCache), (self.herdr_shell, Chosen::HerdrShell), @@ -938,6 +947,7 @@ fn global_command(cli: &Cli, chosen: Chosen) -> Result { Chosen::Purge => Command::Purge { yes: cli.yes }, Chosen::Version => Command::Version, Chosen::Repos => Command::Repos, + Chosen::ClaudeProfiles => Command::ClaudeProfiles, Chosen::CompletionData => Command::CompletionData, Chosen::UpdateCache => Command::UpdateCache { force: cli.force }, Chosen::HerdrShell => Command::HerdrShell, @@ -954,6 +964,7 @@ fn flag_of(chosen: Chosen) -> &'static str { Chosen::Purge => "--purge", Chosen::Version => "--version", Chosen::Repos => "--repos", + Chosen::ClaudeProfiles => "--claude-profiles", Chosen::CompletionData => "--completion-data", Chosen::UpdateCache => "--update-cache", Chosen::HerdrShell => "--herdr-shell", @@ -1856,6 +1867,24 @@ mod tests { ); } + #[test] + fn listing_the_profiles_is_a_command_of_its_own() { + // A global command, so it takes no workspace and no modifier: the two flags + // read alike and mean opposite things, one naming a login to use and one + // asking which exist. + assert_eq!(parse(&["--claude-profiles"]), Ok(Command::ClaudeProfiles)); + assert_eq!( + parse(&["--claude-profiles", "--claude-profile", "work"]), + Err(GrammarError::ClaudeProfileNotAllowed { + command: "--claude-profiles" + }) + ); + assert!(matches!( + parse(&["--claude-profiles", "ws"]), + Err(GrammarError::TargetNotAllowed { .. }) + )); + } + #[test] fn a_claude_profile_is_refused_on_a_command_that_forwards_no_login() { // The same line `--devcontainer` draws below: a global command has no diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index 84141267..ada7421c 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -15,6 +15,7 @@ use devlaunch_core::domain::config; use devlaunch_core::domain::spec::DevcontainerPath; use devlaunch_core::domain::workspace_id::WorkspaceId; use devlaunch_core::domain::xdg; +use devlaunch_core::flows::claude_profiles; use devlaunch_core::flows::completion::{self, FileState, InstallError, Installed, RcChange}; use devlaunch_core::flows::completion_cache::{self, Refreshed}; use devlaunch_core::flows::kept_copies::KeptCopies; @@ -109,6 +110,7 @@ pub(crate) fn dispatch( Command::Version => render_version(), Command::List { output, sizes } => render_list(runner, &mut context, cache, output, sizes), Command::Repos => render_repos(&mut context, cache), + Command::ClaudeProfiles => render_claude_profiles(), Command::CompletionData => render_completion_data(&mut context, cache), Command::UpdateCache { force } => render_update_cache(runner, &mut context, cache, force), Command::Refresh => render_refresh(&mut context, cache), @@ -340,6 +342,135 @@ fn render_json( /// a cache with no repos, where Python's key check fell through to asking devpod. /// The distinguishing input is a `completions.json` written by something other /// than dl; every cache dl writes carries all four keys. +/// `dl --claude-profiles`: the logins `--claude-profile` can name, and who each is. +/// +/// **The account matters more than the name**, which is why this exists at all: a +/// profile's directory name is chosen by a person and verified by nothing, so a +/// profile called `work` holding a personal login reads as correct right up until the +/// work is pushed from the wrong identity. The name is what you type; the account +/// column is what you get. +/// +/// Nothing here reads a token. "authed" is the credential file's existence, so a +/// listing has never touched a secret. +fn render_claude_profiles() -> Ending { + let rows = claude_profiles::from_process(); + warn_if_the_profiles_root_could_not_be_read(); + if rows.is_empty() { + // No home directory and no override, which is a real state: `dl` runs with + // `XDG_CACHE_HOME` set and no home at all. + eprintln!("No Claude configuration directory on this machine, so no profiles to list."); + return Ending::Done; + } + let width = rows + .iter() + .map(|row| row.name.chars().count()) + .max() + .unwrap_or(0) + .max("NAME".len()); + println!("{: "authed", + claude_profiles::ProfileState::NoCredential => "not logged in", + }; + println!("{: = Vec::new(); + for row in rows { + if row.shares_account_with.is_empty() { + continue; + } + let mut group: Vec<&str> = row + .shares_account_with + .iter() + .map(String::as_str) + .chain(std::iter::once(row.name.as_str())) + .collect(); + group.sort_unstable(); + let key = group.join(" "); + if said.contains(&key) { + continue; + } + said.push(key); + let names = group + .iter() + .map(|name| format!("'{name}'")) + .collect::>() + .join(", "); + println!(); + println!("{names} are the same account, so all but one are spare."); + } +} + +/// The account column: what the state file says, or why it says nothing. +fn account_of(row: &claude_profiles::ProfileSummary) -> String { + let Some(account) = &row.account else { + // Two different absences, and the difference is worth a word: a profile with no + // credential has nobody to name, while one that is logged in and unnameable + // means Claude Code's state file was absent or has moved on from the shape this + // reads. Neither is an error and neither stops a launch. + return match row.state { + claude_profiles::ProfileState::NoCredential => "-".to_owned(), + claude_profiles::ProfileState::Authed => "unknown".to_owned(), + }; + }; + let mut parts = Vec::new(); + if let Some(email) = &account.email { + parts.push(email.clone()); + } + if let Some(organization) = &account.organization { + parts.push(organization.clone()); + } + if let Some(seat) = &account.seat_tier { + parts.push(seat.clone()); + } + parts.join(" · ") +} + fn render_repos(context: &mut CommandContext<'_>, cache: &Path) -> Ending { if let Some(cached) = completion_cache::read_completion_cache(&completion_cache::cache_path(cache)) diff --git a/test/test_bash_completion.py b/test/test_bash_completion.py index 6b2937f1..1289bcd8 100644 --- a/test/test_bash_completion.py +++ b/test/test_bash_completion.py @@ -6,6 +6,8 @@ import pytest from unittest.mock import patch +from fixtures.e2e_helpers import dl_command + class TestBashCompletion: """Test bash completion functionality.""" @@ -55,7 +57,7 @@ def teardown_method(self): if self.test_dir and os.path.exists(self.test_dir): shutil.rmtree(self.test_dir) - def run_completion(self, comp_line, comp_point=None): + def run_completion(self, comp_line, comp_point=None, env=None): """ Run bash completion for the given line and cursor position. @@ -66,9 +68,9 @@ def run_completion(self, comp_line, comp_point=None): Returns: List of completion suggestions """ - return self.run_completion_with_options(comp_line, comp_point)[0] + return self.run_completion_with_options(comp_line, comp_point, env)[0] - def run_completion_with_options(self, comp_line, comp_point=None): + def run_completion_with_options(self, comp_line, comp_point=None, env=None): """ As `run_completion`, and also the options the function asked bash for. @@ -93,9 +95,15 @@ def run_completion_with_options(self, comp_line, comp_point=None): # Create a bash script that sources the completion and runs it # Use shlex.quote to properly escape shell arguments + # Exported inside the script rather than passed to `subprocess`, so it is + # visible in the same place as XDG_CACHE_HOME when a failure prints the script. + exports = "\n".join( + f"export {name}={shlex.quote(str(value))}" for name, value in (env or {}).items() + ) script = f""" #!/bin/bash export XDG_CACHE_HOME={shlex.quote(str(self.cache_base))} +{exports} source {shlex.quote(str(self.completion_script))} compopt() {{ printf '%s\\n' "$*" >> {shlex.quote(str(options_file))}; }} @@ -122,6 +130,109 @@ def run_completion_with_options(self, comp_line, comp_point=None): options = [line.strip() for line in recorded.strip().split("\n") if line.strip()] return completions, options + # ------------------------------------------------------------------ + # --claude-profile: the completion and the resolver have to agree + # ------------------------------------------------------------------ + + # Directory names a profile root can hold, and whether `ProfileName::parse` + # accepts each. The invalid ones are not hypothetical: a profile directory is + # made by hand, and `-` and a space are what a hand produces. + PROFILE_FIXTURES = { + "work": True, + "personal.2": True, + "with_underscore": True, + "with-dash": True, + "-flag": False, + "has space": False, + "tab\there": False, + "unicode-é": False, + ".hidden": False, + } + + def _a_profile_root(self): + """A profiles directory holding every name in PROFILE_FIXTURES.""" + root = pathlib.Path(self.test_dir) / "claude-profiles" + for name in self.PROFILE_FIXTURES: + (root / name).mkdir(parents=True) + return root + + def test_the_completion_offers_only_names_a_launch_accepts(self): + """The completion's grammar, diffed against the binary's own listing. + + `dl.bash` restates `ProfileName::parse`, which the repo's standing rule + allows only with a test beside it that diffs the copy against the first. + This is that diff, and it is a real one rather than a restatement: the + expectation comes from running `dl --claude-profiles`, so the two + implementations have to agree about the same directory rather than both + having to agree with a list written here. + + Sourcery caught the original, which offered every directory. Tab-completing + `-flag` and then being refused for a name you never typed is worse than no + completion at all. + """ + root = self._a_profile_root() + env = {"DEVLAUNCH_CLAUDE_PROFILES_DIR": str(root)} + + offered = set(self.run_completion("dl --claude-profile ", env=env)) + + listed = subprocess.run( + dl_command() + ["--claude-profiles"], + capture_output=True, + text=True, + check=True, + env={**os.environ, **env}, + ).stdout + # The NAME column, minus the header. + named = { + line.split()[0] + for line in listed.splitlines()[1:] + if line.strip() and not line.startswith(" ") + } + + assert offered == named, ( + f"the completion offers {sorted(offered)} and `dl --claude-profiles` " + f"lists {sorted(named)}; a name in one and not the other is either a " + "completion the launch refuses or a profile you cannot tab to" + ) + + def test_the_completion_offers_the_valid_names_and_default(self): + """The absolute half of the diff above. + + Two implementations agreeing on nothing would satisfy the comparison, so + this pins what the answer actually is -- and the `default` row, which is a + name the resolver answers for with no directory behind it at all. + """ + root = self._a_profile_root() + offered = set( + self.run_completion( + "dl --claude-profile ", env={"DEVLAUNCH_CLAUDE_PROFILES_DIR": str(root)} + ) + ) + + wanted = {name for name, ok in self.PROFILE_FIXTURES.items() if ok} + assert offered == wanted | {"default"}, sorted(offered) + for name, ok in self.PROFILE_FIXTURES.items(): + if not ok: + assert name not in offered, f"{name!r} is not a name a launch accepts" + + def test_a_partial_profile_name_still_filters(self): + """The filter must not have replaced compgen's own prefix matching.""" + root = self._a_profile_root() + offered = set( + self.run_completion( + "dl --claude-profile with", env={"DEVLAUNCH_CLAUDE_PROFILES_DIR": str(root)} + ) + ) + assert offered == {"with_underscore", "with-dash"}, sorted(offered) + + def test_the_default_name_is_offered_with_no_profile_root_at_all(self): + """A host with no profiles directory still has one name to complete.""" + missing = pathlib.Path(self.test_dir) / "nothing-here" + offered = self.run_completion( + "dl --claude-profile ", env={"DEVLAUNCH_CLAUDE_PROFILES_DIR": str(missing)} + ) + assert offered == ["default"] + def test_completion_with_dashed_workspace(self): """Test completion works with names containing dashes, in both namespaces.""" # "dl my-" matches an owner, so the ids that also start "my-" are held