Repository navigation
feat(aid): name the agent so a session manager can see it - #550
Merged
Merged
Conversation
Every aid launch exports HERDR_AGENT=<agent> for the session it opens, so herdr reports a workspace agent as idle, working or blocked instead of nothing at all. The gap was narrower than it looked. A manager first decides which agent a pane holds, usually from the foreground process, and only then matches that agent's rules against the pane's screen. Under aid the foreground processes are aid, ssh and two devpods with no claude anywhere, so the first step failed and took the second with it. The second step was never broken: `dl <ws> -- <agent>` pipes the agent's own TUI through the pane. Only the name was missing. Measured on hardware before and after (#549). It reaches a manager through the ssh child, not through aid: a setenv after start does not rewrite /proc/<pid>/environ, so aid's own row carries nothing and the ssh it spawns carries HERDR_AGENT=claude. herdr walks the pane's foreground processes, which is what makes the child enough. Closes #548.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideThis PR makes Sequence diagram for aid naming an agent for session managerssequenceDiagram
participant User
participant Aid as aid
participant DL as dl
participant SSH as ssh child
participant Manager as Session manager
User->>Aid: launch with selected agent
Aid->>Aid: parsed.agent()
alt agent selected
Aid->>Aid: set_var HERDR_AGENT=<agent>
Aid->>DL: run(&dl_args)
DL->>SSH: exec with inherited HERDR_AGENT
Manager->>SSH: inspect foreground processes
Manager->>Manager: identify agent and classify pane screen
else no agent selected
Aid->>DL: run(&dl_args)
Note over Aid,DL: HERDR_AGENT is unchanged
end
Flow diagram for aid agent environment namingflowchart TD
A[aid parses launch] --> B{Agent selected?}
B -->|Yes| C[Overwrite HERDR_AGENT with selected agent]
C --> D[Invoke dl]
D --> E[ssh child inherits HERDR_AGENT]
E --> F[Session manager identifies agent]
F --> G[Apply agent rules to pane screen]
B -->|No| H[Leave HERDR_AGENT unchanged]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
aid_names_the_agent_it_starts said it held 'every name in aid's own table is one of herdr's ids ... true as agents are added'. The body round-trips set_var over the table's names, which is true of every string and asserts nothing about herdr's ids. No such list exists in this tree to diff against, so the sentence was a second copy of a fact with no guard beside it, in the shape CLAUDE.md's standing rule names. The doc now claims only what the body proves: the name reaches the environment verbatim, which is worth pinning because a transformation fails silently at herdr's end. The convention the sentence was reaching for moves to the AGENTS table in rewrite.rs, where a row is added and where somebody adding one will read it.
name_agent_for_session_manager's doc explained that the variable reaches herdr through the ssh child rather than through aid, and closed the paragraph with (aid_names_the_agent_it_starts). By this repo's convention a cited test name is the guard pinning the behaviour just described, and that test pins nothing of the kind -- it round-trips set_var in one process. The citation resolves, so the guard that checks citations is satisfied while the sentence is misleading, which is the worse of the two failures. Nothing in this tree can hold that fact: it is about a running herdr, a pty and a container. The comment now says so and points at the /proc reading posted on #548, which is where the evidence actually is.
'nothing you set there survives an aid launch' is false on every line aid does not start an agent on: HERDR_AGENT=mine aid <ws> --autorm parses to Task::Retired, agent() is None, and mine survives -- which is what a_line_that_starts_no_agent_leaves_the_environment_alone already asserts, so the README contradicted a test on the same branch. Same for --help, --version and every parse refusal. It also said dl writes the variable, where docs/workspace-tools.md on the same branch says plain dl sets nothing. aid alone writes it, and the sentence now says which lines leave a value of yours standing. The docs page's own 'every aid launch' gets the same qualifier it was relying on a later paragraph to supply.
with_the_environment promised the variable was 'saved and restored, whatever it does', and restored it on the line after body(). An assertion that fails unwinds past a line, so the promise held for exactly the case nobody needs it and broke for the one they do: the failing test leaked its value, and the next failure would be a test that never touched the variable. The restore is a Drop now, so it runs on the unwind, and the guard is held across it. a_body_that_panics_still_restores_the_environment is red without it -- 'a failed test leaked its value into the tests after it' -- and green with it. No victim today, since nothing else in this binary reads the variable. Filed because the comment claimed the guarantee, and the next test to read it would have inherited the bug rather than found it.
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:
|
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every
aidlaunch now exportsHERDR_AGENT=<agent>for the session it opens, soherdr reports a workspace agent as idle, working or blocked
instead of showing nothing at all.
Closes #548, which #549 measured out first.
Why it is this small
The map proposed a generic forwarding facility:
DEVLAUNCH_FORWARD_ENV,DEVLAUNCH_FORWARD_SOCKET, and the lend path reused for a manager's own CLI.The measurement
says none of it is needed, because the gap was narrower than the map thought.
A manager learns what an agent is doing in two steps. It decides which agent a
pane holds, usually from the foreground process, and only then matches that
agent's rules against the pane's screen. Under
aidthe foreground processes areaid,sshand twodevpods with noclaudeanywhere, so the first step failedand took the second with it.
The second step was never broken.
dl <ws> -- <agent>pipes the agent's own TUIthrough the pane, so the pane holds the real screen. Only the name was missing.
Measured, not reasoned about
Both before the change and after, on hardware, because the plausible-looking
version of this is a silent no-op.
It reaches a manager through the ssh child, not through
aid. Asetenvafterstart does not rewrite
/proc/<pid>/environ, which the kernel fixes at exec, so:herdr walks the pane's foreground processes, which is what makes the child enough.
With the patched
aidand nothing in the launching environment:and into a permission form, tracking Claude's own screen through the container:
The scratch workspace was deleted afterwards and
devpod listis back to what itwas.
The three choices worth arguing about
Written on every launch, over whatever was there.
aidis what decideswhich agent starts, so it is the only thing that can be right about it. A
HERDR_AGENT=codexleft in a profile is wrong the momentaid --clauderuns.No detection first. Gating on
HERDR_ENV=1was the alternative. It buysnothing and adds a second thing to be wrong about, which fails silently, which is
the failure this whole issue is about. A machine with no herdr pays one
setenv.aidonly, never plaindl.dl <ws> -- claudetyped by hand is yourcommand, and reading an agent back out of a command tail would be a guess. Same
reasoning that puts
IS_SANDBOXinaid's table rather than indl.What this does not do
HERDR_AGENTis herdr's name, and it is the one manager-specific word in eitherbinary, which #548 explicitly did not want. That constraint was an argument for
the generic facility; with the facility unnecessary, one name is what is left.
A manager with a different override still needs it set some other way.
Nothing is forwarded into the container. The variable is set on the host, for the
host process a manager inspects.
Tests
Three in
rust/aid/src/main.rs, serialised on a mutex and restoring the variable:every name in
aid's table is written verbatim (herdr matches manifest ids, soclaudeclassifies andClaudedoes not), a stale value is replaced, and a linethat starts no agent leaves the environment untouched.
🤖 Generated with Claude Code
Summary by Sourcery
Have
aidname the agent it launches so session managers can identify and monitor the workspace correctly.New Features:
aididentify the selected agent to session managers throughHERDR_AGENT, enabling workspace status reporting such as idle, working, and blocked.Documentation:
HERDR_AGENTbehavior, its scope, and how it interacts withaidanddl.Tests:
Chores: