feat(herdr): Herdr compat improvements: overhaul agent state reporting and pane integration - #3541
Open
taoeffect wants to merge 23 commits into
Open
feat(herdr): Herdr compat improvements: overhaul agent state reporting and pane integration#3541taoeffect wants to merge 23 commits into
taoeffect wants to merge 23 commits into
Conversation
Switching sessions mid-run left runActive set: the old session's RunComplete is dropped by the session gate, so nothing could clear it and the pane stayed 'working' until a whole new turn finished. SetSession now clears runActive/summarizing on a real switch and recomputes. Blocks survive: they are not session-scoped.
Key herdr blocks by (reason, id) so a resolution only clears the prompt it belongs to, and stop treating the permission service's "request started" notification as a resolution. permissionService.Request publishes a PermissionNotification with neither Granted nor Denied before publishing the request itself, and BridgeLocal consumes the two brokers on separate goroutines, so that marker could land after the request and leave the pane reporting working while the dialog was still on screen. setExclusiveBlockLocked keeps the stale-block self-heal the marker used to provide: both prompting services serialize their requests, so a new prompt proves any older same-reason block is over.
Permission blocks reported an empty herdr message, so the dominant blocking case showed no reason in the pane. PermissionRequested now carries ToolName/Description from both the domain and proto vocabularies, and the block message renders as "Permission: <tool> - <first line of description>", capped at herdr's 80-rune text limit.
Skip the herdr session-title lookup when no herdr client is attached. The lookup is an HTTP GET in client/server mode and a DB read locally, and it ran on every session switch and every reconnect re-assert even outside a herdr pane, where nothing consumes the title.
Mirror the learned session id into the pane presentation so a state report's agent_session_id and the session metadata token can never name different sessions.
Map RunStarted only from user-message creation. Both Translate branches gated on pubsub.DeletedEvent, so a message.Service.Update on a user message would re-arm runActive with no matching RunComplete and strand the pane in working.
Document the herdr notification style in README.md; it was the last place still listing the auto|native|osc|bell|disabled enum.
WithoutHerdrEnv's doc claimed it removed all HERDR_* variables; it removes the three pane-ownership vars. HERDR_TAB_ID and HERDR_WORKSPACE_ID confer no pane authority and are deliberately kept. Reword the doc and the herdrEnvVars comment, and fix the test that encoded the same claim.
Send herdr notifications under the client mutex like every other transport call, and rename the block-message truncation helper to the text-field helper it actually is.
Rename truncateRunes's max parameter to limit so the helper stops shadowing the builtin max inside its own body.
💘 Generated with Crush Assisted-by: Crush:kimi-k3-fast-api
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Crush’s integration with Herdr by making pane state reporting accurate and resilient across session switches, compaction, sub-agent activity, permission prompts, questions, and re-auth flows. It also adds pane metadata (session title + model) and an opt-in Herdr notification backend so toasts can appear in Herdr’s UI.
Changes:
- Overhauls Herdr state translation/state-machine to correctly report
working,blocked(with reason), andidle, including compaction lifecycle and session scoping. - Reports pane presentation metadata (session title + session/model tokens) and refreshes them on changes.
- Adds
notifications=herdrbackend and hardens pane-ownership isolation by stripping Herdr authority env vars and disabling Herdr integration incrush server.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| schema.json | Adds herdr notification style to the JSON schema. |
| README.md | Documents notifications=herdr behavior and fallback. |
| internal/workspace/client_workspace.go | Reports session title/model to Herdr on session switches and workspace refresh. |
| internal/workspace/client_workspace_test.go | Pins that non-Herdr runs avoid extra session title lookups. |
| internal/workspace/app_workspace.go | Threads context into session reporting for Herdr integration. |
| internal/ui/notification/notification.go | Updates notification backend docs to include Herdr. |
| internal/ui/notification/herdr.go | Introduces Herdr notification backend implementation. |
| internal/ui/notification/herdr_test.go | Tests Herdr notification backend behavior. |
| internal/ui/model/ui.go | Selects Herdr notification backend (with OSC fallback). |
| internal/ui/dialog/notifications.go | Exposes “Herdr” notification style in the UI dialog. |
| internal/shell/shell.go | Ensures subprocesses don’t inherit Herdr pane-ownership env vars. |
| internal/shell/run.go | Exports WithoutHerdrEnv and narrows stripping to pane-ownership vars only. |
| internal/shell/run_test.go | Updates tests to reflect the new env-stripping semantics. |
| internal/herdr/translate.go | Expands event translation to cover blocked reasons, compaction, and session metadata updates. |
| internal/herdr/translate_test.go | Adds extensive unit coverage for the new translation rules and bridge wiring. |
| internal/herdr/translate_integration_test.go | Adds integration tests pinning DB-backed publish/translate contracts. |
| internal/herdr/client.go | Reworks Herdr client state machine: blocked reasons, compaction lifecycle, metadata reporting, notifications, and server disablement. |
| internal/herdr/client_test.go | Adds comprehensive tests for state/metadata sequencing, blocking correlation, truncation, and concurrency assumptions. |
| internal/config/config.go | Adds herdr as a notification enum in config schema tags. |
| internal/cmd/server.go | Disables Herdr integration for detached server processes. |
| internal/cmd/run.go | Reports session title and model in non-interactive mode. |
| internal/cmd/root.go | Strips Herdr pane-ownership env vars when starting detached server. |
| internal/app/resolve_session_test.go | Extends session service mock to track Get calls (for “no lookup” assertions). |
| internal/app/report_current_session_test.go | Verifies no session title DB lookup occurs without a Herdr client. |
| internal/app/app.go | Bridges more local pubsub sources into Herdr and reports model/session title metadata. |
| docs/config/README.md | Updates config docs to include notifications=herdr. |
Suppressed comments (1)
internal/herdr/translate.go:87
- Proto question text has the same single-line constraint as domain questions, but this translation can forward multi-line text (it only truncates). Use firstLine before truncation so the blocked reason is always one line.
var text string
if len(e.Payload.Questions) > 0 {
text = e.Payload.Questions[0].Question
}
return QuestionAsked{BatchID: e.Payload.ID, Text: truncateText(text)}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Reduce question text to its first line before truncation on both translation paths so model-generated multi-line questions cannot leak newlines into herdr's single-line message field, and document that the herdr notification style falls back to OSC outside a herdr pane instead of claiming a pane is required. 💘 Generated with Crush Assisted-by: Crush:kimi-k3-fast-api
💘 Generated with Crush Assisted-by: Crush:kimi-k3-fast-api
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.
This PR makes Crush work better inside Herdr. Crush now reliably tells Herdr what it's doing — working, waiting on you, or idle — without getting stuck in the wrong state when you switch sessions, compact context, run sub-agents, or need to re-authenticate. When Crush is blocked, Herdr shows what it's waiting on (a permission prompt, a question, or a sign-in). Crush also shares the current session title and model with Herdr, updates them as they change, and can now send its toast notifications straight to Herdr's UI (opt-in).
Closes #3539
Note: This PR also fixes an unrelated flaky test that existed in
mainvia commit 207c17c.CONTRIBUTING.md.AI Disclosure
Planning and Implementation: Kimi K3 (max)
Review and fixes: Opus 5 (x-high) + Copilot
Manual testing by me.