Skip to content

Surface herdr's real error on herdr-launch.sh failure paths#43

Merged
gering merged 3 commits into
mainfrom
task/surface-herdr-launch-errors
Jul 21, 2026
Merged

Surface herdr's real error on herdr-launch.sh failure paths#43
gering merged 3 commits into
mainfrom
task/surface-herdr-launch-errors

Conversation

@gering

@gering gering commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • herdr-launch.sh discarded 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 /kickoff incident where herdr's stderr named the exact cause (agent_placement_not_found: workspace w9 not found) but nobody could see it.
  • Diagnostics only: no fallback/self-heal logic, no stdout contract change, no behavior change on the success paths.

Changes

  • Capture stderr for agent start, pane move --new-tab, resume's tab create, and resume's pane run "claude -c" instead of discarding it.
  • New shared herdr_diag helper: 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).
  • One-line actionable hint appended when the error names an invalid workspace placement (agent_placement_not_found or the message otherwise naming the workspace target) — points at a stale HERDR_WORKSPACE_ID.
  • Docs + version: knowledge entry (herdr-kickoff-automation.md), CHANGELOG, work-system 1.9.0 → 1.9.1 (patch), plugin.json + marketplace.json in sync.

Readiness

  • ✅ Uncommitted changes: none
  • ✅ Rebase: up-to-date with main
  • ✅ README: no update needed (stderr-only diagnostics, no documented contract change)
  • ✅ Version bumped: work-system 1.9.0 → 1.9.1
  • ✅ Changelog: entry added
  • ✅ Knowledge: herdr-kickoff-automation.md updated
  • ✅ Tests: check-structure.py + test_agent_registry.py pass (shellcheck clean)
  • ➖ Linter: N/A (no linter configured)
  • ➖ Build: N/A (markdown/bash plugin repo)

Test plan

  • bash -n + shellcheck on herdr-launch.sh
  • Stubbed herdr binary reproducing the real incident (agent_placement_not_found) on agent start — confirms the richer error + stale-workspace hint print before the generic message
  • Stubbed failures for pane move --new-tab, resume's tab create, resume's pane run — each surfaces herdr's captured error
  • Stubbed success paths for launch and resume (reuse) — stdout key=value contract unchanged
  • python3 scripts/check-structure.py — 0 errors
  • python3 plugins/work-system/scripts/test_agent_registry.py — all tests passed

🤖 Generated with Claude Code

gering and others added 3 commits July 19, 2026 22:34
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
@gering
gering merged commit 799006d into main Jul 21, 2026
1 check passed
@gering
gering deleted the task/surface-herdr-launch-errors branch July 21, 2026 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant