Surface herdr's real error on herdr-launch.sh failure paths#43
Merged
Conversation
herdr-launch.sh discarded herdr's stderr (2>/dev/null) on every failable
call, so a launch/resume failure only ever showed the generic "did not
return a pane id" guard message — as hit in a real /kickoff incident
where herdr's stderr named the exact cause (agent_placement_not_found:
workspace w9 not found) but nobody could see it.
- Capture stderr for `agent start`, `pane move --new-tab`, resume's
`tab create`, and resume's `pane run "claude -c"`.
- Add a shared `herdr_diag` helper: parse herdr's
{"error":{"code","message"}} JSON defensively (raw text fallback,
never a traceback), print it before the existing generic/contextual
message.
- Append a one-line actionable hint when the error names an invalid
workspace placement, pointing at a stale HERDR_WORKSPACE_ID.
- Diagnostics only: stdout key=value lines, exit codes, and the
moved/reused/resumed/focused semantics are unchanged.
Bump work-system 1.9.0 -> 1.9.1 (patch), update CHANGELOG and the
herdr-kickoff-automation knowledge entry.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017D6Ji3oU3md1ENxY2WfbcT
A swarm review (codex + grok, external-only) of the herdr-launch.sh error-surfacing change found several real gaps, all fixed here: - Scope the stale-HERDR_WORKSPACE_ID hint to calls that actually send --workspace (agent start, tab create) via a new ws_relevant param on herdr_diag — pane move/pane run never take --workspace, so an agent_placement_not_found there can't be a workspace-id problem. - Anchor the message-text fallback to a bounded workspace-id token instead of a loose substring, so ws=w1 no longer false-positives on a message naming w12. - Strip control/escape bytes from herdr's stderr before it reaches the terminal (herdr's stderr is untrusted server output). - Reorder the resume pane-run failure line to print herdr's diag first, matching the other three call sites. - Add `local` to herdr_diag's working variables (they leaked into the global scope under set -eu). - Add an EXIT trap for the four stderr-capture tempfiles, matching the existing herdr-tab-glyph.sh convention (a SIGINT between mktemp and the matching rm -f would otherwise leak the file). - kickoff/continue SKILL.md now explicitly instruct relaying the helper's stderr diagnostic to the user on every failure path — the richer diagnostic is only useful if the skill layer actually shows it, which the prior wording didn't require. Update CHANGELOG (same 1.9.1 entry, not yet released) and the herdr-kickoff-automation knowledge entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017D6Ji3oU3md1ENxY2WfbcT
A second swarm review of the round-1 fixes found the fixes themselves were still wrong in four ways: - Control bytes were stripped only from the RAW stderr blob; a JSON backslash-u-001b-style escape is still plain printable text at that point and only becomes a real ESC byte after python3's json.load decodes it. code/message now get their own strip pass post-decode too. - The "workspace id as a bounded token" check tested token-presence and keyword-presence as independent ANDs, so "workspace w1 is healthy; agent placement is unavailable" still false-triggered the stale-workspace hint. Replaced the bash sed/grep chain with a single python3 re-based check that requires both in the same clause (split on ;/./,), case-insensitively. - The ERE-escaping of $ws before embedding it in a grep -E pattern only escaped a small char class, leaving several ERE metacharacters live — folded into the same python3 rewrite (re.escape covers every metachar). - $HERDR_WORKSPACE_ID itself was never sanitized before being interpolated into the printed hint line — only herdr's own stderr was. Also: the orphaned-tab `herdr tab close` cleanup call's own stderr was discarded even though the CHANGELOG claimed every failing call surfaces stderr — now captured too. And SKILL.md wording that promised a stale-workspace hint on the pane-move/pane-run paths (where it can never fire, ws_relevant=0) is corrected. Add test_herdr_launch.py (mirrors test_agent_registry.py's stub-the- CLI-on-PATH pattern) to lock in all of the above as regression coverage, since this class of bug — a diagnostic helper's own sanitization logic — clearly needed it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017D6Ji3oU3md1ENxY2WfbcT
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.
Summary
herdr-launch.shdiscarded herdr's stderr (2>/dev/null) on every call that can fail, so a launch/resume failure only ever showed the generic "did not return a pane id" guard — as hit in a real/kickoffincident where herdr's stderr named the exact cause (agent_placement_not_found: workspace w9 not found) but nobody could see it.Changes
agent start,pane move --new-tab, resume'stab create, and resume'spane run "claude -c"instead of discarding it.herdr_diaghelper: parses herdr's{"error":{"code","message"}}JSON defensively (raw stderr text as fallback, any exception → never a traceback), prints it before the existing generic/contextual message (kept as the last-resort line).agent_placement_not_foundor the message otherwise naming the workspace target) — points at a staleHERDR_WORKSPACE_ID.herdr-kickoff-automation.md), CHANGELOG, work-system 1.9.0 → 1.9.1 (patch),plugin.json+marketplace.jsonin sync.Readiness
mainherdr-kickoff-automation.mdupdatedcheck-structure.py+test_agent_registry.pypass (shellcheckclean)Test plan
bash -n+shellcheckonherdr-launch.shherdrbinary reproducing the real incident (agent_placement_not_found) onagent start— confirms the richer error + stale-workspace hint print before the generic messagepane move --new-tab, resume'stab create, resume'spane run— each surfaces herdr's captured errorlaunchandresume(reuse) — stdout key=value contract unchangedpython3 scripts/check-structure.py— 0 errorspython3 plugins/work-system/scripts/test_agent_registry.py— all tests passed🤖 Generated with Claude Code