feat: a herdr pane says which Claude account its agent is running as - #573
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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 GuideThe PR adds profile identity visibility to herdr and carries the live agent's Claude profile into adjacent pane shells by reporting metadata at launch, parsing the profile from the agent pane's argv, and extending the workspace destination to transport the inherited profile while preserving default behavior across pane and process boundaries. Sequence diagram for profile reporting and pane inheritancesequenceDiagram
participant DL as dl
participant AgentPane as Agent pane
participant Herdr as herdr
participant Shell as Adjacent pane shell
DL->>AgentPane: Launch agent with claude_profile
DL->>Herdr: report_profile(pane_id, profile)
Herdr-->>AgentPane: Display profile=name or clear profile
Shell->>AgentPane: Read foreground argv
AgentPane-->>Shell: workspace_id and claude_profile
Shell->>DL: Attach workspace with claude_profile
DL-->>Shell: Start next session using inherited profile
Flow diagram for exact Claude profile detectionflowchart TD
A[Read foreground process argv] --> B{Encounter bare --?}
B -- Yes --> C[No profile]
B -- No --> D{Match --claude-profile or --claude-profile=name?}
D -- Yes --> E[Return profile name]
D -- No --> A
C --> F[Use default login]
E --> G[Inherit profile for adjacent pane]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
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:
|
2c10ced to
cc3f2c3
Compare
GitGuardian's finding on this PR was a false positiveThe check flagged 1 secret in // `[OPTIONS] --source <ID> <PANE_ID>`, with --token NAME=VALUE andNothing was leaked and there is nothing to rotate. The giveaway is that the same sentence forty lines away in It is green now, with the comment written the way its sibling already writes it. A follow-up commit cannot clear one of these. That was my first attempt, and If it fires again on this vocabulary, a repository-level ignore is the right pub(crate) const PROFILE_TOKEN: &str = "profile";
args.push("--token".to_owned());Renaming either to satisfy a scanner would leave the code lying about the CLI it For what it is worth the check was never merge-blocking: |
blooop
left a comment
blooop
left a comment
There was a problem hiding this comment.
The pane label is a good idea, and reading the account off the neighbour's own argv rather than a kept record is the right call for the reason given.
I tried hard to break the argv scraper and mostly could not: --claude-profile as the last element, --claude-profile= with an empty value, a prompt containing the flag (aid joins the prompt into one argument after --), and another flag's value being the literal string all behave. The = branch is load-bearing — aid emits --claude-profile=work via the pass-through at rewrite.rs:647 — so the body's "an exact element match" understates the code rather than the reverse.
What survives is one genuine argv disagreement, plus the label being reported before it is known to be true.
Untested: nothing drives destination_for to a non-None profile (both updated destination tests assert claude_profile: None), and nothing asserts a launch actually calls report-metadata.
§4: two doc-comment insertions landed inside the previous item's block. session_manager.rs:572 puts 26 lines — the transport table, the both_transports_… reference, the ssh_host rationale — onto profile_among's rustdoc, so its published docs claim it reads dl's two transports, and workspace_named_by (:615) is left with none. Same mistake at :1193, where both_transports_name_the_workspace_they_were_built_for's doc now documents the new fn process helper.
Spec axis: no spec available (no Closes #n, no linked issue), so that axis was skipped.
| if let Some(value) = argument.strip_prefix("--claude-profile=") { | ||
| return (!value.is_empty()).then_some(value); | ||
| } | ||
| if argument == "--claude-profile" { |
There was a problem hiding this comment.
profile_named_by returns on the first occurrence; clap takes the last. claude_profile: Option<String> is ArgAction::Set under clap 4 (rust/dl/src/cli.rs:623), which last-wins and does not error on a repeat, and aid forwards both pairs in order (rust/aid/src/rewrite.rs:641-645).
So dl ws --claude-profile work --claude-profile personal runs the agent as personal and hands the sibling pane work. A pane silently signed in as somebody else is the exact failure this PR closes, reached through the reader it adds. Keep scanning and keep the last hit.
There was a problem hiding this comment.
Correction, and this one is mine: this is not a defect and I was wrong to raise it.
I claimed clap keeps the last value for a repeated --claude-profile. It does not accept one at all:
$ dl ws --claude-profile work --claude-profile personal
error: the argument '--claude-profile <NAME>' cannot be used multiple times
Option<String> without ArgAction::Append refuses a repeat outright, so no agent is ever running with two of them in its argv and profile_named_by can never see one. First-versus-last is unreachable, and reading the first is correct as written.
Sorry for the round trip. I checked this by running the binary rather than reasoning about clap, which is what I should have done before posting.
The one thing worth keeping from it: the reader looks like it has a first-versus-last choice to make, and it would genuinely have one if that flag ever gained ArgAction::Append. I have pushed a comment saying why it does not, with a test (a_repeated_profile_never_reaches_a_running_agent) that pins the consequence, so a future change to the flag fails something and says so. No behaviour change.
| // rather than leaving the name of a directory as the only clue. Called | ||
| // even when no profile was named, because a pane is reused and a stale | ||
| // label is worse than none. | ||
| session_manager::report_profile( |
There was a problem hiding this comment.
The stale-label clear fires on one exit out of five. report_profile sits inside the Prepared::Ready arm, but every other exit from begin_reporting — no ssh config (:2174), no published alias (:2186), start_forward failed (:2199), Prepared::Refused (:2219) — returns without it, while the session still opens (:2149-2150: "never fatal … the session opens regardless").
Sequence: pane P runs dl ws --claude-profile work, gets profile=work, exits. The next launch in P is dl ws2 before devpod has published ws2's alias → returns at :2186 → the sidebar still asserts profile=work over a session on the default login. That is the mislabelling the commit message says clearing exists to prevent. Calling report_profile right after herdr::Reporting::resolve succeeds (:2097) would cover all five.
| session_manager::report_profile( | ||
| session.runner, | ||
| &reporting, | ||
| session.host.claude.profile.as_deref(), |
There was a problem hiding this comment.
This passes host.claude.profile — the raw CLI string — and begin_reporting runs at :2098, before forwarded_claude()? at :2253. So the label reports what was asked for, not what was forwarded, and three inputs make it lie:
dl ws --claude-profile wrok→ pane labelledprofile=wrok, then the launch is refused (:2031-2048). A label on a pane with no session, never cleared. Same for../../etc.DEVLAUNCH_NO_CLAUDE_TOKEN=1 dl ws --claude-profile work→OptedOut, nothing forwarded, label sayswork.- A repo whose devcontainer mounts its own Claude config →
claude_seen != Ours→Ok(None)(:2022), nothing forwarded, label sayswork.
Also --claude-profile default, a documented spelling meaning the default login, labels profile=default rather than clearing. Reporting from the resolved outcome instead of the request would make the label mean what the sidebar implies it means.
cc3f2c3 to
bcf6665
Compare
bcf6665 to
629be1d
Compare
629be1d to
30baeb8
Compare
…with `--herdr-shell` re-enters as `dl <ws>`, and it was passing `claude_profile: None`. So a session started with `--claude-profile work` got a sibling pane authenticated as the *default* account, which is the one way this feature could mislead quietly: the pane looks like the agent's shell and is signed in as somebody else. Read from the live sibling rather than remembered, which is what lets it exist at all. A profile is named per launch and deliberately not stored with the workspace, so there is nothing on disk to inherit; the running process is the only record. `pane_destination` is already asking herdr what the tab's panes are running, so the profile costs no round trip -- `profile_among` reads the same `PaneProcessInfo` that `workspace_among` does, and reads it off the *same* pane, so a tab holding two sessions cannot pair one session's workspace with another's account. Two properties of the reader are load-bearing, and one case decides both. `aid <ws> add --claude-profile support to the docs` becomes a dl line whose prompt is a single argument holding that text, and the ssh transport carries the same string as one payload argument. Elements are compared whole, so neither is an argv element equal to `--claude-profile` and neither is read as a flag; and the scan stops at the first bare `--`, which says it a second way and cheaply. A real flag before the `--` is still read, so the stop is a stop and not a refusal. Deliberately not gated on the program name. `dl` on a host may be `dl`, `dl-next` or an absolute path, and matching that family by prefix is exactly the fuzzy test that made `ssh_host` necessary -- a sibling pane running `ssh -F myconf.devpod somehost` once made the pane shell claim a workspace called `myconf`. The pane-level coupling is the stronger guard: the profile is asked only of a pane that has already named a workspace, so an unrelated pane is never consulted. `PaneDestination::Workspace` becomes a struct variant and moves the public-API snapshot.
`dl --claude-profiles` names the account behind each profile, but only where somebody thinks to look. herdr's sidebar is where the panes actually are, and it had the directory name or nothing. So a launch that resolved a `Reporting` also reports the profile as a display-only metadata token, which herdr renders as `$profile` in an agent row. A token rather than `--display-agent`: that field is the agent name `report-agent` sets and the one herdr picks a detection manifest by, so writing a profile into it would corrupt state detection to gain a label. **Clearing matters as much as setting**, and is why this is called with `None` too. A pane is reused. A launch naming `work` followed by one naming nothing would otherwise leave the sidebar asserting an account the running session is not using -- the exact mislabelling `--claude-profiles` exists to prevent, moved somewhere more visible and more trusted. Display only, so it cannot disturb the idle/working/blocked the Claude Code hook reports through the forwarded socket. And reported then tolerated, like everything else in this flow: a herdr that is gone, too old for `report-metadata` or merely slow costs the label and never the session. The signature carries most of that guarantee, since there is no outcome to branch on. Syntax verified against herdr 0.8.2 rather than guessed: the socket-API docs name the method `pane.report_metadata` and document no CLI wrapper, so herdr was installed from conda-forge and asked. `pane report-metadata [OPTIONS] --source <ID> <PANE_ID>`, with `--token NAME=VALUE` and `--clear-token NAME`. The pane id is given first, matching the `report-agent` invocation in HOOK that was measured against a live herdr. A guessed argv would have been a feature that never worked and never said so, since every call in this subsystem ends `>/dev/null 2>&1`. `devlaunch:claude` was a bare literal in HOOK and is now a named constant with the diff test the standing rule asks for.
One row removed and three added, and the removal is the interesting half: `PaneDestination::Workspace` stops being a tuple variant and becomes a struct variant carrying `workspace_id` and `claude_profile`. That is a breaking change to anything that constructs or matches it positionally, and it is in the tripwire file rather than the promise file, so the snapshot is the only place it shows. It is in-tree only today -- `dl` is the sole caller -- which is exactly why it is worth writing down: nothing else would have said so. The variant grew a field rather than gaining a sibling because the profile is a property of the destination, not a different kind of destination: a pane opened beside an agent inherits the account that agent is running as, and a pane opened anywhere else has no account to inherit. 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 behaviours, and the second is the one worth the words: a pane opened beside an agent inherits that agent's account, where before it opened in the right workspace under the wrong login. A workspace is not an account.
…can end **`report_profile` sat in `begin_reporting`'s `Ready` arm, which is one exit out of five.** No ssh config, no published alias, a forward that would not start and a container that refused all return `None` from there, and every one of them still opens the session: those notices are never fatal by design. So a pane that had run `dl ws --claude-profile work` and was then reused for a `dl ws2` whose alias devpod had not published yet kept asserting `profile=work` over a session on the default login. That is exactly the stale label the clearing was added to prevent, on the four paths where clearing never ran. It is reported first now, before anything that can fail. Safe there because the label is host-side: `report_profile` runs herdr's own binary against a pane id and needs neither the forward nor a prepared container. `the_profile_label_is_reported_even_when_the_manager_cannot_be_reached` drives the no-ssh-config arm and fails without the move. **Two doc comments were inserted inside the previous item's block.** In `session_manager.rs`, `profile_among`'s doc was appended to `workspace_named_by`'s, so 26 lines about dl's two transports (the table, the `both_transports_...` reference, the `ssh_host` rationale) became `profile_among`'s published rustdoc and `workspace_named_by` was left with none. The same slip in the test module gave `both_transports_name_the_workspace_they_were_built_for`'s doc to the new `process` helper. Both blocks are back above what they describe; no code moved. **And a note where a reader would look for one.** `profile_named_by` returns the first match and looks like it is choosing between first and last. It is not: `--claude-profile` is an `Option<String>` with no `ArgAction::Append`, so clap refuses a repeat outright and no agent ever runs with two of them in its argv. `a_repeated_profile_never_reaches_a_running_agent` pins the consequence, so a future `ArgAction::Append` on that flag fails something and says so.
30baeb8 to
485162c
Compare
2 participants
Two behaviours, and the second is the one that was actually wrong.
A pane's row says which account the agent in it is running as
Reported to herdr as the display label
profile=<name>. Aprofile is chosen per launch and forwarded per session, so two tabs side by side
can be two different accounts with nothing on screen to tell them apart -- and the
failure that matters here is not noticing.
Reported under devlaunch's own source name, so it sits beside herdr's own labels
rather than overwriting one, and cleared rather than left stale when a launch
forwards the default login.
A pane opened beside an agent inherits that agent's profile
The pane shell shipped in 0.29.0 already opens in the workspace its tab holds. A
workspace is not an account, so it read the default login while the agent one
pane over ran as another -- the right container under the wrong identity, which is
the same class of mistake profiles exist to prevent.
It is read from the agent's own argv rather than from a note kept anywhere, for the
reason the pane shell keeps nothing: the argv is what is true, and a record would be
a second copy of it that can go stale.
Two details in that read, both deliberate:
--. A prompt that containsthe words
--claude-profile workis a prompt.and gating on it would be a second thing to keep in step with the transport.
Nothing is inherited across a boundary
default login.
profile reaches the next session and never the one on screen. Worth stating
because a label makes the opposite look plausible.
Public surface
One row removed and three added, and the removal is the interesting half:
PaneDestination::Workspacestops being a tuple variant and becomes a structvariant carrying
workspace_idandclaude_profile.That is a breaking change to anything constructing or matching it
positionally, and it lives in the tripwire file rather than the promise file, so
the snapshot is the only place it shows.
dlis the sole caller today, which isexactly why it is worth writing down: nothing else would have said so.
The variant grew a field rather than gaining a sibling because the profile is a
property of the destination, not a different kind of destination.
One comment reworded for a secret scanner
flows/session_manager.rsdocuments herdr's argument grammar in a test comment,and it read a bare
--token NAME=VALUE. GitGuardian's Generic CLI Secretdetector reads that as a credential passed on a command line, and the "secret" it
found is the literal placeholder. It now reads
`--token` as `NAME=VALUE`,which is the style the same sentence already uses in
clients/herdr.rsfortylines away -- where it was never flagged.
An accommodation, not a correction. The old wording was accurate; nothing was
leaked and nothing needs rotating. It is folded into the commit that introduced
the line rather than sitting on top as a follow-up, and that detail is the
interesting one: the scan is per commit over the whole pull request, so a later
commit cannot clear a finding in an earlier one. Rewording in a follow-up left
the check red and pointing at the original commit.
If the detector fires again on this vocabulary --
PROFILE_TOKEN, orargs.push("--token")-- the answer is a repository-level ignore rather than morerewording.
tokenis herdr's word for a display label andpane report-metadata --token NAME=VALUEis the commanddlactually runs, socode renamed to dodge a scanner would be lying about the CLI it drives.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AdSFnBdxie6TosHVmjLY28
Summary by Sourcery
Show and propagate the Claude account associated with each running agent so adjacent panes use the correct identity and reused panes never display stale profile information.
New Features:
Bug Fixes:
Enhancements:
--handling.Documentation:
Tests:
Chores: