feat: --claude-profile forwards a named Claude login - #571
Conversation
There was a problem hiding this comment.
Sorry @JSmithRobotics, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 7 days by commenting @sourcery-ai review. Upgrade to get a review now.
Reviewer's GuideAdds per-launch forwarding of named Claude credentials managed under the external profiles directory, with explicit precedence, strict path validation, refusal instead of fallback, launch-level propagation, live completion, and comprehensive documentation and tests. Sequence diagram for named Claude profile launch resolutionsequenceDiagram
participant User
participant CLI
participant Host
participant ClaudeResolver
participant SSH
User->>CLI: --claude-profile name
CLI->>Host: with_claude_profile(Some(name))
Host->>ClaudeResolver: resolve_token(home, profiles_root, host)
alt DEVLAUNCH_NO_CLAUDE_TOKEN enabled
ClaudeResolver-->>Host: OptedOut
Host-->>CLI: no token forwarded
else profile is default
ClaudeResolver->>ClaudeResolver: resolve unnamed credential
ClaudeResolver-->>Host: default token or no token
else named profile
ClaudeResolver->>ClaudeResolver: ProfileName::parse(name)
alt invalid name or missing credential
ClaudeResolver-->>Host: ProfileUnreadable or ProfileNotAName
Host-->>CLI: SessionRefused::ClaudeProfile
CLI-->>User: refuse launch
else credential found
ClaudeResolver-->>Host: profile token
Host->>SSH: extend forwarding with profile token
SSH-->>User: start session
end
end
Flow diagram for Claude profile precedence and refusalflowchart TD
Start[Resolve Claude forwarding] --> OptOut{DEVLAUNCH_NO_CLAUDE_TOKEN?}
OptOut -->|yes| None[Forward nothing]
OptOut -->|no| Profile{Named profile?}
Profile -->|default| Ambient[Resolve ordinary Claude login]
Profile -->|named| Validate{Valid directory component?}
Validate -->|no| Refuse[Refuse launch]
Validate -->|yes| Read[Read profile credentials]
Read --> Found{Credential found?}
Found -->|no| Refuse
Found -->|yes| Forward[Forward profile token]
Ambient --> Token{Credential available?}
Token -->|yes| ForwardDefault[Forward ordinary token]
Token -->|no| None
Forward --> Launch[Build and run SSH or devpod session]
ForwardDefault --> Launch
None --> Launch
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
blooop
left a comment
There was a problem hiding this comment.
Forwards a named login, and the refusal-on-a-named-profile decision is the right one: naming an account and silently getting another is the failure worth stopping.
The findings below are all one theme — the refusal does not cover every path that can drop a named profile, and one message describes a rule the validator does not enforce.
Refuted, for the record: path traversal (ProfileName::parse refuses separators and NUL, and --claude-profile ../../etc is refused), the profile name reaching a command line (only CLAUDE_CODE_OAUTH_TOKEN's name does), and the refresh token travelling.
§4: launch.rs:2002 inserts a second summary sentence mid-doc-comment, so rustdoc renders two openings for forwarded_claude. And rust/aid/src/rewrite.rs:164 still says "Python keeps the list in dl.py (DL_VALUE_OPTIONS)" in the present tense — the Python tree retired in #267 and no dl.py exists on this branch.
Spec axis: no spec available (no Closes #n, no linked issue), so that axis was skipped.
| match claude::resolve_token(self.host.home.as_deref(), &self.host.claude) { | ||
| claude::TokenLookup::Found(token) => Some(token), | ||
| claude::TokenLookup::Missing(_) => None, | ||
| return Ok(None); |
There was a problem hiding this comment.
This gates on claude_seen == Ours before the profile is resolved, so a named profile is still dropped in silence on any path where no pass answered.
Sequence: myws is already up and remembered_claude returns None — a workspace predating the record, NoProvisioning/DEVLAUNCH_NO_TOOLS, or a probe report ClaudeConfig::parse could not read (provision.rs:1289). attach (launch.rs:4186) leaves claude_seen at None. dl myws --claude-profile work then opens the session with no CLAUDE_CODE_OAUTH_TOKEN and no word — and --claude-profile typo does the same, never reaching the refusal.
The Ok(None) doc list at :2005-2008 enumerates four causes and omits this one, and every new launch test goes through a_session_on_our_own_claude, which forces Ours — so nothing exercises a named profile against None or Foreign.
The Foreign ordering is argued as deliberate for forwarding, and that part is fair. But Foreign is the case where the container already holds a mounted credential for some other account, so silence there is the "wrong account, found out later somewhere else" outcome. dl <ws> stop already prints Ignoring --claude-profile (rust/dl/src/commands.rs:833); a notice here would cost nothing and match it.
There was a problem hiding this comment.
Fixed. `LaunchNotice::ClaudeProfileNotForwarded` says it once and forwards nothing, so the ordering you argued for is unchanged and only the silence is gone. A launch that named no profile stays quiet. Two tests: one over both non-`Ours` values of `claude_seen`, one asserting the quiet path.
| problem: ClaudeProfileProblem::NotAName, | ||
| } => format!( | ||
| "error: --claude-profile {}: a profile name is one directory component of \ | ||
| letters, digits, '.', '_' and '-', and cannot begin with '.' or '-'.", |
There was a problem hiding this comment.
"cannot begin with '.' or '-'" — ProfileName::parse refuses a leading -, but for . it refuses only the exact strings . and .. (clients/claude.rs:157-159). So --claude-profile .work is accepted and read from <root>/.work/, and a user refused for an unrelated reason (has space) is told a constraint that is not true.
The PR body has it the way the message does — "leading . (which subsumes . and ..)" — so the code looks like the half that drifted. Replacing raw != "." && raw != ".." with !raw.starts_with('.') makes all three agree and subsumes both special cases.
There was a problem hiding this comment.
Fixed in the code rather than the message, since the branch description already claimed the stricter rule. `ProfileName::parse` now refuses a leading `.`, which subsumes `.` and `..`; `.work` and `.hidden` are in the refusal table.
| if let Some(named) = host | ||
| .profile | ||
| .as_deref() | ||
| .filter(|named| *named != DEFAULT_PROFILE) |
There was a problem hiding this comment.
An on-disk profile literally named default is silently unreachable. README tells users to create one with CLAUDE_CONFIG_DIR=~/.claude-profiles/<name> claude; with <name>=default the directory exists, completion offers default (completions/dl.bash:97 seeds it and :102 re-adds it, so twice), and the launch forwards the unnamed host login instead of the directory the user made.
No refusal, no notice — one more silent wrong-account path, and the one a user is most likely to create by following the documented recipe. Refusing when <root>/default/ exists would close it.
There was a problem hiding this comment.
Not changed, and I think you are right: `DEFAULT_PROFILE`'s own doc commits to matching `claude-as default` (never consult `/default/`), and #572 adds `profile_name_is_offerable` so an on-disk `default` is not offered. Leaving your design as it stands. The completion table still seeds `default` and re-adds it, so it can appear twice on tab, which is cosmetic and separate.
da3ec2c to
bb2b1e0
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
One credential cannot serve a host with two accounts. `--claude-profile <name>` names a directory under `<config>/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.
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.
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.
Review catch. `dl --help` sent readers to `<config>/devlaunch/claude-profiles/<name>/`, 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.
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<Option<Token>, 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.
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.
Two defects in the refusal this branch built, and both are the same shape as the one it was written to fix. **The `claude_seen` gate sits above the profile arm, and said nothing.** Not forwarding the host's short-lived token over a credential that can refresh itself is right, and the ordering is deliberate. Doing it silently is not: that gate is reached with a profile named on three ordinary paths -- a repo whose devcontainer mounts its own Claude config, a workspace older than the provisioning record, and `DEVLAUNCH_NO_TOOLS` or a probe report that would not parse, both of which leave `claude_seen` at `None`. So `dl myws --claude-profile work` on a warm attach opened a session forwarding nothing, as whatever account the container's own configuration holds, and reported it nowhere. `--claude-profile typo` did the same and never reached the refusal. `LaunchNotice::ClaudeProfileNotForwarded` says it once. Still not a refusal, because the not-forwarding is correct; the silence was the defect. A launch that named no profile stays quiet, since a host that does not use Claude has earned no warning on every launch. `dl <ws> stop` already printed this courtesy for the same flag, so the launch path was the odd one out. **`ProfileName::parse` did not enforce the rule its own refusal prints.** The message says a name "cannot begin with '.' or '-'", and the check refused a leading `-` but only the exact strings `.` and `..`. So `.work` was accepted and read from `<root>/.work/`, while anyone refused for an unrelated reason was told a constraint that was false. A leading `.` now goes, which subsumes both special cases and is what this branch's own description already claimed. Nothing creates a hidden profile directory and a profile is a thing a person types, so the rule costs nothing. Also folds away a doubled doc comment on `forwarded_claude`: a second summary sentence had been inserted mid-block, so rustdoc rendered two openings.
bb2b1e0 to
7970fe5
Compare
dl --claude-profile <name>forwards a named Claude login instead of the defaultone, 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.
The layout belongs to the tool that manages the profiles
Profiles are directories under
~/.claude-profiles/, or whereverCLAUDE_PROFILES_DIRpoints, each holding the.credentials.jsonaclaudelogin writes and each a
CLAUDE_CONFIG_DIRof its own, which is what makes thelogins independent.
The first version of this invented a devlaunch-shaped root under the config
directory, and the second commit here takes it back out. That was a third
location for one concept, and it would have asked anyone with working profiles to
log every account in again somewhere new. There is no writer here and none is
coming: creating a profile, seeding the configuration it shares with the main
login, and deleting it stay with whatever made the directory. This reads.
DEVLAUNCH_CLAUDE_PROFILES_DIRstill wins overCLAUDE_PROFILES_DIR, so ascratch run reads its own profiles rather than the real credentials.
The refusal is the feature
A named profile that holds no credential stops the launch. 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 validated at the boundary as a single directory component, so
--claude-profile ../../etcis refused by the rule rather than becoming atraversal that fails later on a read. Empty, leading
-, leading.(whichsubsumes
.and..) and anything outside[A-Za-z0-9_.-]are all refused.Where it sits in the resolution order
Above an exported
CLAUDE_CODE_OAUTH_TOKEN, unlike$CLAUDE_CONFIG_DIR, because aprofile was typed on this command line for this launch and nothing ambient should
beat an explicit argument.
DEVLAUNCH_NO_CLAUDE_TOKENstill comes first: a machinethat has opted out has no account to choose.
--claude-profile defaultresolves the login you would get anyway and neverconsults a
default/directory, so a picker has something to select and a recalledline has a way to say "not the profile I used last time".
What it deliberately does not reach
--devcontainer, so no workspace canquietly forward an account chosen weeks ago. It resolves per session.
--purgeand--prunewalk devlaunch's cache, so a login was never in reach ofeither. There is a test asserting the profiles root is unreachable from both.
claudeis paired to for RemoteControl. Two credentials, and this moves one of them.
Foreignforwards nothing, profileor no profile.
aidpasses the flag through. A verb that forwards no login says it is ignoring it,as
--devcontainerdoes; a global command refuses it. Completion offers theprofiles that exist plus
default, read off the disk rather than the completioncache, because a profile made a minute ago has to complete now.
Public surface
flows::launch::Host::with_claude_profileis a promised row: it is a buildermethod on a type
apire-exports, so its signature is a contract.domain::xdg::claude_profiles_rootis binary surface. The third commit carries theregenerated snapshots, with the toolchain and the reasoning in its message.
Reviewed once already, and it found the important one
Three fixes sit on top of the original two commits, each in its own commit.
The refusal above was documented and not implemented.
forwarded_claudematched
TokenLookup::Missing(_) => None, so every reason went the same way as ahost that has simply never run
claude:--claude-profile typostarted a sessionforwarding no token at all and said nothing.
clients::claudebuiltNoToken::ProfileUnreadablewith care, and the doc comment where it is declaredcalls it "the one refusal that must stop a launch" -- and nothing outside that
module ever read a
NoToken. Every test around it passed, because they all askedresolve_tokenand none asked the launch, which is the useful lesson: a unit testof the thing that decides is not a test that anybody acts on the decision.
It now returns
Result<Option<Token>, SessionRefused>, resolved before eithercall site builds an argv, so a refusal leaves devpod and ssh unrun rather than
starting a session that would already contain an agent asking for a login. Only a
named profile refuses; the ordinary misses stay silent.
dl --helpstill named the abandoned directory. The second commit moved theprofile root to the one claude-as already uses and updated the README, the docs
and the changelog; the clap doc comment kept sending readers to
<config>/devlaunch/claude-profiles/<name>/. Not caught bytest_readme_cli_doc.py, which holds flag names to appearing in the README --prose inside a help string naming a path is a different claim.
A test fixture looked like a real key.
sk-ant-oat01-from-the-profilematchesAnthropic's prefix and tripped the secret scanner. This repo's own fixtures say
not-a-real-tokenfor that reason, andToken::parseaccepts any flat ASCIIstring, so the realistic shape bought the test nothing.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AdSFnBdxie6TosHVmjLY28
Summary by Sourcery
Forward explicitly selected host Claude credentials per launch while refusing ambiguous or unavailable profiles and preserving ownership boundaries around profile storage.
New Features:
--claude-profile <name>to forward credentials from named Claude profiles managed outside devlaunch.aid, workspace launches, and shell completion, including the implicitdefaultprofile.Bug Fixes:
Enhancements:
DEVLAUNCH_CLAUDE_PROFILES_DIRandCLAUDE_PROFILES_DIRwith the documented precedence while keeping profiles outside devlaunch-owned cleanup paths.Documentation:
Tests: