Repository navigation
Conversation
herdr 0.9.5 reports the agent's name in 'name' and the top row in 'terminal_title'. bdi's collector still asked for 'display_agent' and 'title', both with #[serde(default)], so they deserialized to None silently: every lane's name and caption came out blank. Measured on a live 49-pane fleet: display_agent null and title null on all 49 panes, while herdr reported a name and a terminal title for each. The agent's name lands in a new Pane::name field rather than in display_agent, which keeps its existing meaning: the pane's own claim about the bead it is working (collect/agents.rs). title falls back to terminal_title, so captions are drawn again.
GraemeF
left a comment
There was a problem hiding this comment.
Thank you for this, and for such a clear write-up! It's the first pull request beady-eye has had from outside, and it's a good idea: a pane that reports nothing does draw a bare row today, and herdr has better text to offer. I'd love to get it in. A few changes first, and then a couple of nitpicks you're welcome to ignore.
One correction to the premise, because it shapes the rest. herdr 0.9.5 still sends display_agent and title. Its AgentInfo at the v0.9.5 tag has name, title, terminal_title, terminal_title_stripped and display_agent side by side, and each one is left out when it is unset. title and display_agent are set with herdr pane report-metadata, and name is set by herdr agent start or herdr agent rename. So the nulls on your fleet mean those panes never reported metadata or were never named. bdi was reading the right fields, and those fields were simply empty.
That makes this a new fallback for the caption rather than a fix, and I think it's a welcome one.
Changes I'd ask for
- Please use
terminal_title_strippedrather thanterminal_title. The raw title carries the agent's animated spinner glyph, such as✳or◑, so the caption would flicker from frame to frame. herdr strips that glyph in the other field and sends no update when only the spinner moves. - Please put
nameinto the caption too, or leave it out. As the patch stands,Pane::nameis set but nothing reads it. herdr accepts a name wherever it accepts a pane id, so a name is a better last resort than the pane id. The order I'd suggest is: state label, thentitle, thenterminal_title_stripped, thenname, then the pane id. - Please fix the test comment that says the older fields are gone, since they are still there.
- Please update the caption rule in
docs/design.md. It currently says the caption is the state label, then the title, then the pane id. If you'd rather not touch the spec, say so and we'll do it.
Nitpicks, take them or leave them
- I'd keep
titleand the terminal title as separate fields onPane, and pick between them inPane::caption, rather than folding one into the other inAgent::into_pane. That way the whole precedence lives in one place. - The tests use invented names, such as
dunwichand paths under/tmp/bdi-ground/. It would be nice if the new test values followed that style.
Thanks again. Happy to help with any of this.
Every bdi user on current herdr gets blank lane names. bdi is a herdr plugin, and herdr 0.9.5 reports an agent's name in
nameand the row it draws interminal_title. The collector still asks fordisplay_agentandtitle, both declared#[serde(default)], so a current herdr silently deserializes them toNone— no error, no warning, just nothing.Measured on a live 49-pane fleet:
display_agent: nullandtitle: nullon all 49 panes, whileherdr agent listreported a name and a terminal title for every one of them. Captions fall back to nothing, so the dashboard reads as an idle fleet.Change
Agentgainsnameandterminal_title;Panegainsname.titlefalls back toterminal_title, so captions draw again.Pane::name, deliberately not indisplay_agent: that field means "the pane's own claim about the bead it is working" (collect/agents.rs:199), and a lane name is not such a claim. Overloading it would present a lane name to the join as a candidate bead id.display_agentortitlekeeps working.Tests
a_pane_takes_its_name_and_title_from_herdrs_current_fields, parsing herdr's current wire shape (name,terminal_title,kind: "path"session).terminal_title, nothing was reading it.Verification
cargo test --libon this branch produces the identical failure set tomain: 42 failures, all present before this change and all environment-dependent (e.g. thecollect::changestests panic withbdi is watching: Os { code: 2, kind: NotFound }on a machine without that socket), 2444 passing. No new failures.Diff
3 files, +45/−5.