Repository navigation
tui: report agent state, resume command and release to Herdr - #7064
Conversation
Codewhale is now a first-class agent in Herdr. Inside a Herdr pane it reports idle/working/blocked through `"$HERDR_BIN_PATH" pane report-agent`, reports `codewhale resume <id>` so a Herdr server restart reopens the same session in the same pane, and releases the pane when the user quits. See https://herdr.dev/docs/add-herdr-support/. State comes from ShellPhase, the same reducer the footer reads, so Approval and Waiting report blocked while Working and Verifying report working; Typing reports idle because the agent is waiting on the user. Reports run on one dedicated background thread that keeps only the latest pending state and bounds every CLI call with a short timeout, so Herdr can never slow down or break the TUI. Outside Herdr every entry point is a no-op. Herdr before 0.9.2 rejects the `--` resume separator as a usage error; that specific error drops resume for the rest of the process and keeps state reports flowing. New module crates/tui/src/tui/herdr.rs, wired into the event-loop tick and the shutdown path. Adds its docs/features.toml registry row and a CHANGELOG entry. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
github-actions
Bot
added
the
contribution-gate
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
amazing - been meaning to get around to this - thank you so much!! |
WlnJBrn09
left a comment
WlnJBrn09
left a comment
There was a problem hiding this comment.
Score: 59 / 100 — promising first PR, not ready to land
External review against CONTRIBUTING.md. This is a NEW contributor PR (@dsingal0), one purpose, and the Herdr contract is wired the way the upstream docs describe. It is still a draft with required checks red, so it should not merge as-is.
| Gate | Score | Notes |
|---|---|---|
| One purpose | 18/20 | Herdr state / resume / release only |
| Tests with new behavior | 2/15 | ~398-line module, no unit tests |
| Off trust-boundary | 14/15 | Env-gated; only execs HERDR_BIN_PATH |
| CI / process | 4/15 | Lint, Safety, Windows green; three required checks red |
| Implementation | 13/15 | Dedicated worker, coalescing, 2s timeout, resume fallback; two retry/wait issues below |
| Size / focus | 8/10 | 5 files, +422, one commit |
| Ready to merge | 0/10 | Draft + contribution-gate |
What is solid
ShellPhasemapping is correct:Approval/Waitingare “waiting on you”, so they should reportblocked.Typingasidlematches the footer.- One reporter thread, latest-pending coalescing, 2s bound, no-op outside Herdr, and release before the terminal is restored.
- Resume argv checks match Herdr’s rules (PATH name, no apostrophe/control, 64 args / 8 KiB).
- Sending resume on the same
report-agentcall as state is the documented “along with” path.
What blocks a direct merge
- Changelog on the branch. CONTRIBUTING says changelog is written on
mainat merge.Version driftfailed:crates/tui/CHANGELOG.md is out of date with the root CHANGELOG.md slice.Drop theCHANGELOG.mdhunk. - No issue line.
PR closes an issuefailed. AddNo-Issue: Herdr pane agent-state reporting(or a realRefs/Closes). - No tests for
state_for_phase,HerdrEnv::from_lookup,valid_resume_argv,report_args, or the usage-error retry. Missing tests are the usual harvest reason. - Still draft, labeled
contribution-gate.
Test (ubuntu-latest) failed on feature_registry_matches_code for three pre-existing rows (computer-use 153 chars, runtime-api 136, runtime-provider-keys 142). The new Herdr summary is within 120 characters. That failure is not this PR.
Code notes
syncwriteslast_submittedbefore the worker runs the CLI. A failed or timed-out report is treated as sent, so the same state never retries until the phase changes. A longWorkingturn that misses its first report stays invisible in Herdr. Clearlast_submittedon CLI failure (the worker already serializes, so the next tick retries without hammering).release()waits oneCOMMAND_TIMEOUT. If a report is already in flight, in-flight time plus the release CLI can exceed 2s, and the TUI continues shutdown while release is still queued. Wait2 * COMMAND_TIMEOUT, or wait untildonewith a budget that covers both.- Feature row is marked
status = "stable"with nosince. Usesince = "unreleased".stableis fine once it ships (env-gated, always on inside Herdr);previewis more honest while this is draft. - Herdr’s current docs say pre-0.9.2 ignores resume argv so state reports still succeed. This PR assumes clap exit 2 on
--. The retry is harmless if you observed that on a real 0.9.1; worth a one-line note of which version you tested.
Landing path
Strip changelog, add the issue line, add tests, fix the retry/wait issues, then mark ready for review. After that this is a clean community win to merge as itself (not harvest), with credit to @dsingal0.
|
|
||
| ## [Unreleased] | ||
|
|
||
| ### Added |
There was a problem hiding this comment.
[process] Changelog entries are written on main at merge, not in PRs (CONTRIBUTING). This hunk is why Version drift is red (crates/tui/CHANGELOG.md is out of date with the root CHANGELOG.md slice). Please drop it: git checkout origin/main -- CHANGELOG.md.
| }; | ||
| *last = Some(report.clone()); | ||
| report | ||
| }; |
There was a problem hiding this comment.
[bug] last_submitted is updated before the worker runs the CLI. A spawn failure, timeout, or non-success status is then treated as sent, so sync drops the same (state, session_id) until the phase changes. A long Working turn that misses its first report stays invisible in Herdr.
Clear last_submitted on CLI failure. The worker already serializes jobs, so the next tick retries without extra in-flight calls.
|
|
||
| fn worker_loop( | ||
| env: &HerdrEnv, | ||
| slot: &Arc<(Mutex<Slot>, Condvar)>, |
There was a problem hiding this comment.
[suggestion] This wait is one COMMAND_TIMEOUT. If the worker is still inside a report (up to 2s), then has to run release-agent (another 2s), this can return while release is still queued. On process exit the reporter thread is killed and Herdr falls back to the idle-prompt safety net — the case this path is trying to avoid.
Wait until done with a budget of 2 * COMMAND_TIMEOUT, or wait out the in-flight job plus the release call.
| /// Map the TUI's perceptual phase onto Herdr's semantic state. `Typing` is the | ||
| /// user drafting while the agent waits, so it reports `idle`. | ||
| #[must_use] | ||
| pub fn state_for_phase(phase: ShellPhase) -> AgentState { |
There was a problem hiding this comment.
[suggestion] This mapping is right (Waiting / Approval are “waiting on you” in ShellPhase::from_app), but none of it is tested. Please add #[cfg(test)] coverage for state_for_phase, HerdrEnv::from_lookup, valid_resume_argv, report_args, and the usage-error retry. That is the usual reason this size of change gets harvested instead of merged as itself.
|
|
||
| [[feature]] | ||
| id = "herdr-agent-state" | ||
| name = "Herdr agent state" |
There was a problem hiding this comment.
[nit] Add since = "unreleased". stable is reasonable for an env-gated always-on integration once it ships; preview is a better fit while the PR is still draft and untested.
All seven founder-selected original PRs (#7057, #7059-#7064) are merged with their source branches retained. Put Herdr in the dated release, record Dhruv Singal on all three credit surfaces and map his original linked commit address. Regenerate the feature and embedded documentation facts. Admit only foldhash to the portable navigation graph: serde_json 1.0.154 adds it for no_std preserve_order, while std builds retain RandomState; foldhash 0.2.0 has no normal dependencies or build script. Verified the combined source without another native compile: Cargo fmt and dated versions passed; the actual normal graph passed all 13 registrations; all 7 existing navigation checker tests passed; the WhaleNet example typechecked and npm audited 7 packages with zero vulnerabilities; all six reviewed crate checksums match crates.io; blocking-call budget remains 541 sites across 164 files; generated facts, changelog mirror and whitespace checks passed. After merging, all 17 reviewed source files matched their retained hashes exactly. Contributor credit check covers 15 contributors and passes, retaining its two historical coauthor-only notices. Final combined native build, product observations and exact-SHA release CI/RC remain required; the previous 354880 candidate is superseded.
What this adds
Makes Codewhale a first-class agent in Herdr from our own code, with no Herdr-side release required. Inside a Herdr pane (
HERDR_ENV=1plus the pane/bin/socket vars):idle/working/blockedthrough"$HERDR_BIN_PATH" pane report-agent, so the sidebar,herdr agent list, andherdr agent waitall work.codewhale resume, so a Herdr server restart reopens the same session in the same pane.How it works
crates/tui/src/tui/herdr.rs. State is derived fromShellPhase, the same reducer the footer reads:Approval/Waiting→blocked,Working/Verifying→working, andIdle/Typing/Done/Failed→idle.Typingis the user drafting while the agent waits, so it reportsidle.sync) and the shutdown path (release, before the terminal is restored, alongside thesession_end/outbox flush).std::threadsends every report. It keeps only the latest pending state, bounds each CLI call with a 2s timeout, and drops unchanged state, so Herdr can never slow down or break the TUI.releasewaits for the worker off the runtime viaspawn_blocking. Outside Herdr every entry point is a no-op.--resume separator as a usage error (exit 2). On that specific error we retry once without the resume command and keep reporting state only for the rest of the process; other failures leave resume enabled.Compatibility
[features]flag: this is env-var driven and on whenever it is running inside Herdr. Registry row added (herdr-agent-state), plus a CHANGELOG entry.pane.report_agent/pane.release_agentcontract; the CLI is the portable path.Verification
cargo check -p codewhale-tui --lib— clean (workspace lints setwarnings = "deny").cargo fmt --all— no changes.python3 scripts/check-blocking-calls-budget.py— within budget (541 sites, 164 files).cargo test -p codewhale-tui --test feature_registry— the new row validates. The test currently fails on three pre-existing rows unrelated to this change (computer-use,runtime-api,runtime-provider-keysall have summaries over the 120-char limit onmain); this PR does not touch them.Known limitation
A hard abort (panic hook or fatal-signal path) skips
release. Herdr's own idle-prompt safety net then clears the pane a second or two later; noted in the module header.Refs #6094
Maintainer release scope: include this contribution in 0.10.2. The release integration will carry the dated changelog slice and contributor credits.