From 367e80f070d6002c24f216dc742e2168e5fce0bb Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Wed, 2 Sep 2026 16:34:45 +0100 Subject: [PATCH 1/6] feat: --claude-profile forwards a named Claude login One credential cannot serve a host with two accounts. `--claude-profile ` names a directory under `/devlaunch/claude-profiles/`, each holding the `.credentials.json` a `claude` login writes, and forwards that one instead of the default. A named profile that holds no credential STOPS the launch. It does not fall back to the 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 the other one would be worse than a launch that fails. The launch you see; the wrong account you find out about later and somewhere else. `NoToken` gains `ProfileUnreadable` for it, apart from `Unreadable` because a profile refusal has a name to quote and must not read as the quiet arm. `ProfileName` checks at the boundary, beside `Token` and for its reason, so `--claude-profile ../../etc` is refused by the rule rather than becoming a traversal that fails later on a read. Empty, `.`, `..`, either separator, a NUL and a leading `-` are all excluded; the grammar deliberately does not double-check, so there is one boundary rather than two that can disagree. Read above an exported CLAUDE_CODE_OAUTH_TOKEN, unlike $CLAUDE_CONFIG_DIR. Both of those are ambient and a profile was typed on this command line for this launch, so nothing ambient beats an explicit argument, and a nested `dl` naming a profile is overriding exactly the token it inherited. DEVLAUNCH_NO_CLAUDE_TOKEN still comes first: a machine that has opted out has no account to choose. The ClaudeConfig::Ours gate is untouched, so a repo whose own devcontainer owns its Claude config still forwards nothing, profile or no profile. Not stored with the workspace, unlike --devcontainer. A profile describes this session rather than the container, and storing it would mean a workspace quietly forwarding an account chosen weeks ago. Profiles live under the config home and never the cache: --purge deletes the cache entire and --prune walks the clones inside it, so a credential there would be one flag away from deletion, and nothing regenerates a login. `claude_profiles_root_in` is split out so that placement is a function of its input and a test states it. Refused on a global command, reported and ignored on a verb that forwards no login, which is the line --devcontainer already draws. Completion offers the profiles that exist, read off the disk rather than the completion cache, since one made a minute ago has to complete now. VALUE_FLAGS gains the flag with a test that diffs the list against clap's own parser, because a value-taking flag missing from it leaves its value behind for wants_startup_cache_refresh to read as a word. Host::with_claude_profile is a new promised item and moves public-api.api.txt; xdg::claude_profiles_root moves public-api.rest.txt. Neither snapshot is regenerated here: the script needs a nightly toolchain this machine does not have, and its own note warns that snapshots move with toolchain drift, so regenerating against the wrong nightly would be noise. --- CHANGELOG.md | 35 +++ README.md | 25 +- docs/workspace-tools.md | 52 ++++ rust/aid/src/rewrite.rs | 19 +- rust/devlaunch-core/completions/dl.bash | 33 ++- rust/devlaunch-core/src/clients/claude.rs | 323 +++++++++++++++++++++- rust/devlaunch-core/src/domain/xdg.rs | 58 ++++ rust/devlaunch-core/src/flows/launch.rs | 31 ++- rust/dl/src/cli.rs | 155 ++++++++++- rust/dl/src/commands.rs | 36 ++- rust/dl/src/launch.rs | 7 +- rust/dl/src/lib.rs | 5 +- 12 files changed, 741 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5f0b324..9fc89891 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,41 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`dl --claude-profile ` forwards a named Claude login instead of the + default one**, for the case one credential cannot serve: two accounts on one + machine, and a workspace that wants the one your host is not signed in to. Profiles + are directories under `/devlaunch/claude-profiles/`, each holding the + `.credentials.json` a `claude` login writes. devlaunch reads them and has no writer, + so creating one is Claude Code's job with its own `CLAUDE_CONFIG_DIR`. + + **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 + than a launch that fails: the launch you see, and the wrong account you find out about + later and somewhere else. The name is checked at the boundary as a single directory + component, so `--claude-profile ../../etc` is refused by the rule rather than becoming + a traversal that fails later on a read. + + Read above an exported `CLAUDE_CODE_OAUTH_TOKEN`, unlike `$CLAUDE_CONFIG_DIR`, because + a profile was typed on this command line for this launch and nothing ambient should + beat an explicit argument. `DEVLAUNCH_NO_CLAUDE_TOKEN` still comes first: a machine + that has opted out has no account to choose. + + **Not stored with the workspace**, unlike `--devcontainer`, so no workspace can + quietly forward an account chosen weeks ago. Profiles live under the config directory + and never the cache, so `--purge` and `--prune` cannot reach a login; + `DEVLAUNCH_CLAUDE_PROFILES_DIR` moves them for a scratch run. Completion offers the + profiles that exist, read off the disk rather than the completion cache, because a + profile made a minute ago has to complete now. + + It is not the claude.ai account a container's `claude` is paired to for Remote + Control, and it does not weaken the check that leaves a repo's own mounted Claude + config alone: `Foreign` forwards nothing, profile or no profile. `aid` passes the flag + through. A verb that forwards no login says it is ignoring it, as `--devcontainer` + does; a global command refuses it. + ### Fixed - **`$CLAUDE_CONFIG_DIR` is now honoured on the host, so a host that has moved its diff --git a/README.md b/README.md index 37b8862c..2dd4dc43 100644 --- a/README.md +++ b/README.md @@ -283,6 +283,23 @@ dl 0.29.0 Projects with several variants, compose sidecars, or a host-side `initializeCommand` are covered in [docs/devcontainer-projects.md](docs/devcontainer-projects.md). +`--claude-profile ` forwards a named Claude login instead of the default one, for +workspaces where you want a different account than the one `claude` on your host is signed in to: + +```bash +dl blooop/devlaunch --claude-profile work +``` + +Profiles live in `/devlaunch/claude-profiles//`, each holding the +`.credentials.json` that a `claude` login writes. Create one by pointing Claude Code's own +`CLAUDE_CONFIG_DIR` at the directory and logging in there; `dl` reads these and never writes them. + +Unlike `--devcontainer` it is **not** stored with the workspace, so it applies to the launch you +typed it on and no workspace ever forwards an account chosen weeks ago. A name that holds no +credential stops the launch and says so rather than falling back to your default login, which is +the whole point of naming one. [docs/workspace-tools.md](docs/workspace-tools.md) has the +precedence order and what a profile does not change. + `dl --help` is the complete reference and is kept in step with the binary by a test. ## aid: an agent instead of a shell @@ -318,6 +335,7 @@ the question and launches one-shot, so scripts behave as they always have. | `--no-remote-control`, `--no-remote` | Start a plain local session. Remote Control is on by default for `claude`: the session is named after the workspace and can be read and steered from claude.ai/code or the Claude app. It needs a claude.ai login in the container | | `--remote-control`, `--remote` | Ask for Remote Control by name. `claude` has it already; beside `--codex` or `--gemini` this says they have not got it and stops | | `--devcontainer ` | Passed through to `dl` | +| `--claude-profile ` | Passed through to `dl`: which host Claude login to forward. Not the claude.ai account the container's `claude` is paired to for Remote Control | **The trade, stated plainly.** `claude` starts with `--dangerously-skip-permissions`, because the agent is already inside a disposable container holding only this repo, and the per-tool prompts @@ -418,11 +436,12 @@ Images are yours: `docker system df` is what shows those. | `DEVLAUNCH_TIMING=1\|json` | Write a timing summary to stderr. See [docs/performance.md](docs/performance.md) | | `DEVPOD_SSH_CONFIG=` | devpod's own, honoured rather than set: it is where `devpod up` publishes host aliases, so it is where `dl` looks for them. See [docs/cli.md](docs/cli.md) | | `CLAUDE_CONFIG_DIR=` | Claude Code's own, honoured rather than set: it is where the host keeps its Claude configuration, so it is where `dl` reads the login to forward. It replaces `~/.claude` rather than being tried before it, exactly as Claude Code treats it. See [docs/workspace-tools.md](docs/workspace-tools.md) | +| `DEVLAUNCH_CLAUDE_PROFILES_DIR=` | Where `--claude-profile` looks. Defaults to `/devlaunch/claude-profiles`. Under the config directory and never the cache, so `dl --purge` cannot reach a login | Every switch here reads the same values: anything but empty, `0`, `false` or `no` counts as -set. On a "no" variable that means turn it off; on an opt-in one it means turn it on. Six -rows are not switches and do not follow it: `DEVLAUNCH_AID_AGENT`, `DEVPOD_SSH_CONFIG` and -`CLAUDE_CONFIG_DIR` take a value, `DEVLAUNCH_TIMING` counts only empty and `0` as off, so `false` and `no` +set. On a "no" variable that means turn it off; on an opt-in one it means turn it on. Seven +rows are not switches and do not follow it: `DEVLAUNCH_AID_AGENT`, `DEVPOD_SSH_CONFIG`, +`CLAUDE_CONFIG_DIR` and `DEVLAUNCH_CLAUDE_PROFILES_DIR` take a value, `DEVLAUNCH_TIMING` counts only empty and `0` as off, so `false` and `no` turn it on, `DEVLAUNCH_AID_REMOTE_CONTROL` takes `1`/`true`/`on`/`yes` or `0`/`false`/`off`/`no` and refuses anything else rather than guessing, and `HERDR_AGENT` is the one written rather than read, so a value of your own survives only a line that starts diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index 7de5ae84..2692239b 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -106,6 +106,58 @@ set it and Claude Code honours it there too. The host side read `~/.claude` regardless, so a host that had moved its configuration reported itself as not logged in while holding a perfectly good login. +### Naming a profile + +`--claude-profile ` forwards a named login instead of the default one, for the +case the order above cannot serve: two accounts on one machine, and a workspace +that wants the one your host is not signed in to. + +```bash +dl owner/repo --claude-profile work +``` + +The name is one directory under `/devlaunch/claude-profiles/`, holding the +`.credentials.json` a `claude` login writes. devlaunch reads these and has no writer +for them, so creating one is Claude Code's job: point its own `CLAUDE_CONFIG_DIR` at +the directory and log in there. + +They sit under the **config** directory and never the cache, which is deliberate. +`dl --purge` deletes devlaunch's cache entire and `dl --prune` walks the clones +inside it, so a credential kept anywhere under there would be one flag away from +deletion. A login is not a cache: nothing regenerates it. + +`DEVLAUNCH_CLAUDE_PROFILES_DIR` replaces the location whole, which is what lets a +scratch run complete and read its own profiles rather than the real ones. + +**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 +the other one would be worse than a launch that fails: the launch you see, and the +wrong account you find out about later and somewhere else. The opt-out still comes +first, because a machine that has opted out has no account to choose. + +It is read **above** an exported `CLAUDE_CODE_OAUTH_TOKEN`, unlike +`$CLAUDE_CONFIG_DIR`, and the difference is where the two come from. A profile was +typed on this command line for this launch, so nothing ambient should beat it, and a +nested `dl` naming a profile is overriding exactly the token it inherited. + +**Not stored with the workspace**, unlike `--devcontainer`. A profile describes this +session and not the container, so storing it would mean a workspace quietly +forwarding an account chosen weeks ago. Pass it per launch, or export +`CLAUDE_CODE_OAUTH_TOKEN` in a shell profile if that is the shape you want. + +Two things it does not change. It is not the claude.ai account the container's own +`claude` is paired to for **Remote Control**, which lives in the container with the +rest of the agent's state, so `aid --claude-profile work` still lists its session +under whichever account the container is signed in to. And it does not weaken the +check below: a repo whose devcontainer owns its Claude config forwards nothing, +profile or no profile. + +A verb that forwards no login at all (`stop`, `kill`, `rm`, `rme`) says it is +ignoring the flag rather than failing, the way `--devcontainer` does there. A global +command such as `--ls` refuses it outright, because there is no workspace for it to +be about. + ### A variable, not the credential file Claude Code authenticates from `CLAUDE_CODE_OAUTH_TOKEN` alone, with an otherwise diff --git a/rust/aid/src/rewrite.rs b/rust/aid/src/rewrite.rs index 1505a693..e804cf36 100644 --- a/rust/aid/src/rewrite.rs +++ b/rust/aid/src/rewrite.rs @@ -161,9 +161,22 @@ const REMOTE_CONTROL_NO: &[&str] = &["0", "false", "off", "no"]; /// /// aid splits its own command line before handing it to dl and has to tell such a /// value from the workspace spec. Python keeps the list in `dl.py` -/// (`DL_VALUE_OPTIONS`) next to the parsing it describes, and it is one entry long; -/// here it is the one thing aid knows about dl's grammar. -const DL_VALUE_OPTIONS: &[&str] = &["--devcontainer"]; +/// (`DL_VALUE_OPTIONS`) next to the parsing it describes; here it is the one thing +/// aid knows about dl's grammar. +/// +/// A flag missing from this list is not passed through wrongly, it is read as *prompt +/// text*, because everything after the workspace is the prompt. `aid --claude-profile +/// work owner/repo fix it` would send `dl` a prompt beginning "work". That is why the +/// list is here rather than inferred. +/// +/// **`--claude-profile` chooses which host credential `dl` forwards, and nothing +/// else.** It is not the claude.ai login the container's own `claude` is paired to +/// for Remote Control, which lives in the container with the rest of the agent's +/// state (docs/cli.md, "It needs a claude.ai login inside the workspace"). So +/// `aid --claude-profile work` still lists its session under whichever account the +/// container is signed in to. Two credentials, and this one is the token forwarded +/// into the session. +const DL_VALUE_OPTIONS: &[&str] = &["--devcontainer", "--claude-profile"]; /// The modifier the suffix options take, peeled only in their company. /// diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index 2ff280d2..5fc35ace 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -65,9 +65,9 @@ _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 --help -h --version" + local global_opts="--ls --install --refresh --prune --reconcile --purge --herdr-shell --rm --devcontainer --claude-profile --help -h --version" if [[ "$cmd" == aid ]]; then - global_opts="--claude --codex --gemini --devcontainer --help -h --version" + global_opts="--claude --codex --gemini --devcontainer --claude-profile --help -h --version" fi # Workspace subcommands @@ -77,8 +77,33 @@ _dl_completion() { # two different requests, docker's `rm` and `run --rm`. local ws_cmds="up stop kill rm rme code restart recreate reset dotfiles --rm --" - # Options that take a value; a variant name or a path follows them. - local value_opts="--devcontainer" + # Options that take a value; a variant name, a profile name or a path follows. + local value_opts="--devcontainer --claude-profile" + + # After --claude-profile, offer the profile directories that exist. Read off the + # disk rather than out of the completion cache, deliberately: profiles are + # created by hand and rarely, the cache is rebuilt by commands that change + # *workspaces*, and a profile you made a minute ago has to complete now. It is one + # readdir of a directory holding a handful of entries. + # + # Honours DEVLAUNCH_CLAUDE_PROFILES_DIR and XDG_CONFIG_HOME the same way + # `domain::xdg::claude_profiles_root` does, so a scratch run completes its own + # profiles and not the real ones. A mistyped name is a hard refusal at launch, not + # a fallback to the default login, which is what makes completing these worth the + # readdir. + if [[ "${prev}" == "--claude-profile" ]]; then + local profiles_root="${DEVLAUNCH_CLAUDE_PROFILES_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/devlaunch/claude-profiles}" + local profiles="" pdir + if [[ -d "${profiles_root}" ]]; then + for pdir in "${profiles_root}"/*/; do + [[ -d "$pdir" ]] || continue + pdir="${pdir%/}" + profiles+=" ${pdir##*/}" + done + fi + COMPREPLY=( $(compgen -W "${profiles}" -- ${cur}) ) + return 0 + fi # After --devcontainer, offer the repo's variant directories (and paths). if [[ " ${value_opts} " == *" ${prev} "* ]]; then diff --git a/rust/devlaunch-core/src/clients/claude.rs b/rust/devlaunch-core/src/clients/claude.rs index d7597579..9cd01a8d 100644 --- a/rust/devlaunch-core/src/clients/claude.rs +++ b/rust/devlaunch-core/src/clients/claude.rs @@ -137,6 +137,37 @@ impl Token { } } +/// A value that can be one directory component under the profiles root. +/// +/// Its own type for [`Token`]'s reason: the check belongs at the boundary, once. A +/// name is joined onto a path, so anything that could climb out of the profiles root +/// or name something other than a leaf is refused rather than cleaned up. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ProfileName(String); + +impl ProfileName { + /// `raw` if it can be a leaf directory name, else nothing. + /// + /// The same flat-ASCII set [`Token::parse`] accepts, minus the three spellings + /// that are not leaves: empty, `.` and `..`. A separator of either kind, a NUL and + /// a leading `-` are excluded by the set itself, the last one because a name that + /// looks like a flag reads as one everywhere it is later printed or passed on. + pub(crate) fn parse(raw: &str) -> Option { + let flat = !raw.is_empty() + && !raw.starts_with('-') + && raw != "." + && raw != ".." + && raw + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '_' | '.' | '-')); + flat.then(|| Self(raw.to_owned())) + } + + pub(crate) fn as_str(&self) -> &str { + &self.0 + } +} + /// The values on the host this decision reads. /// /// Parameters rather than reads of the process environment, exactly as @@ -155,6 +186,14 @@ pub(crate) struct HostEnv { /// keep winning so a `dl` launched from inside a workspace still forwards what it /// was given. Above `$HOME` because it is what Claude Code itself prefers. pub(crate) config_dir: Option, + /// `--claude-profile`, if one was typed. + /// + /// **The one field here that is not a read of the environment**, which is what + /// puts it above [`Self::token`]: it was typed on this command line, for this + /// launch, and nothing ambient should beat an explicit argument. A nested `dl` + /// naming a profile means it, and the inherited token is exactly what it is + /// overriding. [`HostEnv::from_process`] leaves it `None` for that reason. + pub(crate) profile: Option, } impl HostEnv { @@ -164,6 +203,10 @@ impl HostEnv { disable: crate::osext::env_str(DISABLE_VAR), token: crate::osext::env_str(TOKEN_VAR), config_dir: crate::osext::env_str(CONFIG_DIR_VAR), + // Not a read of the environment, deliberately: a profile is typed on a + // command line and reaches this struct from the CLI, which is what puts + // it above the ambient token. See [`Self::profile`]. + profile: None, } } } @@ -183,6 +226,21 @@ pub(crate) enum NoToken { NotLoggedIn, /// The file is there and did not yield a token. Unreadable(String), + /// A profile was named and yielded no token. + /// + /// Its own arm rather than [`Self::Unreadable`], because this is the one refusal + /// that must **stop** a launch's forwarding rather than quietly leaving it + /// unforwarded. The user named an account; forwarding the default one instead is + /// the mistake profiles exist to prevent, and it is invisible until the day it + /// pushes to the wrong place. Carries the name so the message can quote what was + /// typed, which is a profile name and never a credential. + ProfileUnreadable { name: String, path: String }, + /// A profile name that could not be a directory component. + /// + /// Refused at the boundary rather than sanitised, for [`Token::parse`]'s reason: + /// `--claude-profile ../../etc` must be a refusal naming the rule, not a + /// traversal that happens to fail later on a read. + ProfileNotAName(String), } /// The host's Claude token, or why there is none. @@ -198,10 +256,17 @@ pub(crate) enum TokenLookup { /// means. No subprocess and no timing span, unlike [`super::gh::resolve_token`]: /// this is one file read, and there is no CLI to ask. Which directory that read lands /// in is [`config_dir`]'s decision. -pub(crate) fn resolve_token(home: Option<&Path>, host: &HostEnv) -> TokenLookup { +pub(crate) fn resolve_token( + home: Option<&Path>, + profiles_root: Option<&Path>, + host: &HostEnv, +) -> TokenLookup { if forwarding_disabled(host.disable.as_deref()) { return TokenLookup::Missing(NoToken::OptedOut); } + if let Some(named) = host.profile.as_deref() { + return from_profile(named, profiles_root); + } if let Some(token) = host.token.as_deref().and_then(Token::parse) { return TokenLookup::Found(token); } @@ -222,6 +287,40 @@ pub(crate) fn resolve_token(home: Option<&Path>, host: &HostEnv) -> TokenLookup } } +/// The token a named profile holds, or the reason it holds none. +/// +/// **Nothing here falls through to another credential**, and that is the whole point +/// of the feature. Two accounts on one machine is what profiles are for, so a typo +/// that silently forwarded the other one would be worse than a launch that stops: the +/// launch you can see, and the wrong account you find out about later, somewhere else. +fn from_profile(named: &str, profiles_root: Option<&Path>) -> TokenLookup { + let Some(name) = ProfileName::parse(named) else { + return TokenLookup::Missing(NoToken::ProfileNotAName(named.to_owned())); + }; + // No root resolves on a machine that names no home directory and set no override. + // A profile was still typed, so this is that refusal and not `NotLoggedIn`. + let Some(root) = profiles_root else { + return TokenLookup::Missing(NoToken::ProfileUnreadable { + name: name.as_str().to_owned(), + path: String::new(), + }); + }; + let path = root.join(name.as_str()).join(CREDENTIALS_FILENAME); + let unreadable = || { + TokenLookup::Missing(NoToken::ProfileUnreadable { + name: name.as_str().to_owned(), + path: path.display().to_string(), + }) + }; + let Ok(text) = std::fs::read_to_string(&path) else { + return unreadable(); + }; + match token_from_credentials(&text) { + Some(token) => TokenLookup::Found(token), + None => unreadable(), + } +} + /// The directory this host keeps its Claude configuration in, if it names one. /// /// `$CLAUDE_CONFIG_DIR` when it names something, `$HOME/.claude` otherwise, and @@ -331,7 +430,7 @@ mod tests { // holds a refresh token, and a container that refreshed one could rotate the // host's own login away. Only the access token is ever named. let home = logged_in("not-a-real-access-token"); - let found = resolve_token(Some(home.path()), &HostEnv::default()); + let found = resolve_token(Some(home.path()), None, &HostEnv::default()); assert_eq!( found, TokenLookup::Found(Token("not-a-real-access-token".to_owned())) @@ -353,7 +452,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(Some(home.path()), &host), + resolve_token(Some(home.path()), None, &host), TokenLookup::Missing(NoToken::OptedOut) ); } @@ -370,7 +469,7 @@ mod tests { }; assert!( matches!( - resolve_token(Some(home.path()), &host), + resolve_token(Some(home.path()), None, &host), TokenLookup::Found(_) ), "{falsey:?}" @@ -389,6 +488,200 @@ mod tests { dir } + /// A profiles root holding one named profile whose credential carries `token`. + fn profiles_root_with(name: &str, token: &str) -> tempfile::TempDir { + let root = tempfile::tempdir().expect("a scratch profiles root"); + let dir = root.path().join(name); + std::fs::create_dir_all(&dir).expect("a profile dir"); + std::fs::write( + dir.join(CREDENTIALS_FILENAME), + format!(r#"{{"claudeAiOauth":{{"accessToken":"{token}"}}}}"#), + ) + .expect("a credential"); + root + } + + #[test] + fn a_named_profile_is_read_from_its_own_directory() { + let root = profiles_root_with("work", "not-a-real-work-token"); + let host = HostEnv { + profile: Some("work".to_owned()), + ..HostEnv::default() + }; + assert_eq!( + resolve_token(None, Some(root.path()), &host), + TokenLookup::Found(Token("not-a-real-work-token".to_owned())) + ); + } + + #[test] + fn a_named_profile_that_yields_nothing_refuses_rather_than_falling_back() { + // The most important test in the feature. Two accounts on one machine is what + // profiles are for, so a typo must stop the launch rather than forward the + // other account: the launch you see, the wrong account you find out about + // later and somewhere else. + let home = logged_in("not-a-real-home-token"); + let root = profiles_root_with("work", "not-a-real-work-token"); + for named in ["typo", "work2"] { + let host = HostEnv { + profile: Some(named.to_owned()), + ..HostEnv::default() + }; + let lookup = resolve_token(Some(home.path()), Some(root.path()), &host); + assert!( + matches!( + &lookup, + TokenLookup::Missing(NoToken::ProfileUnreadable { name, .. }) if name == named + ), + "{named}: {lookup:?}" + ); + } + } + + #[test] + fn a_profile_whose_credential_is_junk_refuses_under_its_own_name() { + // Distinguished from `Unreadable`, which is the same file failing on the + // unnamed path: a profile was asked for, so the refusal says which one. + let root = tempfile::tempdir().expect("a scratch profiles root"); + let dir = root.path().join("work"); + std::fs::create_dir_all(&dir).expect("a profile dir"); + std::fs::write(dir.join(CREDENTIALS_FILENAME), "not json at all").expect("a file"); + let host = HostEnv { + profile: Some("work".to_owned()), + ..HostEnv::default() + }; + assert!(matches!( + resolve_token(None, Some(root.path()), &host), + TokenLookup::Missing(NoToken::ProfileUnreadable { .. }) + )); + } + + #[test] + fn a_named_profile_beats_an_inherited_token_and_the_config_dir() { + // Typed on this command line, for this launch, so nothing ambient beats it. + // A nested `dl` naming a profile means it, and the inherited token is exactly + // what it is overriding. + let root = profiles_root_with("work", "not-a-real-work-token"); + let moved = credential_dir_holding("not-a-real-moved-token"); + let host = HostEnv { + token: Some("not-a-real-exported-token".to_owned()), + config_dir: Some(moved.path().display().to_string()), + profile: Some("work".to_owned()), + ..HostEnv::default() + }; + assert_eq!( + resolve_token(None, Some(root.path()), &host), + TokenLookup::Found(Token("not-a-real-work-token".to_owned())) + ); + } + + #[test] + fn the_opt_out_is_read_before_the_profile_is() { + // Set, therefore meant: a machine that has opted out has no account to choose, + // so the opt-out stays first even against an explicit argument. + let root = profiles_root_with("work", "not-a-real-work-token"); + let host = HostEnv { + disable: Some("1".to_owned()), + profile: Some("work".to_owned()), + ..HostEnv::default() + }; + assert_eq!( + resolve_token(None, Some(root.path()), &host), + TokenLookup::Missing(NoToken::OptedOut) + ); + } + + #[test] + fn a_profile_name_that_is_not_a_leaf_is_refused_by_name() { + // Refused at the boundary, and no read is attempted: the point is that a name + // which could climb out of the profiles root never reaches a path join. + let root = profiles_root_with("work", "not-a-real-work-token"); + for named in [ + "", + ".", + "..", + "../..", + "a/b", + "/etc", + "..\\windows", + "-flag", + "has space", + "n\u{0}ul", + ] { + let host = HostEnv { + profile: Some(named.to_owned()), + ..HostEnv::default() + }; + let lookup = resolve_token(None, Some(root.path()), &host); + assert!( + matches!(&lookup, TokenLookup::Missing(NoToken::ProfileNotAName(n)) if n == named), + "{named:?}: {lookup:?}" + ); + } + } + + #[test] + fn an_ordinary_profile_name_is_accepted() { + // The other half of the rule above, so it cannot be tightened into refusing + // everything and still pass. + for named in ["work", "work-2", "work_2", "Work.2", "a"] { + assert_eq!( + ProfileName::parse(named).as_ref().map(ProfileName::as_str), + Some(named), + "{named:?}" + ); + } + } + + #[test] + fn a_profile_with_no_root_to_look_in_refuses_as_a_profile() { + // A machine that names no home directory and set no override. A profile was + // still typed, so this is that refusal rather than `NotLoggedIn`. + let host = HostEnv { + profile: Some("work".to_owned()), + ..HostEnv::default() + }; + assert!(matches!( + resolve_token(None, None, &host), + TokenLookup::Missing(NoToken::ProfileUnreadable { .. }) + )); + } + + #[test] + fn the_refresh_token_does_not_travel_from_a_profile_either() { + let root = tempfile::tempdir().expect("a scratch profiles root"); + let dir = root.path().join("work"); + std::fs::create_dir_all(&dir).expect("a profile dir"); + std::fs::write( + dir.join(CREDENTIALS_FILENAME), + r#"{"claudeAiOauth":{"accessToken":"not-a-real-access-token", + "refreshToken":"not-a-real-refresh-token"}}"#, + ) + .expect("a credential"); + let host = HostEnv { + profile: Some("work".to_owned()), + ..HostEnv::default() + }; + let TokenLookup::Found(token) = resolve_token(None, Some(root.path()), &host) else { + panic!("the profile credential should have been read"); + }; + assert!(!token.as_str().contains("refresh"), "{token:?}"); + } + + #[test] + fn a_profile_refusal_never_prints_a_token() { + // `ProfileUnreadable` carries a name and a path, and both are safe. This is + // the guard against someone later adding the value to the message. + let root = tempfile::tempdir().expect("a scratch profiles root"); + let host = HostEnv { + profile: Some("work".to_owned()), + token: Some("not-a-real-secret-token".to_owned()), + ..HostEnv::default() + }; + let lookup = resolve_token(None, Some(root.path()), &host); + assert!(!format!("{lookup:?}").contains("secret"), "{lookup:?}"); + } + #[test] fn the_host_honours_claude_config_dir() { // The asymmetry this closes: the container-side probe has always read this @@ -400,7 +693,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(None, &host), + resolve_token(None, None, &host), TokenLookup::Found(Token("not-a-real-moved-token".to_owned())) ); } @@ -419,7 +712,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(Some(home.path()), &host), + resolve_token(Some(home.path()), None, &host), TokenLookup::Found(Token("not-a-real-moved-token".to_owned())) ); @@ -430,7 +723,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(Some(home.path()), &empty), + resolve_token(Some(home.path()), None, &empty), TokenLookup::Found(Token("not-a-real-home-token".to_owned())) ); } @@ -446,7 +739,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(Some(home.path()), &host), + resolve_token(Some(home.path()), None, &host), TokenLookup::Missing(NoToken::NotLoggedIn) ); } @@ -462,7 +755,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(None, &host), + resolve_token(None, None, &host), TokenLookup::Found(Token("not-a-real-exported-token".to_owned())) ); } @@ -477,7 +770,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(None, &host), + resolve_token(None, None, &host), TokenLookup::Missing(NoToken::OptedOut) ); } @@ -497,7 +790,7 @@ mod tests { config_dir: Some(dir.path().display().to_string()), ..HostEnv::default() }; - let TokenLookup::Found(token) = resolve_token(None, &host) else { + let TokenLookup::Found(token) = resolve_token(None, None, &host) else { panic!("the credential should have been read"); }; assert!(!token.as_str().contains("refresh"), "{token:?}"); @@ -512,7 +805,7 @@ mod tests { ..HostEnv::default() }; assert_eq!( - resolve_token(None, &host), + resolve_token(None, None, &host), TokenLookup::Found(Token("not-a-real-exported-token".to_owned())) ); } @@ -523,11 +816,11 @@ mod tests { // case. Both are ordinary and neither is worth a warning on every launch. let home = tempfile::tempdir().expect("a scratch home"); assert_eq!( - resolve_token(Some(home.path()), &HostEnv::default()), + resolve_token(Some(home.path()), None, &HostEnv::default()), TokenLookup::Missing(NoToken::NotLoggedIn) ); assert_eq!( - resolve_token(None, &HostEnv::default()), + resolve_token(None, None, &HostEnv::default()), TokenLookup::Missing(NoToken::NotLoggedIn) ); } @@ -553,7 +846,7 @@ mod tests { .expect("a file"); assert!( matches!( - resolve_token(Some(home.path()), &HostEnv::default()), + resolve_token(Some(home.path()), None, &HostEnv::default()), TokenLookup::Missing(NoToken::Unreadable(_)) ), "{text:?}" diff --git a/rust/devlaunch-core/src/domain/xdg.rs b/rust/devlaunch-core/src/domain/xdg.rs index ec17bd2c..41591d24 100644 --- a/rust/devlaunch-core/src/domain/xdg.rs +++ b/rust/devlaunch-core/src/domain/xdg.rs @@ -65,6 +65,42 @@ fn devlaunch_cache_in(cache_home: &Path) -> PathBuf { cache_home.join("devlaunch") } +/// Where named Claude profiles live: `config_home()/devlaunch/claude-profiles`. +/// +/// **Under the config home, not the cache**, which is the whole reason this is a +/// function. `dl --purge` deletes [`devlaunch_cache`] entire and `dl --prune` walks +/// [`clone_root_in`] inside it, so a credential kept anywhere under the cache would +/// be one flag away from deletion. A login is not a cache: nothing regenerates it. +/// `the_profiles_root_is_nowhere_a_purge_or_a_prune_can_reach` holds that. +/// +/// Beside `config.toml`, which [`super::config`] already puts one directory under the +/// config home, so devlaunch has one configuration directory rather than two. +/// +/// `$DEVLAUNCH_CLAUDE_PROFILES_DIR` replaces it whole, which is what lets a test and +/// a scratch run read and complete their own profiles rather than the real ones. An +/// empty value counts as unset, the rule every variable here follows. +pub fn claude_profiles_root() -> Result { + match std::env::var_os(CLAUDE_PROFILES_DIR_VAR) { + Some(value) if !value.is_empty() => Ok(PathBuf::from(value)), + _ => config_home().map(|config| claude_profiles_root_in(&config)), + } +} + +/// The placement half of [`claude_profiles_root`], as a function of the config home. +/// +/// Split out for [`devlaunch_cache_in`]'s reason: the decision is then a function of +/// its input, so a test can state the machine it means instead of mutating an +/// environment every other test in the binary shares. +fn claude_profiles_root_in(config_home: &Path) -> PathBuf { + config_home.join("devlaunch").join(CLAUDE_PROFILES_LEAF) +} + +/// The scratch override for [`claude_profiles_root`]. +const CLAUDE_PROFILES_DIR_VAR: &str = "DEVLAUNCH_CLAUDE_PROFILES_DIR"; + +/// The leaf [`claude_profiles_root`] ends in, named once. +const CLAUDE_PROFILES_LEAF: &str = "claude-profiles"; + /// The one directory devlaunch clones into, under the cache directory ownership /// is decided by. /// @@ -164,6 +200,28 @@ mod tests { } } + #[test] + fn the_profiles_root_is_nowhere_a_purge_or_a_prune_can_reach() { + // Mandatory rather than incidental. `dl --purge` deletes `devlaunch_cache()` + // entire and `dl --prune` walks `clone_root_in` inside it, so a credential + // under either is one flag away from deletion and nothing regenerates a + // login. This asserts the placement rather than trusting whoever moves it + // next. + let profiles = claude_profiles_root_in(Path::new("/c")); + let devlaunch = devlaunch_cache_in(Path::new("/k")); + let clones = clone_root_in(&devlaunch); + assert!( + !profiles.starts_with(&devlaunch), + "{} is inside {}", + profiles.display(), + devlaunch.display() + ); + assert!(!profiles.starts_with(&clones)); + // And it is beside the other configuration, so devlaunch has one + // configuration directory rather than two. + assert_eq!(profiles, PathBuf::from("/c/devlaunch/claude-profiles")); + } + #[test] fn devlaunch_cache_is_one_directory_under_the_cache_home() { assert_eq!( diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index 8a2fd7fb..3c37b41a 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -241,6 +241,18 @@ pub struct Host { /// home directory; `dl` still runs there when `XDG_CACHE_HOME` is set, and /// then there is no ssh config for it to look in at all. pub(crate) home: Option, + /// Where named Claude profiles live, if this machine resolves one. + /// + /// Resolved once by the caller, as [`Self::cache_dir`] is and for the same reason: + /// resolving it can fail on a machine that names no home directory, and a second + /// answer computed further down could disagree with the first. `None` is that + /// machine, and a launch that named a profile there refuses rather than falling + /// back to the unnamed credential. + /// + /// **Under the config home, never the cache** + /// ([`crate::domain::xdg::claude_profiles_root`]): `--purge` deletes the cache + /// entire, and a login is not a cache. + pub(crate) claude_profiles_root: Option, /// Everything devlaunch stores: the launch locks, the shared pixi cache and /// the context-options cache all hang off this. pub(crate) cache_dir: PathBuf, @@ -282,11 +294,24 @@ impl Host { devpod_ssh_config: crate::osext::env_str(ssh::CONFIG_VAR), ssh_auth_sock: crate::osext::env_str(SSH_AUTH_SOCK_VAR), home: crate::osext::home_dir(), + claude_profiles_root: crate::domain::xdg::claude_profiles_root().ok(), cache_dir: cache_dir.into(), devpod_home: DevpodHome::locate(), } } + /// Name the Claude profile this run forwards, if one was typed. + /// + /// A builder rather than a parameter on [`Self::from_process`], so adding it does + /// not move that signature, and so a caller with nothing to say passes nothing. + /// The value is not validated here: [`crate::clients::claude`] owns that check and + /// owns the refusal, which keeps one boundary rather than two. + #[must_use] + pub fn with_claude_profile(mut self, profile: Option) -> Self { + self.claude.profile = profile; + self + } + /// The lock two `up`s of one workspace serialize on. pub(crate) fn launch_lock_path(&self, workspace_id: &str) -> PathBuf { self.cache_dir @@ -1935,7 +1960,11 @@ impl<'a> SessionContext<'a> { if self.claude_seen.get() != Some(ClaudeConfig::Ours) { return None; } - match claude::resolve_token(self.host.home.as_deref(), &self.host.claude) { + match claude::resolve_token( + self.host.home.as_deref(), + self.host.claude_profiles_root.as_deref(), + &self.host.claude, + ) { claude::TokenLookup::Found(token) => Some(token), claude::TokenLookup::Missing(_) => None, } diff --git a/rust/dl/src/cli.rs b/rust/dl/src/cli.rs index f1f4c44f..b7cadd29 100644 --- a/rust/dl/src/cli.rs +++ b/rust/dl/src/cli.rs @@ -411,12 +411,14 @@ pub(crate) enum Command { Select { verb: Verb, devcontainer: Option, + claude_profile: Option, }, /// A workspace, and what to do with it. Workspace { target: String, verb: Verb, devcontainer: Option, + claude_profile: Option, }, } @@ -444,6 +446,14 @@ pub(crate) enum GrammarError { CommandNotAllowed { verb: &'static str }, /// `--devcontainer` on a command that opens no workspace. DevcontainerNotAllowed { command: &'static str }, + /// `--claude-profile` on a command that opens no workspace. + /// + /// Refused here and merely *reported* for a workspace verb that forwards no + /// login, which is the split `--devcontainer` already draws: a global command + /// has no workspace for the flag to mean anything about, while `dl stop + /// --claude-profile work` names a real workspace and a flag that does nothing to + /// it. See `claude_profile_ignored` in `crate::commands`. + ClaudeProfileNotAllowed { command: &'static str }, /// `--rm` on a command the flag is not defined for. /// /// Refused rather than ignored, and `code` is why. `dl code --rm` @@ -599,6 +609,12 @@ pub(crate) struct Cli { /// pass it once. #[arg(long, value_name = "VARIANT|PATH")] devcontainer: Option, + /// Forward a named Claude login instead of the default one. Profiles live in + /// `/devlaunch/claude-profiles//`. Per launch: unlike + /// `--devcontainer` it is not stored with the workspace, so a workspace never + /// forwards an account chosen weeks ago. + #[arg(long = "claude-profile", value_name = "NAME")] + claude_profile: Option, /// Delete the workspace once the session ends, like `docker run --rm`. Only /// for the two forms that hand one over: `dl ` and `dl -- `. /// Stops at work that is nowhere else, exactly as the `rm` verb does. @@ -856,6 +872,9 @@ fn global_command(cli: &Cli, chosen: Chosen) -> Result { if cli.devcontainer.is_some() { return Err(GrammarError::DevcontainerNotAllowed { command: name }); } + if cli.claude_profile.is_some() { + return Err(GrammarError::ClaudeProfileNotAllowed { command: name }); + } if cli.rm { return Err(GrammarError::RmNotAllowed { command: name }); } @@ -944,6 +963,10 @@ fn flag_of(chosen: Chosen) -> &'static str { /// The workspace-first and verb-first grammar: up to two words, plus `-- `. fn workspace_command(cli: Cli, argv: &[String]) -> Result { let devcontainer = devcontainer_of(&cli)?; + // Carried as typed. `clients::claude` owns the check that a name can be one + // directory component, and owns the refusal, so the grammar does not get a + // second opinion about what a profile name may be. + let claude_profile = cli.claude_profile.clone(); if cli.yes { return Err(GrammarError::ModifierNotAllowed { modifier: "--yes", @@ -990,6 +1013,7 @@ fn workspace_command(cli: Cli, argv: &[String]) -> Result rm: rm_on_exit_of(&cli), }, devcontainer, + claude_profile, }); } ForcePlace::VerbSlot { target } => { @@ -1081,11 +1105,16 @@ fn workspace_command(cli: Cli, argv: &[String]) -> Result }); } Ok(match target { - None => Command::Select { verb, devcontainer }, + None => Command::Select { + verb, + devcontainer, + claude_profile, + }, Some(target) => Command::Workspace { target, verb, devcontainer, + claude_profile, }, }) } @@ -1107,6 +1136,14 @@ fn devcontainer_of(cli: &Cli) -> Result, GrammarError> } } +/// The `dl` flags that take a separate value, which [`argv_without_value_flags`] +/// has to swallow in pairs. +/// +/// A second copy of a fact about [`Cli`], and `the_value_flags_are_the_ones_clap_takes_values_for` +/// is the test that diffs it against clap's own parser rather than leaving it to be +/// kept true by hand. +const VALUE_FLAGS: [&str; 2] = ["--devcontainer", "--claude-profile"]; + /// The argv `wants_startup_cache_refresh` is asked about. /// /// Python asks the predicate *after* pulling `--devcontainer` out of the argument @@ -1115,9 +1152,13 @@ fn devcontainer_of(cli: &Cli) -> Result, GrammarError> /// stripping happens here rather than after clap: what clap produces is a /// [`Cli`], and the predicate's question is about the words. /// +/// Every `dl` flag that takes a *separate* value belongs in [`VALUE_FLAGS`], or its +/// value is left behind to be read as one of the predicate's words: `dl +/// --claude-profile work --ls` would otherwise look like a line with a word in it. +/// /// Scanning stops at the first bare `--`, because everything after it is the /// command the workspace runs and must not be read as `dl`'s own flags. -pub(crate) fn argv_without_devcontainer(argv: &[String]) -> Vec<&str> { +pub(crate) fn argv_without_value_flags(argv: &[String]) -> Vec<&str> { let mut kept = Vec::with_capacity(argv.len()); let mut rest = argv.iter(); while let Some(argument) = rest.next() { @@ -1126,10 +1167,13 @@ pub(crate) fn argv_without_devcontainer(argv: &[String]) -> Vec<&str> { kept.extend(rest.map(String::as_str)); break; } - if argument.starts_with("--devcontainer=") { + if VALUE_FLAGS + .iter() + .any(|flag| argument.starts_with(&format!("{flag}="))) + { continue; } - if argument == "--devcontainer" { + if VALUE_FLAGS.contains(&argument.as_str()) { // Its value, whatever it is. A missing value is clap's error to // report, not this scan's. rest.next(); @@ -1227,6 +1271,7 @@ mod tests { target: target.to_owned(), verb, devcontainer: None, + claude_profile: None, } } @@ -1243,6 +1288,7 @@ mod tests { Command::Select { verb: Verb::Stop, devcontainer: None, + claude_profile: None, }, ), (&["stop", "ws"], workspace("ws", Verb::Stop)), @@ -1330,7 +1376,8 @@ mod tests { parse(&["rme"]), Ok(Command::Select { verb: remove_and_exit(false), - devcontainer: None + devcontainer: None, + claude_profile: None, }) ); assert!(remove_and_exit(false).several_at_once()); @@ -1545,7 +1592,8 @@ mod tests { parse(&[]), Ok(Command::Select { verb: attach(), - devcontainer: None + devcontainer: None, + claude_profile: None, }) ); } @@ -1763,7 +1811,8 @@ mod tests { parse(&["stop"]), Ok(Command::Select { verb: Verb::Stop, - devcontainer: None + devcontainer: None, + claude_profile: None, }) ); } @@ -1807,6 +1856,93 @@ mod tests { ); } + #[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 + // workspace for the flag to be about. The verb case is not a refusal, it is + // `commands::claude_profile_ignored`. + for command in [ + "--ls", + "--prune", + "--purge", + "--reconcile", + "--install", + "--refresh", + "--version", + ] { + assert_eq!( + parse(&[command, "--claude-profile", "work"]), + Err(GrammarError::ClaudeProfileNotAllowed { command }), + "{command}" + ); + } + } + + #[test] + fn a_claude_profile_rides_the_workspace_forms_that_open_a_session() { + // Carried as typed, and carried by the picker form too: the flag is about + // this launch, so a picked workspace gets it exactly as a named one does. + assert_eq!( + parse(&["ws", "--claude-profile", "work"]), + Ok(Command::Workspace { + target: "ws".to_owned(), + verb: attach(), + devcontainer: None, + claude_profile: Some("work".to_owned()), + }) + ); + assert_eq!( + parse(&["--claude-profile", "work"]), + Ok(Command::Select { + verb: attach(), + devcontainer: None, + claude_profile: Some("work".to_owned()), + }) + ); + } + + #[test] + fn a_profile_name_the_grammar_will_not_judge_still_reaches_the_command() { + // The grammar deliberately does not validate the name: `clients::claude` + // owns that check and owns the refusal, so there is one boundary rather than + // two that can disagree. A traversal attempt is carried here and refused + // there. + assert_eq!( + parse(&["ws", "--claude-profile", "../../etc"]), + Ok(Command::Workspace { + target: "ws".to_owned(), + verb: attach(), + devcontainer: None, + claude_profile: Some("../../etc".to_owned()), + }) + ); + } + + #[test] + fn the_value_flags_are_the_ones_clap_takes_values_for() { + // `VALUE_FLAGS` is a second copy of a fact about `Cli`, and this is the diff + // that keeps it honest rather than leaving it to be remembered. A new + // value-taking flag missing from the list does not reach `dl` wrongly, it + // leaves its value behind for `wants_startup_cache_refresh` to read as a + // word. + use clap::CommandFactory; + let mut wanted: Vec = Cli::command() + .get_arguments() + .filter(|arg| !arg.is_positional()) + .filter(|arg| { + matches!( + arg.get_action(), + clap::ArgAction::Set | clap::ArgAction::Append + ) + }) + .filter_map(|arg| arg.get_long().map(|long| format!("--{long}"))) + .collect(); + wanted.sort(); + let mut listed: Vec = VALUE_FLAGS.iter().map(|f| (*f).to_owned()).collect(); + listed.sort(); + assert_eq!(listed, wanted); + } + #[test] fn devcontainer_is_refused_on_a_command_that_opens_nothing() { assert_eq!( @@ -1918,7 +2054,7 @@ mod tests { ]; for (argv, expected) in cases { let owned: Vec = argv.iter().map(|word| word.to_string()).collect(); - assert_eq!(argv_without_devcontainer(&owned), expected, "{argv:?}"); + assert_eq!(argv_without_value_flags(&owned), expected, "{argv:?}"); } } @@ -1973,7 +2109,8 @@ mod tests { parse(&["--rm"]), Ok(Command::Select { verb: Verb::Attach { rm: RmOnExit::Yes }, - devcontainer: None + devcontainer: None, + claude_profile: None, }) ); } diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index 6f93ee51..84141267 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -140,6 +140,13 @@ pub(crate) fn dispatch( target: workspace_id, verb: Verb::Attach { rm: RmOnExit::No }, devcontainer: None, + // Nothing to inherit, for the same reason `devcontainer` is + // `None`: a profile is named per launch and deliberately not + // stored with the workspace, so a pane opened beside an agent + // has no record of which login started it and gets the default + // one. Consistent with "indistinguishable from one typed by + // hand", since a hand-typed `dl ` names no profile either. + claude_profile: None, }, ); if pane_shell::no_session_ran(ending) { @@ -154,7 +161,11 @@ pub(crate) fn dispatch( // hangup is asked *here* rather than inside either of them: a picked batch // is one command over several workspaces, and the shell it was typed in is // hung up once, when the last of them has gone. See [`crate::hangup`]. - Command::Select { verb, devcontainer } => { + Command::Select { + verb, + devcontainer, + claude_profile, + } => { let after = verb.after_removal(); let ending = render_select( runner, @@ -163,6 +174,7 @@ pub(crate) fn dispatch( refresh, verb, devcontainer.as_ref(), + claude_profile.as_deref(), ); hangup::after_the_command(after, ending) } @@ -170,6 +182,7 @@ pub(crate) fn dispatch( target, verb, devcontainer, + claude_profile, } => { let after = verb.after_removal(); let ending = render_workspace( @@ -180,6 +193,7 @@ pub(crate) fn dispatch( &target, verb, devcontainer.as_ref(), + claude_profile.as_deref(), // A target named on the command line is resolved by the launch // itself; only the picker arrives knowing more than it says. None, @@ -632,6 +646,7 @@ fn render_workspace<'r>( target: &str, verb: Verb, devcontainer: Option<&DevcontainerPath>, + claude_profile: Option<&str>, recognised: Option, ) -> Ending { // The open's own notices are said where they happen, by the same printer every @@ -647,14 +662,17 @@ fn render_workspace<'r>( match launch::family(&verb) { Family::Stop => { devcontainer_ignored(devcontainer.is_some(), word); + claude_profile_ignored(claude_profile.is_some(), word); render_stop(runner, context, refresh, &mut cold, target) } Family::Kill => { devcontainer_ignored(devcontainer.is_some(), word); + claude_profile_ignored(claude_profile.is_some(), word); render_kill(runner, context, cache, refresh, &mut cold, target, word) } Family::Remove { force } => { devcontainer_ignored(devcontainer.is_some(), word); + claude_profile_ignored(claude_profile.is_some(), word); render_remove( runner, context, @@ -677,6 +695,7 @@ fn render_workspace<'r>( target, &launched, devcontainer, + claude_profile, recognised, ); after_the_session(runner, context, cache, refresh, &mut cold, target, rm, ran) @@ -803,6 +822,19 @@ fn after_the_session<'r>( /// A config choice a verb that opens no workspace cannot honour, said rather than /// discarded. +/// Say that `--claude-profile` was given to a verb that forwards no Claude login. +/// +/// Reported rather than refused, which is the line `devcontainer_ignored` already +/// draws for the same three families: a global command has no workspace for the flag +/// to be about, while `dl stop --claude-profile work` names a real workspace and +/// a flag that does nothing to it. Refusing that would break a recalled line for no +/// gain, and silence would let somebody believe an account had been chosen. +fn claude_profile_ignored(given: bool, verb: &str) { + if given { + eprintln!("Ignoring --claude-profile: '{verb}' forwards no Claude login."); + } +} + fn devcontainer_ignored(given: bool, verb: &str) { if given { eprintln!("Ignoring --devcontainer: it does not apply to '{verb}'."); @@ -1457,6 +1489,7 @@ fn render_select<'r>( refresh: &mut Refresh<'_>, verb: Verb, devcontainer: Option<&DevcontainerPath>, + claude_profile: Option<&str>, ) -> Ending { let workspaces = match context.workspaces() { Err(refused) => return refuse_listing(&refused), @@ -1494,6 +1527,7 @@ fn render_select<'r>( &pick.workspace_id, verb.clone(), devcontainer, + claude_profile, // The picker knows what it drew: this row's clone said it is // this triple, and the launch it is about to start knows only // the id. See `Launch::recognised_as`. diff --git a/rust/dl/src/launch.rs b/rust/dl/src/launch.rs index 794f1d26..d1e74ac2 100644 --- a/rust/dl/src/launch.rs +++ b/rust/dl/src/launch.rs @@ -141,6 +141,7 @@ pub(crate) fn render_launch<'r>( target: &str, verb: &LaunchVerb, devcontainer: Option<&DevcontainerPath>, + claude_profile: Option<&str>, recognised: Option, ) -> Ran { // A path or git source whose derived id is empty — `dl /`, `//`, `/.`, `/..`, @@ -164,7 +165,11 @@ pub(crate) fn render_launch<'r>( reached: Reached::Nothing, }; } - let host = Host::from_process(cache); + // Named on this line and applied to this run only: it is deliberately not + // stored with the workspace, unlike `--devcontainer`, so no workspace can + // silently forward an account chosen weeks ago. The name is carried as typed; + // `clients::claude` owns the check and the refusal. + let host = Host::from_process(cache).with_claude_profile(claude_profile.map(str::to_owned)); // The pass's events stream through a sink of their own, and it is the same // printer as the launch's notices: one line on stderr at the moment core makes // the event, which is Python's order. A cold install streams hundreds of diff --git a/rust/dl/src/lib.rs b/rust/dl/src/lib.rs index 7c857855..5316fa8f 100644 --- a/rust/dl/src/lib.rs +++ b/rust/dl/src/lib.rs @@ -485,7 +485,7 @@ fn one_command(argv: &[String]) -> i32 { // parse because Python asks it there too — a command line dl goes on to refuse // has still warmed the cache. let wanted = - completion_cache::wants_startup_cache_refresh(&cli::argv_without_devcontainer(argv)); + completion_cache::wants_startup_cache_refresh(&cli::argv_without_value_flags(argv)); // One process, one cache directory, and one background refresh. Both are // resolved out here rather than per command: two halves of one run that @@ -595,6 +595,9 @@ fn grammar_refusal(refused: &cli::GrammarError) -> String { cli::GrammarError::DevcontainerNotAllowed { command } => { format!("--devcontainer means nothing for {command}: it opens no workspace.") } + cli::GrammarError::ClaudeProfileNotAllowed { command } => { + format!("--claude-profile means nothing for {command}: it forwards no Claude login.") + } // The two forms it *does* apply to are named, and so is what to type to // delete a workspace now: somebody who reached for `--rm` on another verb // wants the workspace gone at some point, and this sentence is where they From 9ed007fb819d77ce5ddaec5ebf513c79605fed33 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Wed, 2 Sep 2026 17:57:21 +0100 Subject: [PATCH 2/6] fix: read the Claude profiles that already exist instead of a new root The first version of --claude-profile put profiles under `config_home()/devlaunch/claude-profiles`, which was wrong for a reason no test could catch: `claude-as` on this machine already manages per-account CLAUDE_CONFIG_DIRs under `~/.claude-profiles`, with two logged-in accounts in it. A devlaunch-shaped root made a third location for one concept and would have asked anyone with working profiles to log every account in again somewhere new. So the layout and the variable are honoured rather than set, the arrangement dl already has with devpod's DEVPOD_SSH_CONFIG and with Claude Code's own CLAUDE_CONFIG_DIR: $DEVLAUNCH_CLAUDE_PROFILES_DIR devlaunch's own, and it wins $CLAUDE_PROFILES_DIR the managing tool's, honoured ~/.claude-profiles its default devlaunch's override stays on top because it is what scopes a scratch run away from real credentials; without it a test would read the machine's actual logins. There is still no writer, and now that is somebody else's job rather than a gap: creating a profile, seeding the config it shares with the main login, and deleting it belong to the tool that made the directory. dl reads one file out of it. The placement argument that put profiles under the config home is unaffected rather than abandoned. --purge deletes devlaunch's cache entire and --prune walks the clones inside it, and `~/.claude-profiles` was never in either path; the test that held it now names the new location. `--claude-profile default` resolves the unnamed credential and never consults a `default/` directory, matching `claude-as default`, which runs claude with no CLAUDE_CONFIG_DIR rather than looking one up. It earns a word rather than being the absence of the flag because a picker needs something to select and a recalled line needs a way to say "not the profile I used last time". It resolves the whole unnamed chain, so it still honours CLAUDE_CONFIG_DIR and still loses to the opt-out. Verified against the real directory: completion offers `default base bear`, the scratch override still scopes to its own, and both real credentials carry the `claudeAiOauth.accessToken` this reads. --- CHANGELOG.md | 28 ++++++--- README.md | 19 ++++-- docs/workspace-tools.md | 40 +++++++++---- rust/devlaunch-core/completions/dl.bash | 15 ++--- rust/devlaunch-core/src/clients/claude.rs | 61 ++++++++++++++++++- rust/devlaunch-core/src/domain/xdg.rs | 73 +++++++++++++++-------- 6 files changed, 178 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fc89891..389b9e99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`dl --claude-profile ` forwards a named Claude login instead of the default one**, for the case one credential cannot serve: two accounts on one machine, and a workspace that wants the one your host is not signed in to. Profiles - are directories under `/devlaunch/claude-profiles/`, each holding the - `.credentials.json` a `claude` login writes. devlaunch reads them and has no writer, - so creating one is Claude Code's job with its own `CLAUDE_CONFIG_DIR`. + are directories under `~/.claude-profiles/`, or wherever `CLAUDE_PROFILES_DIR` points, + each holding the `.credentials.json` a `claude` login writes and each a + `CLAUDE_CONFIG_DIR` of its own, which is what makes the logins independent. + + **That layout and that variable belong to the tool managing the profiles, and `dl` + only reads them.** The first version of this invented a devlaunch-shaped root under + the config directory, which was a third location for one concept and would have asked + anyone with working profiles to log every account in again somewhere new. There is no + writer here: creating, seeding and deleting a profile stay with whatever made the + directory. `DEVLAUNCH_CLAUDE_PROFILES_DIR` still wins over `CLAUDE_PROFILES_DIR`, so a + scratch run reads its own profiles rather than the real credentials. + + `--claude-profile default` resolves the login you would get anyway and never consults + 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". **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 @@ -30,11 +42,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 that has opted out has no account to choose. **Not stored with the workspace**, unlike `--devcontainer`, so no workspace can - quietly forward an account chosen weeks ago. Profiles live under the config directory - and never the cache, so `--purge` and `--prune` cannot reach a login; - `DEVLAUNCH_CLAUDE_PROFILES_DIR` moves them for a scratch run. Completion offers the - profiles that exist, read off the disk rather than the completion cache, because a - profile made a minute ago has to complete now. + quietly forward an account chosen weeks ago. Profiles live outside everything devlaunch owns + so `--purge` and `--prune`, which walk devlaunch's cache, were never in reach of a + login. Completion offers the profiles that exist plus `default`, read off the disk + rather than the completion cache, because a profile made a minute ago has to complete + now. It is not the claude.ai account a container's `claude` is paired to for Remote Control, and it does not weaken the check that leaves a repo's own mounted Claude diff --git a/README.md b/README.md index 2dd4dc43..03ce007f 100644 --- a/README.md +++ b/README.md @@ -290,9 +290,15 @@ workspaces where you want a different account than the one `claude` on your host dl blooop/devlaunch --claude-profile work ``` -Profiles live in `/devlaunch/claude-profiles//`, each holding the -`.credentials.json` that a `claude` login writes. Create one by pointing Claude Code's own -`CLAUDE_CONFIG_DIR` at the directory and logging in there; `dl` reads these and never writes them. +Profiles live in `~/.claude-profiles//`, or wherever `CLAUDE_PROFILES_DIR` points, each +holding the `.credentials.json` that a `claude` login writes. Each is a `CLAUDE_CONFIG_DIR` of its +own, which is what makes the logins independent. `dl` reads that layout rather than inventing one, +so profiles you already have work with no re-login, and it never writes there: creating and +deleting them stays with whatever made the directory. By hand it is +`CLAUDE_CONFIG_DIR=~/.claude-profiles/work claude`, then log in. + +`--claude-profile default` means the login you would get anyway, so a recalled line has a way to +say "not the profile I used last time". Unlike `--devcontainer` it is **not** stored with the workspace, so it applies to the launch you typed it on and no workspace ever forwards an account chosen weeks ago. A name that holds no @@ -436,12 +442,13 @@ Images are yours: `docker system df` is what shows those. | `DEVLAUNCH_TIMING=1\|json` | Write a timing summary to stderr. See [docs/performance.md](docs/performance.md) | | `DEVPOD_SSH_CONFIG=` | devpod's own, honoured rather than set: it is where `devpod up` publishes host aliases, so it is where `dl` looks for them. See [docs/cli.md](docs/cli.md) | | `CLAUDE_CONFIG_DIR=` | Claude Code's own, honoured rather than set: it is where the host keeps its Claude configuration, so it is where `dl` reads the login to forward. It replaces `~/.claude` rather than being tried before it, exactly as Claude Code treats it. See [docs/workspace-tools.md](docs/workspace-tools.md) | -| `DEVLAUNCH_CLAUDE_PROFILES_DIR=` | Where `--claude-profile` looks. Defaults to `/devlaunch/claude-profiles`. Under the config directory and never the cache, so `dl --purge` cannot reach a login | +| `DEVLAUNCH_CLAUDE_PROFILES_DIR=` | Where `--claude-profile` looks, and it wins over the row below. For scoping a scratch run away from real credentials | +| `CLAUDE_PROFILES_DIR=` | Honoured rather than set: the profile directory `claude-as` manages. Defaults to `~/.claude-profiles`. Nothing `dl` deletes reaches it | Every switch here reads the same values: anything but empty, `0`, `false` or `no` counts as -set. On a "no" variable that means turn it off; on an opt-in one it means turn it on. Seven +set. On a "no" variable that means turn it off; on an opt-in one it means turn it on. Eight rows are not switches and do not follow it: `DEVLAUNCH_AID_AGENT`, `DEVPOD_SSH_CONFIG`, -`CLAUDE_CONFIG_DIR` and `DEVLAUNCH_CLAUDE_PROFILES_DIR` take a value, `DEVLAUNCH_TIMING` counts only empty and `0` as off, so `false` and `no` +`CLAUDE_CONFIG_DIR`, `DEVLAUNCH_CLAUDE_PROFILES_DIR` and `CLAUDE_PROFILES_DIR` take a value, `DEVLAUNCH_TIMING` counts only empty and `0` as off, so `false` and `no` turn it on, `DEVLAUNCH_AID_REMOTE_CONTROL` takes `1`/`true`/`on`/`yes` or `0`/`false`/`off`/`no` and refuses anything else rather than guessing, and `HERDR_AGENT` is the one written rather than read, so a value of your own survives only a line that starts diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index 2692239b..eb48c843 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -116,18 +116,34 @@ that wants the one your host is not signed in to. dl owner/repo --claude-profile work ``` -The name is one directory under `/devlaunch/claude-profiles/`, holding the -`.credentials.json` a `claude` login writes. devlaunch reads these and has no writer -for them, so creating one is Claude Code's job: point its own `CLAUDE_CONFIG_DIR` at -the directory and log in there. - -They sit under the **config** directory and never the cache, which is deliberate. -`dl --purge` deletes devlaunch's cache entire and `dl --prune` walks the clones -inside it, so a credential kept anywhere under there would be one flag away from -deletion. A login is not a cache: nothing regenerates it. - -`DEVLAUNCH_CLAUDE_PROFILES_DIR` replaces the location whole, which is what lets a -scratch run complete and read its own profiles rather than the real ones. +The name is one directory under `~/.claude-profiles/`, holding the +`.credentials.json` a `claude` login writes. Each such directory is a +`CLAUDE_CONFIG_DIR` of its own, which is what makes the logins independent. + +**That is somebody else's directory and `dl` only reads it.** The layout and the +`CLAUDE_PROFILES_DIR` variable belong to the tool that manages them, honoured here +rather than set, the same arrangement `dl` has with devpod's own +`DEVPOD_SSH_CONFIG`. An earlier version of this feature invented a devlaunch-shaped +root under the config directory, and that was wrong: it made a third location for one +concept and would have asked anyone with working profiles to log every account in +again somewhere new. + +So there is no writer. Creating a profile, seeding the config it shares with your main +login, and deleting it belong to whatever made the directory; `dl` reads one file out +of it. By hand that is `CLAUDE_CONFIG_DIR=~/.claude-profiles/work claude`, then a +login. + +Nothing `dl` deletes can reach them. `dl --purge` removes devlaunch's cache entire and +`dl --prune` walks the clones inside it, and a login was never in either path. + +Two variables, in this order: `DEVLAUNCH_CLAUDE_PROFILES_DIR` is devlaunch's own and +wins, which is what lets a scratch run read and complete its own profiles rather than +the real credentials; `CLAUDE_PROFILES_DIR` is the managing tool's and is honoured +next. + +`--claude-profile default` resolves the login you would get anyway and never consults +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". **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 diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index 5fc35ace..8db296f9 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -86,14 +86,15 @@ _dl_completion() { # *workspaces*, and a profile you made a minute ago has to complete now. It is one # readdir of a directory holding a handful of entries. # - # Honours DEVLAUNCH_CLAUDE_PROFILES_DIR and XDG_CONFIG_HOME the same way - # `domain::xdg::claude_profiles_root` does, so a scratch run completes its own - # profiles and not the real ones. A mistyped name is a hard refusal at launch, not - # a fallback to the default login, which is what makes completing these worth the - # readdir. + # A mistyped name is a hard refusal at launch rather than a fallback to the default + # login, which is what makes completing these worth the readdir. if [[ "${prev}" == "--claude-profile" ]]; then - local profiles_root="${DEVLAUNCH_CLAUDE_PROFILES_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/devlaunch/claude-profiles}" - local profiles="" pdir + # The same three sources `domain::xdg::claude_profiles_root` reads, in the same + # order: devlaunch's own scratch override, then claude-as's own variable, then + # 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 if [[ -d "${profiles_root}" ]]; then for pdir in "${profiles_root}"/*/; do [[ -d "$pdir" ]] || continue diff --git a/rust/devlaunch-core/src/clients/claude.rs b/rust/devlaunch-core/src/clients/claude.rs index 9cd01a8d..340ffc51 100644 --- a/rust/devlaunch-core/src/clients/claude.rs +++ b/rust/devlaunch-core/src/clients/claude.rs @@ -264,7 +264,11 @@ pub(crate) fn resolve_token( if forwarding_disabled(host.disable.as_deref()) { return TokenLookup::Missing(NoToken::OptedOut); } - if let Some(named) = host.profile.as_deref() { + if let Some(named) = host + .profile + .as_deref() + .filter(|named| *named != DEFAULT_PROFILE) + { return from_profile(named, profiles_root); } if let Some(token) = host.token.as_deref().and_then(Token::parse) { @@ -287,6 +291,16 @@ pub(crate) fn resolve_token( } } +/// The profile name that means "the login this host uses anyway". +/// +/// `claude-as default` runs `claude` with no `CLAUDE_CONFIG_DIR` at all rather than +/// looking for a directory of that name, and this matches it: `--claude-profile +/// default` resolves the unnamed credential below and **never** consults +/// `/default/`, even if one exists. Worth having as a word rather than as the +/// absence of a flag, because a picker needs something to select and a recalled line +/// needs a way to say "not the profile I used last time". +const DEFAULT_PROFILE: &str = "default"; + /// The token a named profile holds, or the reason it holds none. /// /// **Nothing here falls through to another credential**, and that is the whole point @@ -514,6 +528,51 @@ mod tests { ); } + #[test] + fn the_default_profile_names_the_unnamed_credential() { + // `claude-as default` runs claude with no CLAUDE_CONFIG_DIR rather than looking + // for a directory called default, and this matches it. A picker needs a word for + // "the ordinary login", and a recalled line needs a way to say "not the profile + // I used last time". + let home = logged_in("not-a-real-home-token"); + let root = profiles_root_with("default", "not-a-real-directory-token"); + let host = HostEnv { + profile: Some("default".to_owned()), + ..HostEnv::default() + }; + // The home credential, and emphatically not `/default/`, which exists + // here precisely so the test can tell the two apart. + assert_eq!( + resolve_token(Some(home.path()), Some(root.path()), &host), + TokenLookup::Found(Token("not-a-real-home-token".to_owned())) + ); + } + + #[test] + fn the_default_profile_still_honours_the_config_dir_and_the_opt_out() { + // It resolves the unnamed credential, so it picks up everything that decides + // which one that is rather than jumping straight to `$HOME/.claude`. + let moved = credential_dir_holding("not-a-real-moved-token"); + let host = HostEnv { + profile: Some("default".to_owned()), + config_dir: Some(moved.path().display().to_string()), + ..HostEnv::default() + }; + assert_eq!( + resolve_token(None, None, &host), + TokenLookup::Found(Token("not-a-real-moved-token".to_owned())) + ); + let opted_out = HostEnv { + disable: Some("1".to_owned()), + profile: Some("default".to_owned()), + ..HostEnv::default() + }; + assert_eq!( + resolve_token(None, None, &opted_out), + TokenLookup::Missing(NoToken::OptedOut) + ); + } + #[test] fn a_named_profile_that_yields_nothing_refuses_rather_than_falling_back() { // The most important test in the feature. Two accounts on one machine is what diff --git a/rust/devlaunch-core/src/domain/xdg.rs b/rust/devlaunch-core/src/domain/xdg.rs index 41591d24..cec1ed9a 100644 --- a/rust/devlaunch-core/src/domain/xdg.rs +++ b/rust/devlaunch-core/src/domain/xdg.rs @@ -65,41 +65,65 @@ fn devlaunch_cache_in(cache_home: &Path) -> PathBuf { cache_home.join("devlaunch") } -/// Where named Claude profiles live: `config_home()/devlaunch/claude-profiles`. +/// Where named Claude profiles live: `~/.claude-profiles`, or wherever +/// `$CLAUDE_PROFILES_DIR` says. /// -/// **Under the config home, not the cache**, which is the whole reason this is a -/// function. `dl --purge` deletes [`devlaunch_cache`] entire and `dl --prune` walks -/// [`clone_root_in`] inside it, so a credential kept anywhere under the cache would -/// be one flag away from deletion. A login is not a cache: nothing regenerates it. -/// `the_profiles_root_is_nowhere_a_purge_or_a_prune_can_reach` holds that. +/// **This is somebody else's directory and devlaunch only reads it.** The layout is +/// `claude-as`'s (a per-account `CLAUDE_CONFIG_DIR` per subdirectory, each holding the +/// `.credentials.json` a login writes), and the variable is `claude-as`'s too, so it +/// is honoured rather than set -- the arrangement [`super::super::clients::ssh::CONFIG_VAR`] +/// already has with devpod's own. Inventing a devlaunch-shaped root instead was the +/// first version of this and it was wrong: it made a *third* location for one concept +/// and would have asked a user with working profiles to log every account in again +/// somewhere new. /// -/// Beside `config.toml`, which [`super::config`] already puts one directory under the -/// config home, so devlaunch has one configuration directory rather than two. +/// So there is no writer here, deliberately, matching [`super::config`]'s note about +/// `config.toml`. Creating a profile, seeding its shared config and deleting it belong +/// to whatever made the directory; devlaunch reads one file out of it. /// -/// `$DEVLAUNCH_CLAUDE_PROFILES_DIR` replaces it whole, which is what lets a test and -/// a scratch run read and complete their own profiles rather than the real ones. An -/// empty value counts as unset, the rule every variable here follows. +/// **Nothing devlaunch deletes can reach it.** `dl --purge` removes +/// [`devlaunch_cache`] entire and `dl --prune` walks [`clone_root_in`] inside that, so +/// a login was never in their path and still is not: +/// `the_profiles_root_is_nowhere_a_purge_or_a_prune_can_reach` holds it. +/// +/// `$DEVLAUNCH_CLAUDE_PROFILES_DIR` is devlaunch's own and wins over both, which is +/// what lets a test and a scratch run read and complete their own profiles instead of +/// the real credentials. An empty value counts as unset in every case, the rule every +/// variable here follows. pub fn claude_profiles_root() -> Result { - match std::env::var_os(CLAUDE_PROFILES_DIR_VAR) { - Some(value) if !value.is_empty() => Ok(PathBuf::from(value)), - _ => config_home().map(|config| claude_profiles_root_in(&config)), + if let Some(scoped) = non_empty(CLAUDE_PROFILES_DIR_VAR) { + return Ok(PathBuf::from(scoped)); + } + if let Some(theirs) = non_empty(FOREIGN_PROFILES_DIR_VAR) { + return Ok(PathBuf::from(theirs)); } + crate::osext::home_dir() + .map(|home| claude_profiles_root_in(&home)) + .ok_or(NoHomeDirectory) } -/// The placement half of [`claude_profiles_root`], as a function of the config home. +/// The placement half of [`claude_profiles_root`], as a function of the home directory. /// /// Split out for [`devlaunch_cache_in`]'s reason: the decision is then a function of /// its input, so a test can state the machine it means instead of mutating an /// environment every other test in the binary shares. -fn claude_profiles_root_in(config_home: &Path) -> PathBuf { - config_home.join("devlaunch").join(CLAUDE_PROFILES_LEAF) +fn claude_profiles_root_in(home: &Path) -> PathBuf { + home.join(CLAUDE_PROFILES_LEAF) } -/// The scratch override for [`claude_profiles_root`]. +/// A variable's value, if it has one that is not empty. +fn non_empty(name: &str) -> Option { + std::env::var_os(name).filter(|value| !value.is_empty()) +} + +/// devlaunch's own override, which scopes a scratch run away from real credentials. const CLAUDE_PROFILES_DIR_VAR: &str = "DEVLAUNCH_CLAUDE_PROFILES_DIR"; -/// The leaf [`claude_profiles_root`] ends in, named once. -const CLAUDE_PROFILES_LEAF: &str = "claude-profiles"; +/// `claude-as`'s own variable for the same directory, honoured rather than set. +const FOREIGN_PROFILES_DIR_VAR: &str = "CLAUDE_PROFILES_DIR"; + +/// The leaf [`claude_profiles_root`] ends in under `$HOME`, named once. +const CLAUDE_PROFILES_LEAF: &str = ".claude-profiles"; /// The one directory devlaunch clones into, under the cache directory ownership /// is decided by. @@ -207,7 +231,7 @@ mod tests { // under either is one flag away from deletion and nothing regenerates a // login. This asserts the placement rather than trusting whoever moves it // next. - let profiles = claude_profiles_root_in(Path::new("/c")); + let profiles = claude_profiles_root_in(Path::new("/h")); let devlaunch = devlaunch_cache_in(Path::new("/k")); let clones = clone_root_in(&devlaunch); assert!( @@ -217,9 +241,10 @@ mod tests { devlaunch.display() ); assert!(!profiles.starts_with(&clones)); - // And it is beside the other configuration, so devlaunch has one - // configuration directory rather than two. - assert_eq!(profiles, PathBuf::from("/c/devlaunch/claude-profiles")); + // And it is `claude-as`'s directory, not one devlaunch invented: a second + // location for one concept would ask a user with working profiles to log + // every account in again somewhere new. + assert_eq!(profiles, PathBuf::from("/h/.claude-profiles")); } #[test] From 9dbefaa4f72e4a78ee67985d1df47e9a2c36905f Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Fri, 4 Sep 2026 10:41:18 +0100 Subject: [PATCH 3/6] chore: regenerate the public-API snapshots for --claude-profile Two rows, and the first of them is a promise rather than a tripwire. `public-api.api.txt` gains `flows::launch::Host::with_claude_profile`, twice, because the generator renders a promised type's inherent methods under both the `api` re-export section and the module that owns them. It is a builder method on a type `api` re-exports, so an external consumer is entitled to depend on it and a later change to its signature is a break. `public-api.rest.txt` gains `domain::xdg::claude_profiles_root`, which is binary surface: `dl` calls it and nothing promises it. Regenerated with `scripts/public-api-snapshots.sh` on nightly 1.100.0 (2026-09-03) and cargo-public-api 0.52.0, the pin the script names. The script reproduces main's three files byte for byte on that toolchain, which is what makes these two rows the change rather than a rendering difference. --- rust/devlaunch-core/public-api.api.txt | 2 ++ rust/devlaunch-core/public-api.rest.txt | 1 + 2 files changed, 3 insertions(+) diff --git a/rust/devlaunch-core/public-api.api.txt b/rust/devlaunch-core/public-api.api.txt index 123423d8..f3eb866e 100644 --- a/rust/devlaunch-core/public-api.api.txt +++ b/rust/devlaunch-core/public-api.api.txt @@ -315,6 +315,7 @@ impl core::marker::StructuralPartialEq for devlaunch_core::clients::devpod_home: pub struct devlaunch_core::api::Host impl devlaunch_core::flows::launch::Host pub fn devlaunch_core::flows::launch::Host::from_process(impl core::convert::Into) -> Self +pub fn devlaunch_core::flows::launch::Host::with_claude_profile(self, core::option::Option) -> Self impl core::clone::Clone for devlaunch_core::flows::launch::Host pub fn devlaunch_core::flows::launch::Host::clone(&self) -> devlaunch_core::flows::launch::Host impl core::cmp::Eq for devlaunch_core::flows::launch::Host @@ -598,6 +599,7 @@ pub fn devlaunch_core::flows::launch::ColdPath<'r, '_>::recorded(&mut self) -> c pub struct devlaunch_core::flows::launch::Host impl devlaunch_core::flows::launch::Host pub fn devlaunch_core::flows::launch::Host::from_process(impl core::convert::Into) -> Self +pub fn devlaunch_core::flows::launch::Host::with_claude_profile(self, core::option::Option) -> Self impl core::clone::Clone for devlaunch_core::flows::launch::Host pub fn devlaunch_core::flows::launch::Host::clone(&self) -> devlaunch_core::flows::launch::Host impl core::cmp::Eq for devlaunch_core::flows::launch::Host diff --git a/rust/devlaunch-core/public-api.rest.txt b/rust/devlaunch-core/public-api.rest.txt index fa944e56..0725657b 100644 --- a/rust/devlaunch-core/public-api.rest.txt +++ b/rust/devlaunch-core/public-api.rest.txt @@ -641,6 +641,7 @@ impl core::fmt::Debug for devlaunch_core::domain::xdg::NoHomeDirectory pub fn devlaunch_core::domain::xdg::NoHomeDirectory::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::Copy for devlaunch_core::domain::xdg::NoHomeDirectory impl core::marker::StructuralPartialEq for devlaunch_core::domain::xdg::NoHomeDirectory +pub fn devlaunch_core::domain::xdg::claude_profiles_root() -> core::result::Result pub fn devlaunch_core::domain::xdg::clone_root_in(&std::path::Path) -> std::path::PathBuf pub fn devlaunch_core::domain::xdg::config_home() -> core::result::Result pub fn devlaunch_core::domain::xdg::devlaunch_cache() -> core::result::Result From 04cee5a23430cd096f8081f4e528a16e5b4b4858 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Fri, 4 Sep 2026 11:00:28 +0100 Subject: [PATCH 4/6] fix: --help named the profile directory the tree stopped reading Review catch. `dl --help` sent readers to `/devlaunch/claude-profiles//`, the devlaunch-shaped root the commit below it took back out in favour of the one claude-as already uses. The README, docs/workspace-tools.md and the changelog all moved; this string did not, so the one place a reader looks *while typing the flag* was the one place still naming a directory nothing reads. Following it produces a profile the resolver cannot find, which surfaces as a refusal about a profile the user just created -- the least tractable shape this mistake has. Not caught by test_readme_cli_doc.py, and it is worth saying why rather than adding a guard here: that test holds every flag `--help` offers to appearing in the README, which is a check about flag *names*. Prose inside a help string naming a path is a different claim and nothing checks it. A guard that diffed this string against the README would be a third copy of the path. `CLAUDE_PROFILES_DIR` is named too, since it is the override anyone with existing profiles is already using, and "dl reads them and never creates one" replaces the sentence a reader would otherwise have to infer. --- rust/dl/src/cli.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/dl/src/cli.rs b/rust/dl/src/cli.rs index b7cadd29..fa74da4a 100644 --- a/rust/dl/src/cli.rs +++ b/rust/dl/src/cli.rs @@ -610,9 +610,9 @@ pub(crate) struct Cli { #[arg(long, value_name = "VARIANT|PATH")] devcontainer: Option, /// Forward a named Claude login instead of the default one. Profiles live in - /// `/devlaunch/claude-profiles//`. Per launch: unlike - /// `--devcontainer` it is not stored with the workspace, so a workspace never - /// forwards an account chosen weeks ago. + /// `~/.claude-profiles//`, or under `CLAUDE_PROFILES_DIR`; dl reads them + /// and never creates one. Per launch: unlike `--devcontainer` it is not stored + /// with the workspace, so a workspace never forwards an account chosen weeks ago. #[arg(long = "claude-profile", value_name = "NAME")] claude_profile: Option, /// Delete the workspace once the session ends, like `docker run --rm`. Only From fe1369b1358a6ba3af78618d6243d79cd9aa7a3d Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Fri, 4 Sep 2026 11:07:00 +0100 Subject: [PATCH 5/6] fix: the profile refusal was built, carried and thrown away Review catch, and the one that matters: `--claude-profile typo` did not stop a launch. It started a session forwarding no Claude token at all, and said nothing. `clients::claude` builds `NoToken::ProfileUnreadable` and `ProfileNotAName` with some care, and the doc comment where the first is declared calls it "the one refusal that must **stop** a launch's forwarding rather than quietly leaving it unforwarded". Nothing outside that module ever read a `NoToken`: `forwarded_claude` matched `TokenLookup::Missing(_) => None` and every reason went the same way as a host that has simply never run `claude`. So the refusal existed as a value and as three paragraphs of documentation, and the behaviour was the fallback those paragraphs argue against. Every test around it passed, because they all asked `resolve_token` and none asked the launch -- which is the useful lesson here: a unit test of the thing that decides is not a test that anybody acts on the decision. `forwarded_claude` now returns `Result, SessionRefused>`. Both call sites resolve it *before* building an argv, so a refusal leaves devpod and ssh unrun rather than starting a session and complaining: a session that started would already contain an agent asking for a login. Only a named profile refuses. `Ok(None)` stays the answer for no credential file, a macOS keychain login, `DEVLAUNCH_NO_CLAUDE_TOKEN`, and a pass that has not answered -- none of those named an account for this launch, and warning about them would fire on every launch of every host that does not use Claude. The `Foreign` check still comes first and is untouched: a container with its own mounted Claude config forwards nothing and refuses nothing, because forwarding the host's short-lived token over a credential that can refresh itself is wrong whether or not a profile was named. Three problems rather than one string, because the fix differs and only one of them has somewhere to point: a name that could not be a directory component, a host that resolves no profiles root at all, and a directory with no readable credential. The third carries the **directory** and not the credential file the client looked for, because that string is handed back inside a `CLAUDE_CONFIG_DIR=...` and a message telling somebody to point that at a `.json` file would be wrong. `no_refusal_message_carries_a_credential` asserts what the messages do not say. This is the one refusal path with a token in scope a call away, so it is worth an assertion rather than a reading. --- rust/devlaunch-core/src/flows/launch.rs | 262 +++++++++++++++++++++++- rust/dl/src/render.rs | 114 ++++++++++- 2 files changed, 368 insertions(+), 8 deletions(-) diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index 3c37b41a..57d32b47 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -1908,6 +1908,49 @@ pub enum SessionRefused { UnsafeRequest(ssh::UnsafeRequest), /// The command could not be made into a shell word. Unquotable(UnquotableCommand), + /// A named Claude profile yielded no token, so the launch stops here. + /// + /// **The one credential lookup on this path that refuses rather than shrugging.** + /// Every other way of having no Claude token leaves the session unforwarded and + /// says nothing, deliberately: a host that never ran `claude`, a macOS host whose + /// login is in the keychain, and an opted-out host are all ordinary. A profile is + /// different because the user named an account *for this launch*, and the two + /// silent outcomes are both worse than stopping -- forwarding the default login + /// means working as the wrong identity, and forwarding nothing means an agent that + /// asks for a login while the account it was told to use sits on disk. + /// + /// Carries the name as typed and the directory searched. Neither is a secret: a + /// profile name is a directory component the user chose, and the path is where a + /// credential would be rather than anything read out of one. + ClaudeProfile { + /// The profile name, as typed. + name: String, + /// Which way it failed, because the three want different sentences. + problem: ClaudeProfileProblem, + }, +} + +/// Why a named Claude profile yielded nothing. +/// +/// Three arms rather than one string, because the fix differs: one is a rule about the +/// name, one is a host with nowhere to look, and one is a directory with no login in it +/// -- and only the third has somewhere to point at. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ClaudeProfileProblem { + /// The name could not be a single directory component. + NotAName, + /// This host resolves no profiles directory at all: no home directory and no + /// override. A profile was still typed, so this is a refusal rather than the + /// ordinary "not logged in". + NoRoot, + /// The directory resolved and held no readable credential. + NoCredential { + /// The directory searched, which is also the `CLAUDE_CONFIG_DIR` a `claude` + /// login into this profile would need. Not the credential file: a message + /// telling somebody to point `CLAUDE_CONFIG_DIR` at a `.json` file would be + /// wrong, and the directory is the thing they can act on. + directory: String, + }, } /// What a session is opened through. @@ -1956,17 +1999,54 @@ impl<'a> SessionContext<'a> { /// into that would override a credential that can refresh itself with one that /// cannot -- Claude Code prefers the variable over the file. `None` is not /// knowing, which gets the same answer for the same reason. - fn forwarded_claude(&self) -> Option { + /// The Claude token this session forwards, or the refusal that stops it. + /// + /// # Why one of the misses is an error and the rest are not + /// + /// `Ok(None)` is the ordinary "nothing to forward": no credential file, a macOS + /// keychain login, `DEVLAUNCH_NO_CLAUDE_TOKEN`, or a mounted config this container + /// owns rather than the host. None of those is worth a word, and warning about + /// them would fire on every launch of a host that simply does not use Claude. + /// + /// A **named profile** that yields nothing is the exception, and + /// [`claude::NoToken::ProfileUnreadable`] says why where it is declared: the user + /// named an account for this launch, so the two silent outcomes are both wrong in + /// a way that surfaces later and somewhere else. + /// + /// Note the order: the `Foreign` check comes first, so a container with its own + /// mounted Claude config still forwards nothing and still refuses nothing. That is + /// deliberate and is not this arm's business -- forwarding the host's short-lived + /// token over a credential that can refresh itself is the case `ClaudeConfig` + /// exists to prevent, and it is wrong whether or not a profile was named. + fn forwarded_claude(&self) -> Result, SessionRefused> { if self.claude_seen.get() != Some(ClaudeConfig::Ours) { - return None; + return Ok(None); } match claude::resolve_token( self.host.home.as_deref(), self.host.claude_profiles_root.as_deref(), &self.host.claude, ) { - claude::TokenLookup::Found(token) => Some(token), - claude::TokenLookup::Missing(_) => None, + claude::TokenLookup::Found(token) => Ok(Some(token)), + claude::TokenLookup::Missing(claude::NoToken::ProfileUnreadable { name, path }) => { + // `path` is the credential *file* the client looked for, and empty when + // no root resolved at all. A message wants the directory: it is what + // `CLAUDE_CONFIG_DIR` takes, and it is what the user made. + let problem = match Path::new(&path).parent() { + Some(directory) if !path.is_empty() => ClaudeProfileProblem::NoCredential { + directory: directory.display().to_string(), + }, + _ => ClaudeProfileProblem::NoRoot, + }; + Err(SessionRefused::ClaudeProfile { name, problem }) + } + claude::TokenLookup::Missing(claude::NoToken::ProfileNotAName(name)) => { + Err(SessionRefused::ClaudeProfile { + name, + problem: ClaudeProfileProblem::NotAName, + }) + } + claude::TokenLookup::Missing(_) => Ok(None), } } } @@ -2158,9 +2238,13 @@ fn devpod_session( args.push("--command".to_owned()); args.push(payload.as_str().to_owned()); } + // Resolved before the argv is built, so a refusal costs no process: `?` here + // leaves devpod unrun rather than spawning a session that would forward the wrong + // account. + let claude_token = session.forwarded_claude()?; let forwarding = claude::extend_ssh_forwarding( gh::ssh_forwarding(session.forwarded_token(notices)), - session.forwarded_claude().as_ref(), + claude_token.as_ref(), ); args.extend(forwarding.args.iter().cloned()); // `--set-env` and not `--send-env`: these are the container's own paths for a @@ -2214,10 +2298,11 @@ fn ssh_with_terminal( // permit list `Reuse::derive` keys the control socket on is the same list the // two credentials built (`clients::herdr`). The manager's coordinates, unlike // the name, do cross the transport and so do join that list. + let claude_token = session.forwarded_claude()?; let forwarding = herdr::extend_openssh_forwarding( claude::extend_openssh_forwarding( gh::openssh_forwarding(session.forwarded_token(notices)), - session.forwarded_claude().as_ref(), + claude_token.as_ref(), ), visible.agent, ); @@ -4324,6 +4409,33 @@ mod tests { self } + /// A host that was told to forward a named Claude profile. + /// + /// `logged_in` is the whole point of the switch: a profile directory that + /// exists with no `.credentials.json` in it is the ordinary state right after + /// something created one, and it is the state a typo produces too. + fn naming_a_claude_profile(mut self, name: &str, logged_in: bool) -> Self { + let root = self.dir.path().join("claude-profiles"); + let profile = root.join(name); + std::fs::create_dir_all(&profile).expect("a profile directory"); + if logged_in { + // `not-a-real-token` and not a realistic `sk-ant-...`, matching what + // the rest of this repo's fixtures use. A literal shaped like an + // Anthropic key is flagged by the secret scanner on every pull request + // that carries it, and a fixture is not worth a standing false positive + // -- `Token::parse` accepts any flat ASCII string, so the shape buys + // the test nothing. + std::fs::write( + profile.join(".credentials.json"), + r#"{"claudeAiOauth":{"accessToken":"not-a-real-token-from-the-profile"}}"#, + ) + .expect("a credential"); + } + self.host.claude_profiles_root = Some(root); + self.host.claude.profile = Some(name.to_owned()); + self + } + fn with_stopped(self, workspace_id: &str) -> Self { self.runner .add_workspace(workspace_id, WorkspaceState::Stopped); @@ -6679,6 +6791,144 @@ mod tests { (session, notices, said) } + /// A session on a container whose Claude config is its own. + /// + /// [`ClaudeSeen`] starts empty and empty forwards nothing, so a test that wants to + /// watch the credential path at all has to say a pass answered. That is not a + /// convenience: it is the same reason the refusal cannot fire before provisioning + /// has looked, and a helper that hid it would let a test pass while asserting + /// nothing. + fn a_session_on_our_own_claude( + scene: &Scene, + command: Option<&str>, + ) -> Result { + let token = HostToken::new(); + let mut notices = no_notices(); + let claude_seen = ClaudeSeen::new(); + claude_seen.set(Some(ClaudeConfig::Ours)); + let context = SessionContext::new(&scene.runner, &scene.host, &token, &claude_seen); + workspace_ssh(&context, "myws", command, None, &mut |_| {}, &mut notices) + } + + /// The refusal `clients::claude` builds and nothing used to read. + /// + /// `NoToken::ProfileUnreadable` is documented where it is declared as "the one + /// refusal that must stop a launch", and it was constructed, carried and dropped: + /// `forwarded_claude` mapped every `Missing` to `None`, so `--claude-profile + /// typo` started a session forwarding no token at all while the flag reported + /// nothing. Every unit test around it passed, because they all asked + /// `resolve_token` and none asked the launch. + #[test] + fn a_named_profile_with_no_credential_stops_the_session() { + let scene = Scene::new() + .on_a_terminal(&["myws"]) + .with_running("myws") + .naming_a_claude_profile("work", false); + + match a_session_on_our_own_claude(&scene, Some("claude")) { + Err(SessionRefused::ClaudeProfile { name, problem }) => { + assert_eq!(name, "work"); + match problem { + ClaudeProfileProblem::NoCredential { directory } => assert!( + directory.ends_with("claude-profiles/work"), + // The directory and not the credential file: this string is + // what the message hands back as a `CLAUDE_CONFIG_DIR`. + "{directory}" + ), + other => panic!("expected NoCredential, got {other:?}"), + } + } + other => panic!("expected a refusal, got {other:?}"), + } + } + + #[test] + fn a_refused_profile_spawns_nothing_at_all() { + // The half that makes it a refusal rather than a message: the token is resolved + // before the argv is built, so nothing reaches ssh or devpod. A session that + // started and then complained would already have an agent in it asking for a + // login. + let scene = Scene::new() + .on_a_terminal(&["myws"]) + .with_running("myws") + .naming_a_claude_profile("work", false); + + let _ = a_session_on_our_own_claude(&scene, Some("claude")); + + assert!(scene.runner.calls_to("ssh").is_empty(), "openssh was run"); + assert!( + scene + .runner + .calls_to("devpod") + .iter() + .all(|call| { call.invocation().argv().get(1).map(String::as_str) != Some("ssh") }), + "devpod ssh was run" + ); + } + + #[test] + fn a_named_profile_that_is_logged_in_forwards_its_own_token() { + // The other side of the same branch, so the refusal cannot be satisfied by + // refusing everything. + let scene = Scene::new() + .on_a_terminal(&["myws"]) + .with_running("myws") + .naming_a_claude_profile("work", true); + + a_session_on_our_own_claude(&scene, Some("claude")).expect("a session"); + + let calls = scene.runner.calls_to("ssh"); + let call = calls.last().expect("an openssh session"); + assert_eq!( + call.invocation() + .env + .entries + .get("CLAUDE_CODE_OAUTH_TOKEN") + .map(String::as_str), + Some("not-a-real-token-from-the-profile"), + ); + } + + #[test] + fn an_ordinary_missing_login_stays_silent_and_starts_the_session() { + // The behaviour the refusal must not swallow. A host that never ran `claude`, + // a macOS host whose login is in the keychain and an opted-out host are all + // ordinary, and none of them named an account for this launch. Warning here + // would fire on every launch of every host that does not use Claude. + let scene = Scene::new().on_a_terminal(&["myws"]).with_running("myws"); + + a_session_on_our_own_claude(&scene, Some("echo hi")).expect("a session"); + + let calls = scene.runner.calls_to("ssh"); + let call = calls.last().expect("an openssh session"); + assert!( + !call + .invocation() + .env + .entries + .contains_key("CLAUDE_CODE_OAUTH_TOKEN"), + "nothing to forward, and nothing forwarded", + ); + } + + #[test] + fn a_profile_name_that_could_not_be_a_directory_names_the_rule() { + let mut scene = Scene::new().on_a_terminal(&["myws"]).with_running("myws"); + scene.host.claude_profiles_root = Some(scene.dir.path().join("claude-profiles")); + scene.host.claude.profile = Some("../../etc".to_owned()); + + match a_session_on_our_own_claude(&scene, Some("claude")) { + Err(SessionRefused::ClaudeProfile { name, problem }) => { + assert_eq!(name, "../../etc"); + // No directory, because the name was refused before one was built from + // it. A path here would mean a traversal had been composed and only + // then failed on a read. + assert_eq!(problem, ClaudeProfileProblem::NotAName); + } + other => panic!("expected a refusal, got {other:?}"), + } + } + /// The wiring `clients::herdr`'s own tests cannot reach. /// /// Its unit tests know that `claude` is an agent and that the name belongs in diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index 2396721b..6248bf27 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -33,8 +33,8 @@ use devlaunch_core::flows::kill::{ TableUnreadable, }; use devlaunch_core::flows::launch::{ - BranchNotNamed, ColdRefused, LaunchAborted, LaunchNotice, LaunchRefusal, NotPrepared, - SessionRefused, + BranchNotNamed, ClaudeProfileProblem, ColdRefused, LaunchAborted, LaunchNotice, LaunchRefusal, + NotPrepared, SessionRefused, }; use devlaunch_core::flows::lifecycle::{ Insistence, KeptBecause, LifecycleNotice, NotAdopted, Promotion, PrunePlan, PruneReport, @@ -3175,6 +3175,38 @@ fn session_refusal(refused: &SessionRefused) -> String { SessionRefused::Ssh(SshNotRun::Blocked(failure)) => { format!("error: ssh could not be run ({})", os_error_phrase(failure)) } + // Names what was typed, where it was looked, and the one command that fixes + // it. The directory is worth printing because the usual cause is a profile + // that exists with nothing logged in to it, which reads as "the profile is + // right there, why can dl not see it". + // + // The last sentence is the part that is not obvious from the failure: dl + // could have forwarded the default login and did not, on purpose. + SessionRefused::ClaudeProfile { + name, + problem: ClaudeProfileProblem::NoCredential { directory }, + } => format!( + "error: --claude-profile {name}: no Claude credential in {directory}. Log \ + that profile in with 'CLAUDE_CONFIG_DIR={directory} claude', or drop the \ + flag to forward the default login. Refusing rather than forwarding a \ + different account." + ), + SessionRefused::ClaudeProfile { + name, + problem: ClaudeProfileProblem::NotAName, + } => format!( + "error: --claude-profile {}: a profile name is one directory component of \ + letters, digits, '.', '_' and '-', and cannot begin with '.' or '-'.", + python_repr(name) + ), + SessionRefused::ClaudeProfile { + name, + problem: ClaudeProfileProblem::NoRoot, + } => format!( + "error: --claude-profile {name}: this host resolves no Claude profiles \ + directory, because it names no home directory and set neither \ + CLAUDE_PROFILES_DIR nor DEVLAUNCH_CLAUDE_PROFILES_DIR." + ), } } @@ -3264,6 +3296,84 @@ mod tests { use super::*; + /// The three sentences a refused `--claude-profile` produces. + /// + /// Worth pinning as text rather than as "it errored", because the whole argument + /// for refusing instead of falling back is that the person reading this can act on + /// it. A refusal nobody can act on is a worse outcome than the silent fallback it + /// replaced. + #[test] + fn a_refused_profile_says_what_to_do_about_it() { + let no_credential = session_refusal(&SessionRefused::ClaudeProfile { + name: "work".to_owned(), + problem: ClaudeProfileProblem::NoCredential { + directory: "/home/me/.claude-profiles/work".to_owned(), + }, + }); + assert!( + no_credential.starts_with("error: --claude-profile work: "), + "{no_credential}" + ); + // The directory, twice: once as where it looked and once inside the command + // that fixes it. `CLAUDE_CONFIG_DIR` takes a directory, so a message naming + // the credential file here would be telling somebody to point it at a .json. + assert!( + no_credential.contains("CLAUDE_CONFIG_DIR=/home/me/.claude-profiles/work claude"), + "{no_credential}" + ); + // And the fact that is not deducible from the failure itself. + assert!( + no_credential.contains("Refusing rather than forwarding"), + "{no_credential}" + ); + + let not_a_name = session_refusal(&SessionRefused::ClaudeProfile { + name: "../../etc".to_owned(), + problem: ClaudeProfileProblem::NotAName, + }); + // Quoted, so a name full of dots and slashes reads as one argument rather than + // as prose that happens to contain them. + assert!(not_a_name.contains("'../../etc'"), "{not_a_name}"); + assert!( + not_a_name.contains("one directory component"), + "{not_a_name}" + ); + + let no_root = session_refusal(&SessionRefused::ClaudeProfile { + name: "work".to_owned(), + problem: ClaudeProfileProblem::NoRoot, + }); + // Names both variables, because a host in this state has set neither and + // either one answers it. + assert!(no_root.contains("CLAUDE_PROFILES_DIR"), "{no_root}"); + assert!( + no_root.contains("DEVLAUNCH_CLAUDE_PROFILES_DIR"), + "{no_root}" + ); + } + + #[test] + fn no_refusal_message_carries_a_credential() { + // The refusal is built from a name the user typed and a directory they made, + // and neither is a secret -- but this is the one path that has a token in + // scope one call away, so it is worth an assertion rather than a reading. + for problem in [ + ClaudeProfileProblem::NotAName, + ClaudeProfileProblem::NoRoot, + ClaudeProfileProblem::NoCredential { + directory: "/home/me/.claude-profiles/work".to_owned(), + }, + ] { + let said = session_refusal(&SessionRefused::ClaudeProfile { + name: "work".to_owned(), + problem, + }); + assert!(!said.contains("sk-ant"), "{said}"); + assert!(!said.contains("accessToken"), "{said}"); + assert!(!said.contains(".credentials.json"), "{said}"); + } + } + fn row(id: &str, kind: SourceKind, detail: &str, size: SizeCell, when: LastUsed) -> TableRow { TableRow { id: id.to_owned(), From da3ec2cf0d6d6912e30cde46e495dd8620510c42 Mon Sep 17 00:00:00 2001 From: Joshua Smith Date: Fri, 4 Sep 2026 11:25:03 +0100 Subject: [PATCH 6/6] chore: the refusal's own two public types Fourteen rows in the tripwire file, from making the profile refusal real: `SessionRefused` gains a `ClaudeProfile` variant and `ClaudeProfileProblem` is a new public enum beside it. Both are `pub` with `String` fields on purpose. The obvious shape was `ClaudeProfile(claude::NoToken)`, carrying the client's own reason straight through -- and `clients::claude` is `pub(crate)`, so that would have put a crate-private type inside a public variant and produced exactly the wart the `--claude-profiles` branch fixes one commit later: a field a caller can read and whose type they cannot name. Regenerated with `scripts/public-api-snapshots.sh` on nightly 1.100.0 (2026-09-03) and cargo-public-api 0.52.0, the pin the script names. --- rust/devlaunch-core/public-api.rest.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/rust/devlaunch-core/public-api.rest.txt b/rust/devlaunch-core/public-api.rest.txt index 0725657b..11c38147 100644 --- a/rust/devlaunch-core/public-api.rest.txt +++ b/rust/devlaunch-core/public-api.rest.txt @@ -1400,6 +1400,19 @@ pub fn devlaunch_core::flows::launch::BranchNotNamed::eq(&self, &devlaunch_core: impl core::fmt::Debug for devlaunch_core::flows::launch::BranchNotNamed pub fn devlaunch_core::flows::launch::BranchNotNamed::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::launch::BranchNotNamed +pub enum devlaunch_core::flows::launch::ClaudeProfileProblem +pub devlaunch_core::flows::launch::ClaudeProfileProblem::NoCredential +pub devlaunch_core::flows::launch::ClaudeProfileProblem::NoCredential::directory: alloc::string::String +pub devlaunch_core::flows::launch::ClaudeProfileProblem::NoRoot +pub devlaunch_core::flows::launch::ClaudeProfileProblem::NotAName +impl core::clone::Clone for devlaunch_core::flows::launch::ClaudeProfileProblem +pub fn devlaunch_core::flows::launch::ClaudeProfileProblem::clone(&self) -> devlaunch_core::flows::launch::ClaudeProfileProblem +impl core::cmp::Eq for devlaunch_core::flows::launch::ClaudeProfileProblem +impl core::cmp::PartialEq for devlaunch_core::flows::launch::ClaudeProfileProblem +pub fn devlaunch_core::flows::launch::ClaudeProfileProblem::eq(&self, &devlaunch_core::flows::launch::ClaudeProfileProblem) -> bool +impl core::fmt::Debug for devlaunch_core::flows::launch::ClaudeProfileProblem +pub fn devlaunch_core::flows::launch::ClaudeProfileProblem::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::launch::ClaudeProfileProblem pub enum devlaunch_core::flows::launch::HerdrTabRename pub devlaunch_core::flows::launch::HerdrTabRename::Off pub devlaunch_core::flows::launch::HerdrTabRename::Run @@ -1561,6 +1574,9 @@ pub fn devlaunch_core::flows::launch::Session::fmt(&self, &mut core::fmt::Format impl core::marker::Copy for devlaunch_core::flows::launch::Session impl core::marker::StructuralPartialEq for devlaunch_core::flows::launch::Session pub enum devlaunch_core::flows::launch::SessionRefused +pub devlaunch_core::flows::launch::SessionRefused::ClaudeProfile +pub devlaunch_core::flows::launch::SessionRefused::ClaudeProfile::name: alloc::string::String +pub devlaunch_core::flows::launch::SessionRefused::ClaudeProfile::problem: devlaunch_core::flows::launch::ClaudeProfileProblem pub devlaunch_core::flows::launch::SessionRefused::Devpod(devlaunch_core::clients::devpod::NotRun) pub devlaunch_core::flows::launch::SessionRefused::Ssh(devlaunch_core::clients::ssh::NotRun) pub devlaunch_core::flows::launch::SessionRefused::Unquotable(devlaunch_core::flows::launch::UnquotableCommand)